From fe5d0849dbe4a8a6f4d8cd1d0384b9112fc802b9 Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 5 Nov 2020 17:21:13 -0500 Subject: cmNinjaTargetGenerator: Consolidate redundant methods --- Source/cmNinjaTargetGenerator.cxx | 12 +++--------- Source/cmNinjaTargetGenerator.h | 3 +-- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/Source/cmNinjaTargetGenerator.cxx b/Source/cmNinjaTargetGenerator.cxx index 5e27356..41067d6 100644 --- a/Source/cmNinjaTargetGenerator.cxx +++ b/Source/cmNinjaTargetGenerator.cxx @@ -129,12 +129,6 @@ bool cmNinjaTargetGenerator::NeedExplicitPreprocessing( return lang == "Fortran"; } -bool cmNinjaTargetGenerator::UsePreprocessedSource( - std::string const& lang) const -{ - return lang == "Fortran"; -} - bool cmNinjaTargetGenerator::CompilePreprocessedSourceWithDefines( std::string const& lang) const { @@ -639,8 +633,8 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang, // For some cases we do an explicit preprocessor invocation. bool const explicitPP = this->NeedExplicitPreprocessing(lang); - bool const compilePPWithDefines = this->UsePreprocessedSource(lang) && - this->CompilePreprocessedSourceWithDefines(lang); + bool const compilePPWithDefines = + explicitPP && this->CompilePreprocessedSourceWithDefines(lang); bool const needDyndep = this->NeedDyndep(lang); std::string flags = "$FLAGS"; @@ -1303,7 +1297,7 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatement( preprocess = cmOutputConverter::GetFortranPreprocess(tgtpp); } - bool const compilePP = this->UsePreprocessedSource(language) && + bool const compilePP = explicitPP && (preprocess != cmOutputConverter::FortranPreprocess::NotNeeded); bool const compilePPWithDefines = compilePP && this->CompilePreprocessedSourceWithDefines(language); diff --git a/Source/cmNinjaTargetGenerator.h b/Source/cmNinjaTargetGenerator.h index a27c9b4..17c7747 100644 --- a/Source/cmNinjaTargetGenerator.h +++ b/Source/cmNinjaTargetGenerator.h @@ -71,11 +71,10 @@ protected: const std::string& config) const; std::string LanguageDependencyRule(std::string const& lang, const std::string& config) const; - bool NeedExplicitPreprocessing(std::string const& lang) const; std::string LanguageDyndepRule(std::string const& lang, const std::string& config) const; bool NeedDyndep(std::string const& lang) const; - bool UsePreprocessedSource(std::string const& lang) const; + bool NeedExplicitPreprocessing(std::string const& lang) const; bool CompilePreprocessedSourceWithDefines(std::string const& lang) const; std::string OrderDependsTargetForTarget(const std::string& config); -- cgit v0.12 From fcf3fc4447e8eaa9cdd67223d9bf6ca8975632fa Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 5 Nov 2020 17:22:22 -0500 Subject: cmNinjaTargetGenerator: Clarify method names --- Source/cmNinjaTargetGenerator.cxx | 21 ++++++++++----------- Source/cmNinjaTargetGenerator.h | 10 +++++----- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/Source/cmNinjaTargetGenerator.cxx b/Source/cmNinjaTargetGenerator.cxx index 41067d6..534fb3d 100644 --- a/Source/cmNinjaTargetGenerator.cxx +++ b/Source/cmNinjaTargetGenerator.cxx @@ -105,7 +105,7 @@ std::string cmNinjaTargetGenerator::LanguageCompilerRule( '_', config); } -std::string cmNinjaTargetGenerator::LanguagePreprocessRule( +std::string cmNinjaTargetGenerator::LanguagePreprocessAndScanRule( std::string const& lang, const std::string& config) const { return cmStrCat( @@ -114,7 +114,7 @@ std::string cmNinjaTargetGenerator::LanguagePreprocessRule( '_', config); } -std::string cmNinjaTargetGenerator::LanguageDependencyRule( +std::string cmNinjaTargetGenerator::LanguageScanRule( std::string const& lang, const std::string& config) const { return cmStrCat( @@ -129,8 +129,7 @@ bool cmNinjaTargetGenerator::NeedExplicitPreprocessing( return lang == "Fortran"; } -bool cmNinjaTargetGenerator::CompilePreprocessedSourceWithDefines( - std::string const& lang) const +bool cmNinjaTargetGenerator::CompileWithDefines(std::string const& lang) const { return this->Makefile->IsOn( cmStrCat("CMAKE_", lang, "_COMPILE_WITH_DEFINES")); @@ -634,7 +633,7 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang, // For some cases we do an explicit preprocessor invocation. bool const explicitPP = this->NeedExplicitPreprocessing(lang); bool const compilePPWithDefines = - explicitPP && this->CompilePreprocessedSourceWithDefines(lang); + explicitPP && this->CompileWithDefines(lang); bool const needDyndep = this->NeedDyndep(lang); std::string flags = "$FLAGS"; @@ -675,8 +674,8 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang, const auto ppVar = cmStrCat("CMAKE_", lang, "_PREPROCESS_SOURCE"); auto ppRule = GetPreprocessScanRule( - this->LanguagePreprocessRule(lang, config), vars, responseFlag, flags, - launcher, rulePlaceholderExpander.get(), ppScanCommand, + this->LanguagePreprocessAndScanRule(lang, config), vars, responseFlag, + flags, launcher, rulePlaceholderExpander.get(), ppScanCommand, this->GetLocalGenerator(), mf->GetRequiredDefinition(ppVar)); // Write the rule for preprocessing file of the given language. @@ -695,7 +694,7 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang, GetScanCommand(cmakeCmd, tdi, lang, "$in", needDyndep, "$out"); auto scanRule = GetPreprocessScanRule( - this->LanguageDependencyRule(lang, config), vars, "", flags, launcher, + this->LanguageScanRule(lang, config), vars, "", flags, launcher, rulePlaceholderExpander.get(), scanCommand, this->GetLocalGenerator()); // Write the rule for generating dependencies for the given language. @@ -1300,15 +1299,15 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatement( bool const compilePP = explicitPP && (preprocess != cmOutputConverter::FortranPreprocess::NotNeeded); bool const compilePPWithDefines = - compilePP && this->CompilePreprocessedSourceWithDefines(language); + compilePP && this->CompileWithDefines(language); std::string const ppFileName = compilePP ? this->ConvertToNinjaPath(this->GetPreprocessedFilePath(source, config)) : ""; std::string const buildName = compilePP - ? this->LanguagePreprocessRule(language, config) - : this->LanguageDependencyRule(language, config); + ? this->LanguagePreprocessAndScanRule(language, config) + : this->LanguageScanRule(language, config); const auto depExtension = compilePP ? ".pp.d" : ".d"; const std::string depFileName = diff --git a/Source/cmNinjaTargetGenerator.h b/Source/cmNinjaTargetGenerator.h index 17c7747..4ba37ad 100644 --- a/Source/cmNinjaTargetGenerator.h +++ b/Source/cmNinjaTargetGenerator.h @@ -67,15 +67,15 @@ protected: std::string LanguageCompilerRule(const std::string& lang, const std::string& config) const; - std::string LanguagePreprocessRule(std::string const& lang, - const std::string& config) const; - std::string LanguageDependencyRule(std::string const& lang, - const std::string& config) const; + std::string LanguagePreprocessAndScanRule(std::string const& lang, + const std::string& config) const; + std::string LanguageScanRule(std::string const& lang, + const std::string& config) const; std::string LanguageDyndepRule(std::string const& lang, const std::string& config) const; bool NeedDyndep(std::string const& lang) const; bool NeedExplicitPreprocessing(std::string const& lang) const; - bool CompilePreprocessedSourceWithDefines(std::string const& lang) const; + bool CompileWithDefines(std::string const& lang) const; std::string OrderDependsTargetForTarget(const std::string& config); -- cgit v0.12 From 170cfc764df71f878d8f7e280a24060604b7c49a Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 5 Nov 2020 17:36:13 -0500 Subject: cmNinjaTargetGenerator: Drop unnecessary mutation GetPreprocessScanRule's caller always has `vars.Source` set to `$in`. --- Source/cmNinjaTargetGenerator.cxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/cmNinjaTargetGenerator.cxx b/Source/cmNinjaTargetGenerator.cxx index 534fb3d..757245f 100644 --- a/Source/cmNinjaTargetGenerator.cxx +++ b/Source/cmNinjaTargetGenerator.cxx @@ -543,7 +543,8 @@ std::string GetScanCommand(const std::string& cmakeCmd, const std::string& tdi, // Helper function to create dependency scanning rule, with optional // explicit preprocessing step if preprocessCommand is non-empty cmNinjaRule GetPreprocessScanRule( - const std::string& ruleName, cmRulePlaceholderExpander::RuleVariables& vars, + const std::string& ruleName, + cmRulePlaceholderExpander::RuleVariables const& vars, const std::string& responseFlag, const std::string& flags, const std::string& launcher, cmRulePlaceholderExpander* const rulePlaceholderExpander, @@ -566,7 +567,6 @@ cmNinjaRule GetPreprocessScanRule( // Preprocessing uses the original source, compilation uses // preprocessed output or original source ppVars.Source = vars.Source; - vars.Source = "$in"; // Copy preprocessor definitions to the preprocessor rule. ppVars.Defines = vars.Defines; -- cgit v0.12 From f2eec04728fef5ec2d2db9e56a4498e1902ed17a Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 5 Nov 2020 17:29:46 -0500 Subject: cmNinjaTargetGenerator: Clarify scan rule helper functions Revise names to clarify that these helpers are for the dependency scanning commands, which may happen to preprocess. --- Source/cmNinjaTargetGenerator.cxx | 141 ++++++++++++++++++-------------------- 1 file changed, 68 insertions(+), 73 deletions(-) diff --git a/Source/cmNinjaTargetGenerator.cxx b/Source/cmNinjaTargetGenerator.cxx index 757245f..7277798 100644 --- a/Source/cmNinjaTargetGenerator.cxx +++ b/Source/cmNinjaTargetGenerator.cxx @@ -540,9 +540,9 @@ std::string GetScanCommand(const std::string& cmakeCmd, const std::string& tdi, return ccmd; } -// Helper function to create dependency scanning rule, with optional -// explicit preprocessing step if preprocessCommand is non-empty -cmNinjaRule GetPreprocessScanRule( +// Helper function to create dependency scanning rule that may or may +// not perform explicit preprocessing too. +cmNinjaRule GetScanRule( const std::string& ruleName, cmRulePlaceholderExpander::RuleVariables const& vars, const std::string& responseFlag, const std::string& flags, @@ -552,59 +552,53 @@ cmNinjaRule GetPreprocessScanRule( const std::string& preprocessCommand = "") { cmNinjaRule rule(ruleName); - // Explicit preprocessing always uses a depfile. + // Scanning always uses a depfile for preprocessor dependencies. rule.DepType = ""; // no deps= for multiple outputs rule.DepFile = "$DEP_FILE"; - cmRulePlaceholderExpander::RuleVariables ppVars; - ppVars.CMTargetName = vars.CMTargetName; - ppVars.CMTargetType = vars.CMTargetType; - ppVars.Language = vars.Language; - ppVars.Object = "$out"; // for RULE_LAUNCH_COMPILE - ppVars.PreprocessedSource = "$out"; - ppVars.DependencyFile = rule.DepFile.c_str(); - - // Preprocessing uses the original source, compilation uses - // preprocessed output or original source - ppVars.Source = vars.Source; - - // Copy preprocessor definitions to the preprocessor rule. - ppVars.Defines = vars.Defines; + cmRulePlaceholderExpander::RuleVariables scanVars; + scanVars.CMTargetName = vars.CMTargetName; + scanVars.CMTargetType = vars.CMTargetType; + scanVars.Language = vars.Language; + scanVars.Object = "$out"; // for RULE_LAUNCH_COMPILE + scanVars.PreprocessedSource = "$out"; + scanVars.DependencyFile = rule.DepFile.c_str(); - // Copy include directories to the preprocessor rule. The Fortran - // compilation rule still needs them for the INCLUDE directive. - ppVars.Includes = vars.Includes; + // Scanning needs the same preprocessor settings as direct compilation would. + scanVars.Source = vars.Source; + scanVars.Defines = vars.Defines; + scanVars.Includes = vars.Includes; - // Preprocessing and compilation use the same flags. - std::string ppFlags = flags; + // Scanning needs the compilation flags too. + std::string scanFlags = flags; // If using a response file, move defines, includes, and flags into it. if (!responseFlag.empty()) { rule.RspFile = "$RSP_FILE"; rule.RspContent = - cmStrCat(' ', ppVars.Defines, ' ', ppVars.Includes, ' ', ppFlags); - ppFlags = cmStrCat(responseFlag, rule.RspFile); - ppVars.Defines = ""; - ppVars.Includes = ""; + cmStrCat(' ', scanVars.Defines, ' ', scanVars.Includes, ' ', scanFlags); + scanFlags = cmStrCat(responseFlag, rule.RspFile); + scanVars.Defines = ""; + scanVars.Includes = ""; } - ppVars.Flags = ppFlags.c_str(); + scanVars.Flags = scanFlags.c_str(); - // Rule for preprocessing source file. - std::vector ppCmds; + // Rule for scanning a source file. + std::vector scanCmds; if (!preprocessCommand.empty()) { // Lookup the explicit preprocessing rule. - cmExpandList(preprocessCommand, ppCmds); - for (std::string& i : ppCmds) { + cmExpandList(preprocessCommand, scanCmds); + for (std::string& i : scanCmds) { i = cmStrCat(launcher, i); - rulePlaceholderExpander->ExpandRuleVariables(generator, i, ppVars); + rulePlaceholderExpander->ExpandRuleVariables(generator, i, scanVars); } } // Run CMake dependency scanner on either preprocessed output or source file - ppCmds.emplace_back(std::move(scanCommand)); - rule.Command = generator->BuildCommandLine(ppCmds); + scanCmds.emplace_back(std::move(scanCommand)); + rule.Command = generator->BuildCommandLine(scanCmds); return rule; } @@ -673,7 +667,7 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang, cmakeCmd, tdi, lang, "$out", needDyndep, "$DYNDEP_INTERMEDIATE_FILE"); const auto ppVar = cmStrCat("CMAKE_", lang, "_PREPROCESS_SOURCE"); - auto ppRule = GetPreprocessScanRule( + auto ppRule = GetScanRule( this->LanguagePreprocessAndScanRule(lang, config), vars, responseFlag, flags, launcher, rulePlaceholderExpander.get(), ppScanCommand, this->GetLocalGenerator(), mf->GetRequiredDefinition(ppVar)); @@ -693,9 +687,9 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang, const auto scanCommand = GetScanCommand(cmakeCmd, tdi, lang, "$in", needDyndep, "$out"); - auto scanRule = GetPreprocessScanRule( - this->LanguageScanRule(lang, config), vars, "", flags, launcher, - rulePlaceholderExpander.get(), scanCommand, this->GetLocalGenerator()); + auto scanRule = GetScanRule(this->LanguageScanRule(lang, config), vars, "", + flags, launcher, rulePlaceholderExpander.get(), + scanCommand, this->GetLocalGenerator()); // Write the rule for generating dependencies for the given language. scanRule.Comment = cmStrCat("Rule for generating ", lang, @@ -1070,57 +1064,58 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatements( } namespace { -cmNinjaBuild GetPreprocessOrScanBuild( - const std::string& ruleName, const std::string& ppFileName, bool compilePP, - bool compilePPWithDefines, cmNinjaBuild& objBuild, cmNinjaVars& vars, - const std::string& depFileName, bool needDyndep, - const std::string& objectFileName) +cmNinjaBuild GetScanBuildStatement(const std::string& ruleName, + const std::string& ppFileName, + bool compilePP, bool compilePPWithDefines, + cmNinjaBuild& objBuild, cmNinjaVars& vars, + const std::string& depFileName, + bool needDyndep, + const std::string& objectFileName) { - // Explicit preprocessing and dependency - cmNinjaBuild ppBuild(ruleName); + cmNinjaBuild scanBuild(ruleName); if (!ppFileName.empty()) { - ppBuild.Outputs.push_back(ppFileName); - ppBuild.RspFile = cmStrCat(ppFileName, ".rsp"); + scanBuild.Outputs.push_back(ppFileName); + scanBuild.RspFile = cmStrCat(ppFileName, ".rsp"); } else { - ppBuild.RspFile = "$out.rsp"; + scanBuild.RspFile = "$out.rsp"; } if (compilePP) { - // Move compilation dependencies to the preprocessing build statement. - std::swap(ppBuild.ExplicitDeps, objBuild.ExplicitDeps); - std::swap(ppBuild.ImplicitDeps, objBuild.ImplicitDeps); - std::swap(ppBuild.OrderOnlyDeps, objBuild.OrderOnlyDeps); - std::swap(ppBuild.Variables["IN_ABS"], vars["IN_ABS"]); + // Move compilation dependencies to the scan/preprocessing build statement. + std::swap(scanBuild.ExplicitDeps, objBuild.ExplicitDeps); + std::swap(scanBuild.ImplicitDeps, objBuild.ImplicitDeps); + std::swap(scanBuild.OrderOnlyDeps, objBuild.OrderOnlyDeps); + std::swap(scanBuild.Variables["IN_ABS"], vars["IN_ABS"]); // The actual compilation will now use the preprocessed source. objBuild.ExplicitDeps.push_back(ppFileName); } else { - // Copy compilation dependencies to the preprocessing build statement. - ppBuild.ExplicitDeps = objBuild.ExplicitDeps; - ppBuild.ImplicitDeps = objBuild.ImplicitDeps; - ppBuild.OrderOnlyDeps = objBuild.OrderOnlyDeps; - ppBuild.Variables["IN_ABS"] = vars["IN_ABS"]; + // Copy compilation dependencies to the scan/preprocessing build statement. + scanBuild.ExplicitDeps = objBuild.ExplicitDeps; + scanBuild.ImplicitDeps = objBuild.ImplicitDeps; + scanBuild.OrderOnlyDeps = objBuild.OrderOnlyDeps; + scanBuild.Variables["IN_ABS"] = vars["IN_ABS"]; } - // Preprocessing and compilation generally use the same flags. - ppBuild.Variables["FLAGS"] = vars["FLAGS"]; + // Scanning and compilation generally use the same flags. + scanBuild.Variables["FLAGS"] = vars["FLAGS"]; if (compilePP && !compilePPWithDefines) { - // Move preprocessor definitions to the preprocessor build statement. - std::swap(ppBuild.Variables["DEFINES"], vars["DEFINES"]); + // Move preprocessor definitions to the scan/preprocessor build statement. + std::swap(scanBuild.Variables["DEFINES"], vars["DEFINES"]); } else { - // Copy preprocessor definitions to the preprocessor build statement. - ppBuild.Variables["DEFINES"] = vars["DEFINES"]; + // Copy preprocessor definitions to the scan/preprocessor build statement. + scanBuild.Variables["DEFINES"] = vars["DEFINES"]; } // Copy include directories to the preprocessor build statement. The // Fortran compilation build statement still needs them for the INCLUDE // directive. - ppBuild.Variables["INCLUDES"] = vars["INCLUDES"]; + scanBuild.Variables["INCLUDES"] = vars["INCLUDES"]; // Explicit preprocessing always uses a depfile. - ppBuild.Variables["DEP_FILE"] = depFileName; + scanBuild.Variables["DEP_FILE"] = depFileName; if (compilePP) { // The actual compilation does not need a depfile because it // depends on the already-preprocessed source. @@ -1130,18 +1125,18 @@ cmNinjaBuild GetPreprocessOrScanBuild( if (needDyndep) { // Tell dependency scanner the object file that will result from // compiling the source. - ppBuild.Variables["OBJ_FILE"] = objectFileName; + scanBuild.Variables["OBJ_FILE"] = objectFileName; // Tell dependency scanner where to store dyndep intermediate results. std::string const ddiFile = cmStrCat(objectFileName, ".ddi"); if (ppFileName.empty()) { - ppBuild.Outputs.push_back(ddiFile); + scanBuild.Outputs.push_back(ddiFile); } else { - ppBuild.Variables["DYNDEP_INTERMEDIATE_FILE"] = ddiFile; - ppBuild.ImplicitOuts.push_back(ddiFile); + scanBuild.Variables["DYNDEP_INTERMEDIATE_FILE"] = ddiFile; + scanBuild.ImplicitOuts.push_back(ddiFile); } } - return ppBuild; + return scanBuild; } } @@ -1314,7 +1309,7 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatement( this->GetLocalGenerator()->ConvertToOutputFormat( cmStrCat(objectFileName, depExtension), cmOutputConverter::SHELL); - cmNinjaBuild ppBuild = GetPreprocessOrScanBuild( + cmNinjaBuild ppBuild = GetScanBuildStatement( buildName, ppFileName, compilePP, compilePPWithDefines, objBuild, vars, depFileName, needDyndep, objectFileName); -- cgit v0.12 From 1416012f2cd1d385c74eec69e880c0f1b00e9965 Mon Sep 17 00:00:00 2001 From: Brad King Date: Fri, 6 Nov 2020 11:05:47 -0500 Subject: cmNinjaTargetGenerator: Clarify scan rule code grouping --- Source/cmNinjaTargetGenerator.cxx | 52 +++++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 24 deletions(-) diff --git a/Source/cmNinjaTargetGenerator.cxx b/Source/cmNinjaTargetGenerator.cxx index 7277798..af4f6e1 100644 --- a/Source/cmNinjaTargetGenerator.cxx +++ b/Source/cmNinjaTargetGenerator.cxx @@ -662,42 +662,46 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang, cmSystemTools::GetCMakeCommand(), cmLocalGenerator::SHELL); if (explicitPP) { - // Combined preprocessing and dependency scanning - const auto ppScanCommand = GetScanCommand( - cmakeCmd, tdi, lang, "$out", needDyndep, "$DYNDEP_INTERMEDIATE_FILE"); - const auto ppVar = cmStrCat("CMAKE_", lang, "_PREPROCESS_SOURCE"); + // Rule to scan dependencies of sources that need preprocessing. + { + const auto ppScanCommand = GetScanCommand( + cmakeCmd, tdi, lang, "$out", needDyndep, "$DYNDEP_INTERMEDIATE_FILE"); + const auto ppVar = cmStrCat("CMAKE_", lang, "_PREPROCESS_SOURCE"); - auto ppRule = GetScanRule( - this->LanguagePreprocessAndScanRule(lang, config), vars, responseFlag, - flags, launcher, rulePlaceholderExpander.get(), ppScanCommand, - this->GetLocalGenerator(), mf->GetRequiredDefinition(ppVar)); + auto ppRule = GetScanRule( + this->LanguagePreprocessAndScanRule(lang, config), vars, responseFlag, + flags, launcher, rulePlaceholderExpander.get(), ppScanCommand, + this->GetLocalGenerator(), mf->GetRequiredDefinition(ppVar)); - // Write the rule for preprocessing file of the given language. - ppRule.Comment = cmStrCat("Rule for preprocessing ", lang, " files."); - ppRule.Description = cmStrCat("Building ", lang, " preprocessed $out"); + // Write the rule for preprocessing file of the given language. + ppRule.Comment = cmStrCat("Rule for preprocessing ", lang, " files."); + ppRule.Description = cmStrCat("Building ", lang, " preprocessed $out"); - this->GetGlobalGenerator()->AddRule(ppRule); + this->GetGlobalGenerator()->AddRule(ppRule); + } if (!compilePPWithDefines) { // Remove preprocessor definitions from compilation step vars.Defines = ""; } - // Just dependency scanning for files that have preprocessing turned off - const auto scanCommand = - GetScanCommand(cmakeCmd, tdi, lang, "$in", needDyndep, "$out"); + // Rule to scan dependencies of sources that do not need preprocessing. + { + const auto scanCommand = + GetScanCommand(cmakeCmd, tdi, lang, "$in", needDyndep, "$out"); - auto scanRule = GetScanRule(this->LanguageScanRule(lang, config), vars, "", - flags, launcher, rulePlaceholderExpander.get(), - scanCommand, this->GetLocalGenerator()); + auto scanRule = GetScanRule( + this->LanguageScanRule(lang, config), vars, "", flags, launcher, + rulePlaceholderExpander.get(), scanCommand, this->GetLocalGenerator()); - // Write the rule for generating dependencies for the given language. - scanRule.Comment = cmStrCat("Rule for generating ", lang, - " dependencies on non-preprocessed files."); - scanRule.Description = - cmStrCat("Generating ", lang, " dependencies for $in"); + // Write the rule for generating dependencies for the given language. + scanRule.Comment = cmStrCat("Rule for generating ", lang, + " dependencies on non-preprocessed files."); + scanRule.Description = + cmStrCat("Generating ", lang, " dependencies for $in"); - this->GetGlobalGenerator()->AddRule(scanRule); + this->GetGlobalGenerator()->AddRule(scanRule); + } } if (needDyndep) { -- cgit v0.12 From 43fe7e0c977459ab0e9129b099fb9eee7f2f90e8 Mon Sep 17 00:00:00 2001 From: Brad King Date: Fri, 6 Nov 2020 11:11:50 -0500 Subject: cmNinjaTargetGenerator: Revise conditions to clarify dyndep code paths All the scan-related code paths are actually about dyndep rather than explicit preprocessing. It just happens that the implementation for Fortran requires explicit preprocessing. --- Source/cmNinjaTargetGenerator.cxx | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/Source/cmNinjaTargetGenerator.cxx b/Source/cmNinjaTargetGenerator.cxx index af4f6e1..9db1726 100644 --- a/Source/cmNinjaTargetGenerator.cxx +++ b/Source/cmNinjaTargetGenerator.cxx @@ -624,11 +624,12 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang, cmMakefile* mf = this->GetMakefile(); + // For some cases we scan to dynamically discover dependencies. + bool const needDyndep = this->NeedDyndep(lang); // For some cases we do an explicit preprocessor invocation. bool const explicitPP = this->NeedExplicitPreprocessing(lang); bool const compilePPWithDefines = explicitPP && this->CompileWithDefines(lang); - bool const needDyndep = this->NeedDyndep(lang); std::string flags = "$FLAGS"; @@ -661,7 +662,7 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang, this->GetLocalGenerator()->ConvertToOutputFormat( cmSystemTools::GetCMakeCommand(), cmLocalGenerator::SHELL); - if (explicitPP) { + if (needDyndep) { // Rule to scan dependencies of sources that need preprocessing. { const auto ppScanCommand = GetScanCommand( @@ -702,9 +703,7 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang, this->GetGlobalGenerator()->AddRule(scanRule); } - } - if (needDyndep) { // Write the rule for ninja dyndep file generation. cmNinjaRule rule(this->LanguageDyndepRule(lang, config)); // Command line length is almost always limited -> use response file for @@ -1280,9 +1279,9 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatement( // For some cases we need to generate a ninja dyndep file. bool const needDyndep = this->NeedDyndep(language); - // For some cases we do an explicit preprocessor invocation. - bool const explicitPP = this->NeedExplicitPreprocessing(language); - if (explicitPP) { + if (needDyndep) { + // For some cases we do an explicit preprocessor invocation. + bool const explicitPP = this->NeedExplicitPreprocessing(language); // If source/target has preprocessing turned off, we still need to // generate an explicit dependency step @@ -1337,7 +1336,7 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatement( vars["INCLUDES"] = cmStrCat(sourceDirectoryFlag, ' ', vars["INCLUDES"]); } - if (firstForConfig && needDyndep) { + if (firstForConfig) { std::string const ddiFile = cmStrCat(objectFileName, ".ddi"); this->Configs[config].DDIFiles[language].push_back(ddiFile); } @@ -1347,8 +1346,7 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatement( this->GetGlobalGenerator()->WriteBuild(this->GetImplFileStream(fileConfig), ppBuild, commandLineLengthLimit); - } - if (needDyndep) { + std::string const dyndep = this->GetDyndepFilePath(language, config); objBuild.OrderOnlyDeps.push_back(dyndep); vars["dyndep"] = dyndep; -- cgit v0.12 From 9f60e8aa52f4347445e323f66ceaa490262eaeee Mon Sep 17 00:00:00 2001 From: Brad King Date: Fri, 6 Nov 2020 11:14:59 -0500 Subject: cmNinjaTargetGenerator: Remove redundant conditions for dyndep blocks The GetScanCommand and GetScanBuildStatement helpers are called only in code paths for dyndep. Drop their checks for this condition. --- Source/cmNinjaTargetGenerator.cxx | 45 +++++++++++++++++---------------------- 1 file changed, 19 insertions(+), 26 deletions(-) diff --git a/Source/cmNinjaTargetGenerator.cxx b/Source/cmNinjaTargetGenerator.cxx index 9db1726..231583b 100644 --- a/Source/cmNinjaTargetGenerator.cxx +++ b/Source/cmNinjaTargetGenerator.cxx @@ -529,15 +529,11 @@ namespace { // Create the command to run the dependency scanner std::string GetScanCommand(const std::string& cmakeCmd, const std::string& tdi, const std::string& lang, const std::string& ppFile, - bool needDyndep, const std::string& ddiFile) + const std::string& ddiFile) { - std::string ccmd = - cmStrCat(cmakeCmd, " -E cmake_ninja_depends --tdi=", tdi, " --lang=", lang, - " --pp=", ppFile, " --dep=$DEP_FILE"); - if (needDyndep) { - ccmd = cmStrCat(ccmd, " --obj=$OBJ_FILE --ddi=", ddiFile); - } - return ccmd; + return cmStrCat(cmakeCmd, " -E cmake_ninja_depends --tdi=", tdi, + " --lang=", lang, " --pp=", ppFile, + " --dep=$DEP_FILE --obj=$OBJ_FILE --ddi=", ddiFile); } // Helper function to create dependency scanning rule that may or may @@ -665,8 +661,8 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang, if (needDyndep) { // Rule to scan dependencies of sources that need preprocessing. { - const auto ppScanCommand = GetScanCommand( - cmakeCmd, tdi, lang, "$out", needDyndep, "$DYNDEP_INTERMEDIATE_FILE"); + const auto ppScanCommand = GetScanCommand(cmakeCmd, tdi, lang, "$out", + "$DYNDEP_INTERMEDIATE_FILE"); const auto ppVar = cmStrCat("CMAKE_", lang, "_PREPROCESS_SOURCE"); auto ppRule = GetScanRule( @@ -689,7 +685,7 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang, // Rule to scan dependencies of sources that do not need preprocessing. { const auto scanCommand = - GetScanCommand(cmakeCmd, tdi, lang, "$in", needDyndep, "$out"); + GetScanCommand(cmakeCmd, tdi, lang, "$in", "$out"); auto scanRule = GetScanRule( this->LanguageScanRule(lang, config), vars, "", flags, launcher, @@ -1072,7 +1068,6 @@ cmNinjaBuild GetScanBuildStatement(const std::string& ruleName, bool compilePP, bool compilePPWithDefines, cmNinjaBuild& objBuild, cmNinjaVars& vars, const std::string& depFileName, - bool needDyndep, const std::string& objectFileName) { cmNinjaBuild scanBuild(ruleName); @@ -1125,19 +1120,17 @@ cmNinjaBuild GetScanBuildStatement(const std::string& ruleName, vars.erase("DEP_FILE"); } - if (needDyndep) { - // Tell dependency scanner the object file that will result from - // compiling the source. - scanBuild.Variables["OBJ_FILE"] = objectFileName; - - // Tell dependency scanner where to store dyndep intermediate results. - std::string const ddiFile = cmStrCat(objectFileName, ".ddi"); - if (ppFileName.empty()) { - scanBuild.Outputs.push_back(ddiFile); - } else { - scanBuild.Variables["DYNDEP_INTERMEDIATE_FILE"] = ddiFile; - scanBuild.ImplicitOuts.push_back(ddiFile); - } + // Tell dependency scanner the object file that will result from + // compiling the source. + scanBuild.Variables["OBJ_FILE"] = objectFileName; + + // Tell dependency scanner where to store dyndep intermediate results. + std::string const ddiFile = cmStrCat(objectFileName, ".ddi"); + if (ppFileName.empty()) { + scanBuild.Outputs.push_back(ddiFile); + } else { + scanBuild.Variables["DYNDEP_INTERMEDIATE_FILE"] = ddiFile; + scanBuild.ImplicitOuts.push_back(ddiFile); } return scanBuild; } @@ -1314,7 +1307,7 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatement( cmNinjaBuild ppBuild = GetScanBuildStatement( buildName, ppFileName, compilePP, compilePPWithDefines, objBuild, vars, - depFileName, needDyndep, objectFileName); + depFileName, objectFileName); if (compilePP) { // In case compilation requires flags that are incompatible with -- cgit v0.12 From bff1871c3922a1730ea61a0b155d1ebad5d254c3 Mon Sep 17 00:00:00 2001 From: Brad King Date: Fri, 6 Nov 2020 11:21:17 -0500 Subject: cmNinjaTargetGenerator: Generalize GetScanRule helper Allow the caller to provide any number of commands, and to choose which ones get a launcher. --- Source/cmNinjaTargetGenerator.cxx | 57 +++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 30 deletions(-) diff --git a/Source/cmNinjaTargetGenerator.cxx b/Source/cmNinjaTargetGenerator.cxx index 231583b..49d2ae2 100644 --- a/Source/cmNinjaTargetGenerator.cxx +++ b/Source/cmNinjaTargetGenerator.cxx @@ -542,10 +542,8 @@ cmNinjaRule GetScanRule( const std::string& ruleName, cmRulePlaceholderExpander::RuleVariables const& vars, const std::string& responseFlag, const std::string& flags, - const std::string& launcher, cmRulePlaceholderExpander* const rulePlaceholderExpander, - std::string scanCommand, cmLocalNinjaGenerator* generator, - const std::string& preprocessCommand = "") + cmLocalNinjaGenerator* generator, std::vector scanCmds) { cmNinjaRule rule(ruleName); // Scanning always uses a depfile for preprocessor dependencies. @@ -581,19 +579,9 @@ cmNinjaRule GetScanRule( scanVars.Flags = scanFlags.c_str(); // Rule for scanning a source file. - std::vector scanCmds; - - if (!preprocessCommand.empty()) { - // Lookup the explicit preprocessing rule. - cmExpandList(preprocessCommand, scanCmds); - for (std::string& i : scanCmds) { - i = cmStrCat(launcher, i); - rulePlaceholderExpander->ExpandRuleVariables(generator, i, scanVars); - } + for (std::string& scanCmd : scanCmds) { + rulePlaceholderExpander->ExpandRuleVariables(generator, scanCmd, scanVars); } - - // Run CMake dependency scanner on either preprocessed output or source file - scanCmds.emplace_back(std::move(scanCommand)); rule.Command = generator->BuildCommandLine(scanCmds); return rule; @@ -661,20 +649,27 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang, if (needDyndep) { // Rule to scan dependencies of sources that need preprocessing. { - const auto ppScanCommand = GetScanCommand(cmakeCmd, tdi, lang, "$out", - "$DYNDEP_INTERMEDIATE_FILE"); - const auto ppVar = cmStrCat("CMAKE_", lang, "_PREPROCESS_SOURCE"); + std::vector scanCommands; + std::string const& scanRuleName = + this->LanguagePreprocessAndScanRule(lang, config); + std::string const& ppCommmand = mf->GetRequiredDefinition( + cmStrCat("CMAKE_", lang, "_PREPROCESS_SOURCE")); + cmExpandList(ppCommmand, scanCommands); + for (std::string& i : scanCommands) { + i = cmStrCat(launcher, i); + } + scanCommands.emplace_back(GetScanCommand(cmakeCmd, tdi, lang, "$out", + "$DYNDEP_INTERMEDIATE_FILE")); - auto ppRule = GetScanRule( - this->LanguagePreprocessAndScanRule(lang, config), vars, responseFlag, - flags, launcher, rulePlaceholderExpander.get(), ppScanCommand, - this->GetLocalGenerator(), mf->GetRequiredDefinition(ppVar)); + auto scanRule = GetScanRule( + scanRuleName, vars, responseFlag, flags, rulePlaceholderExpander.get(), + this->GetLocalGenerator(), std::move(scanCommands)); - // Write the rule for preprocessing file of the given language. - ppRule.Comment = cmStrCat("Rule for preprocessing ", lang, " files."); - ppRule.Description = cmStrCat("Building ", lang, " preprocessed $out"); + scanRule.Comment = + cmStrCat("Rule for generating ", lang, " dependencies."); + scanRule.Description = cmStrCat("Building ", lang, " preprocessed $out"); - this->GetGlobalGenerator()->AddRule(ppRule); + this->GetGlobalGenerator()->AddRule(scanRule); } if (!compilePPWithDefines) { @@ -684,12 +679,14 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang, // Rule to scan dependencies of sources that do not need preprocessing. { - const auto scanCommand = - GetScanCommand(cmakeCmd, tdi, lang, "$in", "$out"); + std::string const& scanRuleName = this->LanguageScanRule(lang, config); + std::vector scanCommands; + scanCommands.emplace_back( + GetScanCommand(cmakeCmd, tdi, lang, "$in", "$out")); auto scanRule = GetScanRule( - this->LanguageScanRule(lang, config), vars, "", flags, launcher, - rulePlaceholderExpander.get(), scanCommand, this->GetLocalGenerator()); + scanRuleName, vars, "", flags, rulePlaceholderExpander.get(), + this->GetLocalGenerator(), std::move(scanCommands)); // Write the rule for generating dependencies for the given language. scanRule.Comment = cmStrCat("Rule for generating ", lang, -- cgit v0.12 From 33a8e0bb093f33fb0c206c820fe3b3945cae5a78 Mon Sep 17 00:00:00 2001 From: Brad King Date: Fri, 6 Nov 2020 11:26:42 -0500 Subject: cmNinjaTargetGenerator: Simplify scan rule depfile selection The depfile can always be the first output of the build statement with a `.d` suffix added. This approach easily avoids conflicts. --- Source/cmNinjaTargetGenerator.cxx | 62 ++++++++++++++++++++------------------- 1 file changed, 32 insertions(+), 30 deletions(-) diff --git a/Source/cmNinjaTargetGenerator.cxx b/Source/cmNinjaTargetGenerator.cxx index 49d2ae2..2b63a09 100644 --- a/Source/cmNinjaTargetGenerator.cxx +++ b/Source/cmNinjaTargetGenerator.cxx @@ -1064,13 +1064,12 @@ cmNinjaBuild GetScanBuildStatement(const std::string& ruleName, const std::string& ppFileName, bool compilePP, bool compilePPWithDefines, cmNinjaBuild& objBuild, cmNinjaVars& vars, - const std::string& depFileName, - const std::string& objectFileName) + const std::string& objectFileName, + cmLocalGenerator* lg) { cmNinjaBuild scanBuild(ruleName); if (!ppFileName.empty()) { - scanBuild.Outputs.push_back(ppFileName); scanBuild.RspFile = cmStrCat(ppFileName, ".rsp"); } else { scanBuild.RspFile = "$out.rsp"; @@ -1109,26 +1108,32 @@ cmNinjaBuild GetScanBuildStatement(const std::string& ruleName, // directive. scanBuild.Variables["INCLUDES"] = vars["INCLUDES"]; - // Explicit preprocessing always uses a depfile. - scanBuild.Variables["DEP_FILE"] = depFileName; - if (compilePP) { - // The actual compilation does not need a depfile because it - // depends on the already-preprocessed source. - vars.erase("DEP_FILE"); - } - // Tell dependency scanner the object file that will result from // compiling the source. scanBuild.Variables["OBJ_FILE"] = objectFileName; // Tell dependency scanner where to store dyndep intermediate results. - std::string const ddiFile = cmStrCat(objectFileName, ".ddi"); - if (ppFileName.empty()) { - scanBuild.Outputs.push_back(ddiFile); - } else { - scanBuild.Variables["DYNDEP_INTERMEDIATE_FILE"] = ddiFile; + std::string const& ddiFile = cmStrCat(objectFileName, ".ddi"); + scanBuild.Variables["DYNDEP_INTERMEDIATE_FILE"] = ddiFile; + + // Outputs of the scan/preprocessor build statement. + if (!ppFileName.empty()) { + scanBuild.Outputs.push_back(ppFileName); scanBuild.ImplicitOuts.push_back(ddiFile); + } else { + scanBuild.Outputs.push_back(ddiFile); } + + // Scanning always uses a depfile for preprocessor dependencies. + std::string const& depFileName = cmStrCat(scanBuild.Outputs.front(), ".d"); + scanBuild.Variables["DEP_FILE"] = + lg->ConvertToOutputFormat(depFileName, cmOutputConverter::SHELL); + if (compilePP) { + // The actual compilation does not need a depfile because it + // depends on the already-preprocessed source. + vars.erase("DEP_FILE"); + } + return scanBuild; } } @@ -1289,22 +1294,19 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatement( bool const compilePPWithDefines = compilePP && this->CompileWithDefines(language); - std::string const ppFileName = compilePP - ? this->ConvertToNinjaPath(this->GetPreprocessedFilePath(source, config)) - : ""; - - std::string const buildName = compilePP - ? this->LanguagePreprocessAndScanRule(language, config) - : this->LanguageScanRule(language, config); - - const auto depExtension = compilePP ? ".pp.d" : ".d"; - const std::string depFileName = - this->GetLocalGenerator()->ConvertToOutputFormat( - cmStrCat(objectFileName, depExtension), cmOutputConverter::SHELL); + std::string scanRuleName; + std::string ppFileName; + if (compilePP) { + scanRuleName = this->LanguagePreprocessAndScanRule(language, config); + ppFileName = this->ConvertToNinjaPath( + this->GetPreprocessedFilePath(source, config)); + } else { + scanRuleName = this->LanguageScanRule(language, config); + } cmNinjaBuild ppBuild = GetScanBuildStatement( - buildName, ppFileName, compilePP, compilePPWithDefines, objBuild, vars, - depFileName, objectFileName); + scanRuleName, ppFileName, compilePP, compilePPWithDefines, objBuild, + vars, objectFileName, this->LocalGenerator); if (compilePP) { // In case compilation requires flags that are incompatible with -- cgit v0.12 From 941be1d3568506351228fcf0627e6ce5fbc92da8 Mon Sep 17 00:00:00 2001 From: Brad King Date: Fri, 6 Nov 2020 11:29:58 -0500 Subject: cmNinjaTargetGenerator: Clarify variable names for preprocessing conditions What is important about code paths for Fortran's explicit preprocessing is that the compilation step following it does not do preprocessing. --- Source/cmNinjaTargetGenerator.cxx | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/Source/cmNinjaTargetGenerator.cxx b/Source/cmNinjaTargetGenerator.cxx index 2b63a09..f2bec8c 100644 --- a/Source/cmNinjaTargetGenerator.cxx +++ b/Source/cmNinjaTargetGenerator.cxx @@ -610,10 +610,7 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang, // For some cases we scan to dynamically discover dependencies. bool const needDyndep = this->NeedDyndep(lang); - // For some cases we do an explicit preprocessor invocation. - bool const explicitPP = this->NeedExplicitPreprocessing(lang); - bool const compilePPWithDefines = - explicitPP && this->CompileWithDefines(lang); + bool const compilationPreprocesses = !this->NeedExplicitPreprocessing(lang); std::string flags = "$FLAGS"; @@ -672,13 +669,14 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang, this->GetGlobalGenerator()->AddRule(scanRule); } - if (!compilePPWithDefines) { - // Remove preprocessor definitions from compilation step - vars.Defines = ""; - } - - // Rule to scan dependencies of sources that do not need preprocessing. { + // Compilation will not preprocess, so it does not need the defines + // unless the compiler wants them for some other purpose. + if (!this->CompileWithDefines(lang)) { + vars.Defines = ""; + } + + // Rule to scan dependencies of sources that do not need preprocessing. std::string const& scanRuleName = this->LanguageScanRule(lang, config); std::vector scanCommands; scanCommands.emplace_back( @@ -735,8 +733,8 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang, // Tell ninja dependency format so all deps can be loaded into a database std::string cldeps; - if (explicitPP) { - // The explicit preprocessing step will handle dependency scanning. + if (!compilationPreprocesses) { + // The compiler will not do preprocessing, so it has no such dependencies. } else if (this->NeedDepTypeMSVC(lang)) { rule.DepType = "msvc"; rule.DepFile.clear(); @@ -1271,13 +1269,12 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatement( sourceFileName, objBuild.OrderOnlyDeps); } - // For some cases we need to generate a ninja dyndep file. + // For some cases we scan to dynamically discover dependencies. bool const needDyndep = this->NeedDyndep(language); + bool const compilationPreprocesses = + !this->NeedExplicitPreprocessing(language); if (needDyndep) { - // For some cases we do an explicit preprocessor invocation. - bool const explicitPP = this->NeedExplicitPreprocessing(language); - // If source/target has preprocessing turned off, we still need to // generate an explicit dependency step const auto srcpp = source->GetSafeProperty("Fortran_PREPROCESS"); @@ -1289,7 +1286,7 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatement( preprocess = cmOutputConverter::GetFortranPreprocess(tgtpp); } - bool const compilePP = explicitPP && + bool const compilePP = !compilationPreprocesses && (preprocess != cmOutputConverter::FortranPreprocess::NotNeeded); bool const compilePPWithDefines = compilePP && this->CompileWithDefines(language); -- cgit v0.12