diff --git a/Help/command/add_custom_rule.rst b/Help/command/add_custom_rule.rst index f78b6e3837..b249b88848 100644 --- a/Help/command/add_custom_rule.rst +++ b/Help/command/add_custom_rule.rst @@ -68,6 +68,10 @@ Generating Files The instantiation of the patterns are done in the context of the directory where the file set was created. + .. versionchanged:: 4.6 + The expansion of patterns is done recursively at generation time so the + value of a pattern can refer any other pattern. + These patterns cannot be changed by the functions specified by the ``CONFIGURATOR`` option. @@ -202,7 +206,7 @@ Generating Files used to enrich the list of patterns. The expected value is a :ref:`semicolon-separated list ` of items having the syntax ``PATTERN=VALUE`` or ``PATTERN=``. More precisely, each item - must match the regular expression ``(^[A-Z][A-Z0-9_]+)=(.*)$``. Items + must match the regular expression ``(^[A-Z][A-Z0-9_]*)=(.*)$``. Items which do not match this regular expression will be ignored. ``FOR_SOURCE`` @@ -219,7 +223,7 @@ Generating Files used to enrich the list of patterns. The expected value is a :ref:`semicolon-separated list ` of items having the syntax ``PATTERN=VALUE`` or ``PATTERN=``. More precisely, each item - must match the regular expression ``(^[A-Z][A-Z0-9_]+)=(.*)$``. Items + must match the regular expression ``(^[A-Z][A-Z0-9_]*)=(.*)$``. Items which do not match this regular expression will be ignored. .. note:: diff --git a/Help/prop_fs/RULE_PATTERNS.rst b/Help/prop_fs/RULE_PATTERNS.rst index 815c22495e..ae36139759 100644 --- a/Help/prop_fs/RULE_PATTERNS.rst +++ b/Help/prop_fs/RULE_PATTERNS.rst @@ -8,15 +8,14 @@ Patterns specification for instantiating a rule. The ``RULE_PATTERNS`` property may be set to a :ref:`semicolon-separated list ` of patterns using the syntax ``PATTERN=VALUE`` or ``PATTERN=``. More precisely, each item must match -the regular expression ``(^[A-Z][A-Z0-9_]+)=(.*)$``. +the regular expression ``(^[A-Z][A-Z0-9_]*)=(.*)$``. CMake will automatically drop any patterns which do not match against this regular expression. -The list is ordered, so a pattern can use in its definition a previously -defined pattern. In the following example, -``OUTPUT_DIR=/some/path;OUTPUT_FILE=/my_file``, when the pattern -```` is expanded, the pattern ```` is already known. +.. versionchanged:: 4.6 + The expansion of patterns is done recursively at generation time so the value + of a pattern can refer any other pattern. Related properties: diff --git a/Help/prop_sf/RULE_PATTERNS.rst b/Help/prop_sf/RULE_PATTERNS.rst index b5d9e6a49c..827fe21ed7 100644 --- a/Help/prop_sf/RULE_PATTERNS.rst +++ b/Help/prop_sf/RULE_PATTERNS.rst @@ -13,10 +13,9 @@ the regular expression ``(^[A-Z][A-Z0-9_]+)=(.*)$``. CMake will automatically drop any patterns which do not match against this regular expression. -The list is ordered, so a pattern can use in its definition a previously -defined pattern. In the following example, -``OUTPUT_DIR=/some/path;OUTPUT_FILE=/my_file``, when the pattern -```` is expanded, the pattern ```` is already known. +.. versionchanged:: 4.6 + The expansion of patterns is done recursively at generation time so the value + of a pattern can refer any other pattern. Related properties: diff --git a/Source/CMakeLists.txt b/Source/CMakeLists.txt index 34368d97e1..fb278d4e2d 100644 --- a/Source/CMakeLists.txt +++ b/Source/CMakeLists.txt @@ -163,6 +163,8 @@ add_library( cmCryptoHash.h cmCurl.cxx cmCurl.h + cmRuleHelper.cxx + cmRuleHelper.h cmRule.cxx cmRule.h cmCustomRule.cxx diff --git a/Source/cmGeneratorRule.cxx b/Source/cmGeneratorRule.cxx index d7db4d37c5..9597042755 100644 --- a/Source/cmGeneratorRule.cxx +++ b/Source/cmGeneratorRule.cxx @@ -5,6 +5,8 @@ #include #include +#include +#include #include #include @@ -23,12 +25,22 @@ #include "cmList.h" #include "cmListFileCache.h" #include "cmMakefile.h" +#include "cmMessageType.h" #include "cmSourceFile.h" #include "cmStringAlgorithms.h" #include "cmSystemTools.h" #include "cmTarget.h" namespace { +class rule_pattern_error : public std::runtime_error +{ +public: + rule_pattern_error(std::string const& msg) + : std::runtime_error(msg) + { + } +}; + std::vector ReservedPatterns{ "RULE"_s, "TARGET"_s, "FILE_SET"_s, @@ -47,101 +59,86 @@ std::vector ReservedPatterns{ "RULE"_s, "COMPILE_DEFINITIONS"_s }; } -std::string cmGeneratorRule::RulePlaceholderExpander::ExpandVariable( - std::string const& variable) -{ - if (cm::contains(this->Values, variable)) { - return this->Values[variable]; - } - // If there is no variable defined, mark unresolved variable by '{' and '}' - return cmStrCat('{', variable, '}'); -} - -cmGeneratorRule::RulePlaceholderExpander::VariableMap const - cmGeneratorRule::DefaultProperties{ - { "INCLUDE_DIRECTORIES", - "$,TARGET:,INCLUDE_" - "DIRECTORIES>;$,TARGET_DIRECTORY:," - "INCLUDE_DIRECTORIES>;$,INCLUDE_DIRECTORIES>," - "EXCLUDE,^$>" }, - { "COMPILE_OPTIONS", - "$,COMPILE_OPTIONS>;$," - "TARGET_DIRECTORY:,COMPILE_OPTIONS>;$,TARGET:,COMPILE_OPTIONS>,EXCLUDE,^$>" }, - { "COMPILE_DEFINITIONS", - "$,COMPILE_DEFINITIONS>;$," - "TARGET_DIRECTORY:,COMPILE_DEFINITIONS>;$,TARGET:,COMPILE_DEFINITIONS>,EXCLUDE,^$>" } - }; +cm::RulePatternSet const cmGeneratorRule::DefaultProperties{ + { "INCLUDE_DIRECTORIES", + "$,TARGET:,INCLUDE_" + "DIRECTORIES>;$,TARGET_DIRECTORY:," + "INCLUDE_DIRECTORIES>;$,INCLUDE_DIRECTORIES>," + "EXCLUDE,^$>" }, + { "COMPILE_OPTIONS", + "$,COMPILE_OPTIONS>;$," + "TARGET_DIRECTORY:,COMPILE_OPTIONS>;$,TARGET:,COMPILE_OPTIONS>,EXCLUDE,^$>" }, + { "COMPILE_DEFINITIONS", + "$,COMPILE_DEFINITIONS>;$," + "TARGET_DIRECTORY:,COMPILE_DEFINITIONS>;$,TARGET:,COMPILE_DEFINITIONS>,EXCLUDE,^$>" } +}; cmGeneratorRule::cmGeneratorRule(cmRule const* rule, cmTarget const* target, cmFileSet const* fileSet, cmFileSet const* outputFileSet, cmSourceFile const* source, - cmRule::PatternSet const& patterns) + cm::RulePatternSet const& patterns) : Rule(rule) , Target(target) , FileSet(fileSet) , OutputFileSet(outputFileSet) , Source(source) , Makefile(*fileSet->GetMakefile()) + , RuleExpander(this->GetMakefile(), this->Rule->GetName(), this->Patterns) { this->Name = cmStrCat(rule->GetName(), '_', std::hash{}(cmStrCat( rule->GetName(), '-', target->GetName(), '-', fileSet->GetName(), '-', source->GetFullPath()))); - auto& values = this->RuleExpander.Values; - cmCMakePath file = cmCMakePath{ this->Source->GetFullPath() }.Normal(); cmCMakePath sourceDir = file.IsAbsolute() ? file.GetParentPath() : this->GetMakefile().GetCurrentSourceDirectory(); /* clang-format off */ - values.insert({ "RULE", this->GetName() }); - values.insert({ "TARGET", this->Target->GetName() }); - values.insert({ "FILE_SET", this->FileSet->GetName() }); - values.insert({ "SOURCE_DIR", this->GetMakefile().GetHomeDirectory() }); - values.insert({ "BINARY_DIR", this->GetMakefile().GetHomeOutputDirectory() }); - values.insert({ "CURRENT_SOURCE_DIR", this->GetMakefile().GetCurrentSourceDirectory() }); - values.insert({ "CURRENT_BINARY_DIR", this->GetMakefile().GetCurrentBinaryDirectory() }); - values.insert({ "OUTPUT_CURRENT_SOURCE_DIR", outputFileSet->GetMakefile()->GetCurrentSourceDirectory() }); - values.insert({ "OUTPUT_CURRENT_BINARY_DIR", outputFileSet->GetMakefile()->GetCurrentBinaryDirectory() }); - values.insert({ "SOURCE", file.GenericString() }); - values.insert({ "INPUT_DIR", file.GetParentPath().GenericString() }); - values.insert({ "FILE_NAME", file.GetFileName().GenericString() }); - values.insert({ "BASE_NAME", file.GetFileName().RemoveWideExtension().GenericString() }); + this->Patterns.insert({ "RULE", this->GetName() }); + this->Patterns.insert({ "TARGET", this->Target->GetName() }); + this->Patterns.insert({ "FILE_SET", this->FileSet->GetName() }); + this->Patterns.insert({ "SOURCE_DIR", this->GetMakefile().GetHomeDirectory() }); + this->Patterns.insert({ "BINARY_DIR", this->GetMakefile().GetHomeOutputDirectory() }); + this->Patterns.insert({ "CURRENT_SOURCE_DIR", this->GetMakefile().GetCurrentSourceDirectory() }); + this->Patterns.insert({ "CURRENT_BINARY_DIR", this->GetMakefile().GetCurrentBinaryDirectory() }); + this->Patterns.insert({ "OUTPUT_CURRENT_SOURCE_DIR", outputFileSet->GetMakefile()->GetCurrentSourceDirectory() }); + this->Patterns.insert({ "OUTPUT_CURRENT_BINARY_DIR", outputFileSet->GetMakefile()->GetCurrentBinaryDirectory() }); + this->Patterns.insert({ "SOURCE", file.GenericString() }); + this->Patterns.insert({ "INPUT_DIR", file.GetParentPath().GenericString() }); + this->Patterns.insert({ "FILE_NAME", file.GetFileName().GenericString() }); + this->Patterns.insert({ "BASE_NAME", file.GetFileName().RemoveWideExtension().GenericString() }); /* clang-format on */ // instantiate default properties pattern for (auto const& item : DefaultProperties) { - values.insert( + this->Patterns.insert( { item.first, this->RuleExpander.ExpandVariables(item.second) }); } this->UpdateRuleExpander(patterns); } -void cmGeneratorRule::UpdateRuleExpander(cmRule::PatternSet const& patterns) +void cmGeneratorRule::UpdateRuleExpander(cm::RulePatternSet const& patterns) { if (patterns.empty()) { return; } - auto& values = this->RuleExpander.Values; - - for (cmRule::Pattern const& pattern : patterns) { - if (cm::contains(ReservedPatterns, pattern.Name)) { + for (auto const& pattern : patterns) { + if (cm::contains(ReservedPatterns, pattern.first)) { // overwriting a reserved pattern is not allowed continue; } - std::string data{ pattern.Value }; - values[pattern.Name] = this->RuleExpander.ExpandVariables(data); + this->Patterns[pattern.first] = pattern.second; } } @@ -182,7 +179,15 @@ cmValue cmGeneratorRule::GetProperty(std::string const& property) const std::unique_ptr cmGeneratorRule::CreateCustomCommand() const { auto expandVariables = [this](std::string const& item) -> std::string { - return this->RuleExpander.ExpandVariables(item); + auto result = this->RuleExpander.ExpandPatterns(item); + if (this->RuleExpander.InError) { + throw rule_pattern_error{ cmStrCat( + "Pattern expression\n ", item, "\nfrom rule \"", + this->GetRule()->GetName(), "\", used by the fileset \"", + this->FileSet->GetName(), "\" of the target \"", + this->Target->GetName(), "\", did not expand correctly.") }; + } + return result; }; auto expandVector = @@ -225,87 +230,95 @@ std::unique_ptr cmGeneratorRule::CreateCustomCommand() const return result; }; - auto cc = cm::make_unique(); + try { + auto cc = cm::make_unique(); - cmCustomCommandLines commandLines; - for (cmRule::CustomCommand const& line : this->Rule->GetCommands()) { - cmCustomCommandLine command; - command.reserve(line.size()); - std::transform(line.begin(), line.end(), std::back_inserter(command), - expandVariables); - commandLines.push_back(command); + cmCustomCommandLines commandLines; + for (cmRule::CustomCommand const& line : this->Rule->GetCommands()) { + cmCustomCommandLine command; + command.reserve(line.size()); + std::transform(line.begin(), line.end(), std::back_inserter(command), + expandVariables); + commandLines.push_back(command); + } + cc->SetCommandLines(std::move(commandLines)); + + cc->SetOutputs(expandPaths(this->GetMakefile().GetCurrentBinaryDirectory(), + this->Rule->GetOutputs())); + + if (!this->Rule->GetByproducts().empty()) { + cc->SetByproducts(expandVector(this->Rule->GetByproducts())); + } + + if (!this->Rule->GetDepfile().empty()) { + // store normalized path as property + this->Properties.SetProperty( + "DEPFILE", + expandPath(this->GetMakefile().GetCurrentBinaryDirectory(), + this->Rule->GetDepfile())); + cc->SetDepfile(expandVariables(this->Rule->GetDepfile())); + } + std::vector depends{ 1, this->Source->GetFullPath() }; + if (!this->Rule->GetDepends().empty()) { + depends.insert(depends.end(), this->Rule->GetDepends().begin(), + this->Rule->GetDepends().end()); + } + cc->SetDepends(expandVector(depends)); + if (cmValue deps_explicit = + this->Rule->GetProperty("DEPENDS_EXPLICIT_ONLY")) { + cc->SetDependsExplicitOnly(deps_explicit.IsOn()); + } else { + cc->SetDependsExplicitOnly(this->GetMakefile().IsOn( + "CMAKE_ADD_CUSTOM_COMMAND_DEPENDS_EXPLICIT_ONLY")); + } + + if (cmValue wd = this->Rule->GetProperty("WORKING_DIRECTORY")) { + // store normalized path as property + this->Properties.SetProperty( + "WORKING_DIRECTORY", + expandPath(this->GetMakefile().GetCurrentBinaryDirectory(), *wd)); + cc->SetWorkingDirectory(expandVariables(*wd)); + } + + if (this->Rule->GetProperty("USES_TERMINAL").IsOn() && + !this->Rule->GetProperty("JOB_POOL_COMPILE")->empty()) { + this->GetMakefile().IssueDiagnostic( + cmDiagnostics::CMD_AUTHOR, + cmStrCat("RULE \"", this->GetName(), "\": JOB_POOL \"", + expandVariables(this->Rule->GetProperty("JOB_POOL_COMPILE")), + "\" is shadowed by USES_TERMINAL.")); + } + if (!this->Rule->GetProperty("JOB_POOL_COMPILE")->empty()) { + cc->SetJobPool( + expandVariables(*this->Rule->GetProperty("JOB_POOL_COMPILE"))); + } else if (this->Rule->GetProperty("USES_TERMINAL").IsOn()) { + cc->SetUsesTerminal(true); + } + + cc->SetJobserverAware(this->Rule->GetProperty("JOB_SERVER_AWARE").IsOn()); + + cc->SetEscapeOldStyle(!this->Rule->GetProperty("VERBATIM").IsOn()); + cc->SetCommandExpandLists( + this->Rule->GetProperty("COMMAND_EXPAND_LISTS").IsOn()); + + if (cmValue comment = this->Rule->GetProperty("COMMENT")) { + cc->SetComment(expandVariables(*comment)); + } + + return cc; + } catch (rule_pattern_error const& error) { + this->GetMakefile().IssueMessage(MessageType::FATAL_ERROR, error.what()); + return nullptr; } - cc->SetCommandLines(std::move(commandLines)); - - cc->SetOutputs(expandPaths(this->GetMakefile().GetCurrentBinaryDirectory(), - this->Rule->GetOutputs())); - - if (!this->Rule->GetByproducts().empty()) { - cc->SetByproducts(expandVector(this->Rule->GetByproducts())); - } - - if (!this->Rule->GetDepfile().empty()) { - // store normalized path as property - this->Properties.SetProperty( - "DEPFILE", - expandPath(this->GetMakefile().GetCurrentBinaryDirectory(), - this->Rule->GetDepfile())); - cc->SetDepfile(expandVariables(this->Rule->GetDepfile())); - } - std::vector depends{ 1, this->Source->GetFullPath() }; - if (!this->Rule->GetDepends().empty()) { - depends.insert(depends.end(), this->Rule->GetDepends().begin(), - this->Rule->GetDepends().end()); - } - cc->SetDepends(expandVector(depends)); - if (cmValue deps_explicit = - this->Rule->GetProperty("DEPENDS_EXPLICIT_ONLY")) { - cc->SetDependsExplicitOnly(deps_explicit.IsOn()); - } else { - cc->SetDependsExplicitOnly(this->GetMakefile().IsOn( - "CMAKE_ADD_CUSTOM_COMMAND_DEPENDS_EXPLICIT_ONLY")); - } - - if (cmValue wd = this->Rule->GetProperty("WORKING_DIRECTORY")) { - // store normalized path as property - this->Properties.SetProperty( - "WORKING_DIRECTORY", - expandPath(this->GetMakefile().GetCurrentBinaryDirectory(), *wd)); - cc->SetWorkingDirectory(expandVariables(*wd)); - } - - if (this->Rule->GetProperty("USES_TERMINAL").IsOn() && - !this->Rule->GetProperty("JOB_POOL_COMPILE")->empty()) { - this->GetMakefile().IssueDiagnostic( - cmDiagnostics::CMD_AUTHOR, - cmStrCat("RULE \"", this->GetName(), "\": JOB_POOL \"", - expandVariables(this->Rule->GetProperty("JOB_POOL_COMPILE")), - "\" is shadowed by USES_TERMINAL.")); - } - if (!this->Rule->GetProperty("JOB_POOL_COMPILE")->empty()) { - cc->SetJobPool( - expandVariables(*this->Rule->GetProperty("JOB_POOL_COMPILE"))); - } else if (this->Rule->GetProperty("USES_TERMINAL").IsOn()) { - cc->SetUsesTerminal(true); - } - - cc->SetJobserverAware(this->Rule->GetProperty("JOB_SERVER_AWARE").IsOn()); - - cc->SetEscapeOldStyle(!this->Rule->GetProperty("VERBATIM").IsOn()); - cc->SetCommandExpandLists( - this->Rule->GetProperty("COMMAND_EXPAND_LISTS").IsOn()); - - if (cmValue comment = this->Rule->GetProperty("COMMENT")) { - cc->SetComment(expandVariables(*comment)); - } - - return cc; } std::unique_ptr cmGeneratorRule::Generate( cmFileSet* outFileSet) const { auto cc = this->CreateCustomCommand(); + if (!cc) { + return nullptr; + } // populate file set with outputs from custom command for (auto const& output : cc->GetOutputs()) { diff --git a/Source/cmGeneratorRule.h b/Source/cmGeneratorRule.h index 8ffc9e9e3d..9b30e8cb26 100644 --- a/Source/cmGeneratorRule.h +++ b/Source/cmGeneratorRule.h @@ -6,11 +6,10 @@ #include #include -#include -#include "cmPlaceholderExpander.h" #include "cmPropertyMap.h" #include "cmRule.h" +#include "cmRuleHelper.h" #include "cmValue.h" class cmMakefile; @@ -25,7 +24,7 @@ public: cmGeneratorRule(cmRule const* rule, cmTarget const* target, cmFileSet const* fileSet, cmFileSet const* outputFileSet, cmSourceFile const* source, - cmRule::PatternSet const& patterns); + cm::RulePatternSet const& patterns); cmGeneratorRule(cmGeneratorRule const&) = delete; cmGeneratorRule& operator=(cmGeneratorRule const&) = delete; @@ -45,7 +44,7 @@ public: std::unique_ptr Generate(cmFileSet* outFileSet) const; private: - void UpdateRuleExpander(cmRule::PatternSet const& patterns); + void UpdateRuleExpander(cm::RulePatternSet const& patterns); std::unique_ptr CreateCustomCommand() const; @@ -58,24 +57,8 @@ private: cmMakefile& Makefile; mutable cmPropertyMap Properties; - class RulePlaceholderExpander : public cmPlaceholderExpander - { - public: - using VariableMap = std::unordered_map; - VariableMap Values; + static cm::RulePatternSet const DefaultProperties; - std::string ExpandVariables(std::string const& string) - { - std::string value{ string }; - return cmPlaceholderExpander::ExpandVariables( - value, cmPlaceholderExpander::HandleGenex::Yes); - } - - private: - std::string ExpandVariable(std::string const& variable) override; - }; - - static RulePlaceholderExpander::VariableMap const DefaultProperties; - - mutable RulePlaceholderExpander RuleExpander; + cm::RulePatternSet Patterns; + mutable cm::RulePatternPlaceholderExpander RuleExpander; }; diff --git a/Source/cmGlobalGenerator.cxx b/Source/cmGlobalGenerator.cxx index a8f3747cdd..bde11d3888 100644 --- a/Source/cmGlobalGenerator.cxx +++ b/Source/cmGlobalGenerator.cxx @@ -64,6 +64,7 @@ #include "cmPolicies.h" #include "cmRange.h" #include "cmRule.h" +#include "cmRuleHelper.h" #include "cmSbomArguments.h" #include "cmSourceFile.h" #include "cmState.h" @@ -3883,7 +3884,7 @@ void cmGlobalGenerator::ConfigureRules() continue; } - cmRule::PatternSet fileSetPatterns; + cm::RulePatternSet fileSetPatterns; if (!rule->Instantiate(&target, fileSet, outFileSet, fileSetPatterns)) { continue; @@ -3895,7 +3896,7 @@ void cmGlobalGenerator::ConfigureRules() cmSourceFile* source = mf->GetOrCreateSource(file); source->ResolveFullPath(); - cmRule::PatternSet sourcePatterns{ fileSetPatterns }; + cm::RulePatternSet sourcePatterns{ fileSetPatterns }; if (!rule->Instantiate(&target, fileSet, outFileSet, source, sourcePatterns)) { continue; @@ -3907,9 +3908,11 @@ void cmGlobalGenerator::ConfigureRules() auto genRule = cm::make_unique( rule, &target, fileSet, outFileSet, source, sourcePatterns); auto cc = genRule->Generate(outFileSet); - lg.AddGeneratorRule(std::move(genRule)); - detail::AddCustomCommandToOutput( - lg, cmCommandOrigin::Generator, std::move(cc), false); + if (cc) { + lg.AddGeneratorRule(std::move(genRule)); + detail::AddCustomCommandToOutput( + lg, cmCommandOrigin::Generator, std::move(cc), false); + } }); } } diff --git a/Source/cmRule.cxx b/Source/cmRule.cxx index 5dce91fd11..d41d30ff27 100644 --- a/Source/cmRule.cxx +++ b/Source/cmRule.cxx @@ -22,8 +22,8 @@ #include "cmListFileCache.h" #include "cmMakefile.h" #include "cmMessageType.h" -#include "cmPlaceholderExpander.h" #include "cmRange.h" +#include "cmRuleHelper.h" #include "cmSourceFile.h" #include "cmStringAlgorithms.h" #include "cmTarget.h" @@ -153,26 +153,6 @@ bool IsSettableProperty(cm::string_view prop, cmRule const* rule) return true; } - -class FileSetNamePlaceholderExpander : public cmPlaceholderExpander -{ -public: - using VariableMap = std::unordered_map; - VariableMap Values; - - bool InError = false; - -private: - std::string ExpandVariable(std::string const& variable) override - { - if (cm::contains(this->Values, variable)) { - return this->Values[variable]; - } - this->InError = true; - // If there is no variable defined, mark unresolved variable by '<' and '>' - return cmStrCat('<', variable, '>'); - } -}; } cmRule::cmRule(cmMakefile& makefile, std::string name, cm::RuleScope scope) @@ -242,27 +222,31 @@ cmFileSet* cmRule::GetOutputFileSet(cmTarget* target, cmFileSet const* fileSet) const { cmList outputFileSet{ this->GetProperty(std::string{ OUTPUT_FILE_SET }) }; - FileSetNamePlaceholderExpander expander; - expander.Values["RULE"] = this->GetName(); - expander.Values["TARGET"] = target->GetName(); - expander.Values["FILE_SET"] = fileSet->GetName(); + cm::RulePatternSet patterns{ + { "RULE", this->GetName() }, + { "TARGET", target->GetName() }, + { "FILE_SET", fileSet->GetName() }, + }; - std::string fsName{ outputFileSet[0] }; - expander.ExpandVariables(fsName); + cm::RulePatternPlaceholderExpander expander{ this->GetMakefile(), + this->GetName(), patterns }; + + std::string const& fsName = outputFileSet[0]; + std::string outputName = expander.ExpandPatterns(fsName); if (expander.InError) { this->GetMakefile().IssueMessage( MessageType::FATAL_ERROR, - cmStrCat("Output File Set name, for rule \"", this->GetName(), - "\", did not expand correctly:\n \"", fsName, "\".")); + cmStrCat("Output File Set name, for the rule \"", this->GetName(), + "\", did not expand correctly:\n \"", outputName, "\".")); return nullptr; } - auto result = target->GetOrCreateFileSet(fsName, outputFileSet[1], + auto result = target->GetOrCreateFileSet(outputName, outputFileSet[1], fileSet->GetVisibility()); if (!result.second && result.first->GetType() != outputFileSet[1]) { this->GetMakefile().IssueMessage( MessageType::FATAL_ERROR, - cmStrCat("The output file set \"", fsName, "\", for the target \"", + cmStrCat("The output file set \"", outputName, "\", for the target \"", target->GetName(), "\", has the type \"", result.first->GetType(), "\" instead of \"", outputFileSet[1], "\", as specified by the rule \"", this->GetName(), "\".")); @@ -272,16 +256,23 @@ cmFileSet* cmRule::GetOutputFileSet(cmTarget* target, if (result.first->GetDirectoryEntries().empty()) { if (cmValue baseDirs = this->GetProperty(std::string{ OUTPUT_FILE_SET_BASE_DIRS })) { - expander.Values["CURRENT_SOURCE_DIR"] = + patterns["CURRENT_SOURCE_DIR"] = fileSet->GetMakefile()->GetCurrentSourceDirectory(); - expander.Values["CURRENT_BINARY_DIR"] = + patterns["CURRENT_BINARY_DIR"] = fileSet->GetMakefile()->GetCurrentBinaryDirectory(); - expander.Values["OUTPUT_CURRENT_SOURCE_DIR"] = + patterns["OUTPUT_CURRENT_SOURCE_DIR"] = result.first->GetMakefile()->GetCurrentSourceDirectory(); - expander.Values["OUTPUT_CURRENT_BINARY_DIR"] = + patterns["OUTPUT_CURRENT_BINARY_DIR"] = result.first->GetMakefile()->GetCurrentBinaryDirectory(); - std::string dirs{ baseDirs }; - expander.ExpandVariables(dirs); + + std::string dirs = expander.ExpandPatterns(*baseDirs); + if (expander.InError) { + this->GetMakefile().IssueMessage( + MessageType::FATAL_ERROR, + cmStrCat("Base directories, for the rule \"", this->GetName(), + "\", did not expand correctly.")); + return nullptr; + } result.first->AddDirectoryEntry(BT{ dirs }); } else { // Add default base directory if none are specified @@ -520,33 +511,30 @@ void cmRule::CheckProperty(std::string const& prop, cmMakefile& context) const } namespace { -cmsys::RegularExpression PatternRegex{ "(^[A-Z][A-Z0-9_]+)=(.*)$" }; +cmsys::RegularExpression PatternRegex{ "(^[A-Z][A-Z0-9_]*)=(.*)$" }; -void UpdatePatterns(cmRule::PatternSet& patterns, cmList const& newPatterns) +void UpdatePatterns(cm::RulePatternSet& patterns, cmList const& newPatterns) { for (auto const& pattern : newPatterns) { if (PatternRegex.find(pattern)) { - auto it = std::find_if(patterns.begin(), patterns.end(), - [](cmRule::Pattern const& item) { - return item.Name == PatternRegex.match(1); - }); - if (it == patterns.end()) { - patterns.emplace_back(PatternRegex.match(1), PatternRegex.match(2)); + if (cm::contains(patterns, PatternRegex.match(1))) { + patterns[PatternRegex.match(1)] = PatternRegex.match(2); } else { - it->Value = PatternRegex.match(2); + patterns.emplace(PatternRegex.match(1), PatternRegex.match(2)); } } } } bool ConfigureRule(cmRule::ConfiguratorSet const& configurators, - cmMakefile* mf, cmRule::PatternSet& patterns, + cmMakefile* mf, cm::RulePatternSet& patterns, std::string const& rule, std::string const& target, std::string const& fileSet, std::string const& outputFileSet, std::string const& source = {}) { cm::string_view patternsVariable{ "_CMAKE_RULE_PATTERNS"_s }; + cm::RulePatternPlaceholderExpander expander{ *mf, rule, patterns }; for (auto const& item : configurators) { // The validator command will be executed in an isolated scope. @@ -586,7 +574,7 @@ bool ConfigureRule(cmRule::ConfiguratorSet const& configurators, bool cmRule::Instantiate(cmTarget const* target, cmFileSet const* fileSet, cmFileSet const* outputFileSet, - PatternSet& patterns) const + cm::RulePatternSet& patterns) const { this->SwitchMode(); @@ -608,7 +596,7 @@ bool cmRule::Instantiate(cmTarget const* target, cmFileSet const* fileSet, bool cmRule::Instantiate(cmTarget const* target, cmFileSet const* fileSet, cmFileSet const* outputFileSet, cmSourceFile* source, - PatternSet& patterns) const + cm::RulePatternSet& patterns) const { // First, take patterns from _PATTERNS source file property, if any if (cmValue sfPatterns = diff --git a/Source/cmRule.h b/Source/cmRule.h index 06924cff04..06555c848c 100644 --- a/Source/cmRule.h +++ b/Source/cmRule.h @@ -14,6 +14,7 @@ #include "cmAlgorithms.h" #include "cmListFileCache.h" #include "cmPropertyMap.h" +#include "cmRuleHelper.h" #include "cmValue.h" class cmMakefile; @@ -127,23 +128,12 @@ public: bool InGeneration() const { return this->Generation; } - struct Pattern - { - Pattern(std::string name, std::string value) - : Name(std::move(name)) - , Value(std::move(value)) - { - } - - std::string Name; - std::string Value; - }; - using PatternSet = std::vector; bool Instantiate(cmTarget const* target, cmFileSet const* fileSet, - cmFileSet const* outputFileSet, PatternSet& patterns) const; + cmFileSet const* outputFileSet, + cm::RulePatternSet& patterns) const; bool Instantiate(cmTarget const* target, cmFileSet const* fileSet, cmFileSet const* outputFileSet, cmSourceFile* source, - PatternSet& patterns) const; + cm::RulePatternSet& patterns) const; protected: cmRule(cmMakefile& makefile, std::string name, cm::RuleScope scope); diff --git a/Source/cmRuleHelper.cxx b/Source/cmRuleHelper.cxx new file mode 100644 index 0000000000..d3fadd6c6a --- /dev/null +++ b/Source/cmRuleHelper.cxx @@ -0,0 +1,130 @@ +/* Distributed under the OSI-approved BSD 3-Clause License. See accompanying + file LICENSE.rst or https://cmake.org/licensing for details. */ + +#include "cmRuleHelper.h" + +#include + +#include + +#include "cmMakefile.h" +#include "cmMessageType.h" +#include "cmStringAlgorithms.h" + +namespace cm { +class RulePatternDAGChecker +{ +public: + RulePatternDAGChecker(std::string pattern, RulePatternDAGChecker* parent) + : Parent(parent) + , Top(parent ? parent->Top : this) + , Pattern(std::move(pattern)) + { + this->CheckResult = this->CheckGraph(); + } + + enum Result + { + DAG, + SELF_REFERENCE, + CYCLIC_REFERENCE + }; + + Result Check() const { return this->CheckResult; } + + void ReportError(cmMakefile& makefile, std ::string const& rule, + std::string const& expression) + { + if (this->CheckResult == DAG) { + return; + } + + RulePatternDAGChecker const* parent = this->Parent; + + if (parent && !parent->Parent) { + makefile.IssueMessage(MessageType::FATAL_ERROR, + cmStrCat("Error evaluating the pattern \"", + this->Pattern, + "\" in the expression:\n" + " ", + expression, + "\n" + "Self reference on rule \"", + rule, "\".\n")); + return; + } + + { + makefile.IssueMessage(MessageType::FATAL_ERROR, + cmStrCat("Error evaluating the pattern \"", + this->Pattern, + "\" in the expression:\n" + " ", + expression, + "\n" + "Dependency loop found.")); + } + } + +private: + Result CheckGraph() const + { + RulePatternDAGChecker const* parent = this->Parent; + while (parent) { + if (this->Pattern == parent->Pattern) { + return (parent == this->Parent) ? SELF_REFERENCE : CYCLIC_REFERENCE; + } + parent = parent->Parent; + } + return DAG; + } + + RulePatternDAGChecker const* const Parent; + RulePatternDAGChecker const* const Top; + std::string const Pattern; + Result CheckResult; +}; + +std::string RulePatternPlaceholderExpander::ExpandVariables( + std::string const& string) +{ + std::string value{ string }; + return cmPlaceholderExpander::ExpandVariables( + value, cmPlaceholderExpander::HandleGenex::Yes); +} + +std::string RulePatternPlaceholderExpander::ExpandVariable( + std::string const& variable) +{ + if (cm::contains(this->Patterns, variable)) { + RulePatternDAGChecker dagChecker{ + variable, this->DAGStack.empty() ? nullptr : this->DAGStack.top() + }; + + switch (dagChecker.Check()) { + case RulePatternDAGChecker::SELF_REFERENCE: + case RulePatternDAGChecker::CYCLIC_REFERENCE: { + dagChecker.ReportError(this->Makefile, this->Rule, this->Expression); + this->InError = true; + return cmStrCat('<', variable, '>'); + } + case RulePatternDAGChecker::DAG: + break; + } + + this->DAGStack.push(&dagChecker); + auto result = this->ExpandVariables(this->Patterns.at(variable)); + this->DAGStack.pop(); + + return result; + } + + this->InError = true; + this->Makefile.IssueMessage( + MessageType::FATAL_ERROR, + cmStrCat("The pattern \"", variable, "\" is not defined.")); + // If there is no variable defined, mark unresolved variable by '<' and + // '>' + return cmStrCat('<', variable, '>'); +} +} diff --git a/Source/cmRuleHelper.h b/Source/cmRuleHelper.h new file mode 100644 index 0000000000..5d0ecad7af --- /dev/null +++ b/Source/cmRuleHelper.h @@ -0,0 +1,49 @@ +/* Distributed under the OSI-approved BSD 3-Clause License. See accompanying + file LICENSE.rst or https://cmake.org/licensing for details. */ +#pragma once + +#include +#include +#include + +#include "cmPlaceholderExpander.h" + +class cmMakefile; + +namespace cm { +using RulePatternSet = std::unordered_map; + +class RulePatternDAGChecker; + +class RulePatternPlaceholderExpander : public cmPlaceholderExpander +{ +public: + RulePatternPlaceholderExpander(cmMakefile& makefile, std::string const& rule, + RulePatternSet const& patterns) + : Makefile(makefile) + , Rule(rule) + , Patterns(patterns) + { + } + + std::string ExpandPatterns(std::string const& string) + { + this->Expression = string; + this->InError = false; + return this->ExpandVariables(string); + } + + std::string ExpandVariables(std::string const& string); + + bool InError = false; + +private: + cmMakefile& Makefile; + std::string const& Rule; + std::string Expression; + RulePatternSet const& Patterns; + std::stack DAGStack; + + std::string ExpandVariable(std::string const& variable) override; +}; +} diff --git a/Tests/RunCMake/CustomRule/OUTPUT_FILE_SET-BadPattern-stderr.txt b/Tests/RunCMake/CustomRule/OUTPUT_FILE_SET-BadPattern-stderr.txt index fd4d1b016c..1a6e184bf5 100644 --- a/Tests/RunCMake/CustomRule/OUTPUT_FILE_SET-BadPattern-stderr.txt +++ b/Tests/RunCMake/CustomRule/OUTPUT_FILE_SET-BadPattern-stderr.txt @@ -1,4 +1,8 @@ CMake Error in CMakeLists\.txt: - Output File Set name, for rule "foo", did not expand correctly: + The pattern "PATTERN" is not defined. + + +CMake Error in CMakeLists\.txt: + Output File Set name, for the rule "foo", did not expand correctly: "bad__foo_bar_fs" diff --git a/Tests/RunCMake/CustomRule/PatternCyclicReference-result.txt b/Tests/RunCMake/CustomRule/PatternCyclicReference-result.txt new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/Tests/RunCMake/CustomRule/PatternCyclicReference-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/CustomRule/PatternCyclicReference-stderr.txt b/Tests/RunCMake/CustomRule/PatternCyclicReference-stderr.txt new file mode 100644 index 0000000000..b0770c72c4 --- /dev/null +++ b/Tests/RunCMake/CustomRule/PatternCyclicReference-stderr.txt @@ -0,0 +1,15 @@ +CMake Error in CMakeLists.txt: + Error evaluating the pattern "OUTPUT_FILE" in the expression: + + + + Dependency loop found. + + +CMake Error in CMakeLists.txt: + Pattern expression + + + + from rule "simple", used by the fileset "fs" of the target "foo", did not + expand correctly. diff --git a/Tests/RunCMake/CustomRule/PatternCyclicReference.cmake b/Tests/RunCMake/CustomRule/PatternCyclicReference.cmake new file mode 100644 index 0000000000..fe05720357 --- /dev/null +++ b/Tests/RunCMake/CustomRule/PatternCyclicReference.cmake @@ -0,0 +1,10 @@ + +enable_language(C) + +add_custom_rule(simple OUTPUT + COMMAND cmd) + +add_library(foo STATIC) + +target_sources(foo PRIVATE FILE_SET fs TYPE simple FILES file.i) +set_property(FILE_SET fs TARGET foo PROPERTY RULE_PATTERNS "OUTPUT_FILE=;OUTPUT2=") diff --git a/Tests/RunCMake/CustomRule/PatternSelfReference-result.txt b/Tests/RunCMake/CustomRule/PatternSelfReference-result.txt new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/Tests/RunCMake/CustomRule/PatternSelfReference-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/CustomRule/PatternSelfReference-stderr.txt b/Tests/RunCMake/CustomRule/PatternSelfReference-stderr.txt new file mode 100644 index 0000000000..2c4c6156ee --- /dev/null +++ b/Tests/RunCMake/CustomRule/PatternSelfReference-stderr.txt @@ -0,0 +1,16 @@ +CMake Error in CMakeLists.txt: + Error evaluating the pattern "OUTPUT_FILE" in the expression: + + + + Self reference on rule "simple". + + + +CMake Error in CMakeLists.txt: + Pattern expression + + + + from rule "simple", used by the fileset "fs" of the target "foo", did not + expand correctly. diff --git a/Tests/RunCMake/CustomRule/PatternSelfReference.cmake b/Tests/RunCMake/CustomRule/PatternSelfReference.cmake new file mode 100644 index 0000000000..12ebe0b3db --- /dev/null +++ b/Tests/RunCMake/CustomRule/PatternSelfReference.cmake @@ -0,0 +1,10 @@ + +enable_language(C) + +add_custom_rule(simple OUTPUT + COMMAND cmd) + +add_library(foo STATIC) + +target_sources(foo PRIVATE FILE_SET fs TYPE simple FILES file.i) +set_property(FILE_SET fs TARGET foo PROPERTY RULE_PATTERNS "OUTPUT_FILE=") diff --git a/Tests/RunCMake/CustomRule/Patterns.cmake b/Tests/RunCMake/CustomRule/Patterns.cmake index cbf27444fd..e10ede922e 100644 --- a/Tests/RunCMake/CustomRule/Patterns.cmake +++ b/Tests/RunCMake/CustomRule/Patterns.cmake @@ -45,3 +45,12 @@ target_sources(foo3 PRIVATE FILE_SET fs TYPE preprocess FILES file1.c) set_property(FILE_SET fs TARGET foo3 PROPERTY RULE_PATTERNS "DEFINES=${CMAKE_C_DEFINE_FLAG}WRONG=1") + + +# Define complex multi-level pattern +add_library(foo4 STATIC) + +target_sources(foo4 PRIVATE FILE_SET fs TYPE preprocess FILES file1.c) + +set_property(FILE_SET fs TARGET foo4 PROPERTY RULE_PATTERNS + "INPUT=$>;OUTPUT_DIR=/;CMAKE_C_COMPILER=${CMAKE_C_COMPILER};PREPROCESSED_SOURCE=$/.c>;INCLUDES=;DEFINES=;DEFINE_FLAG=${CMAKE_C_DEFINE_FLAG};VALUE=RULE_PATTERN=;DATA=1;FLAGS=") diff --git a/Tests/RunCMake/CustomRule/RunCMakeTest.cmake b/Tests/RunCMake/CustomRule/RunCMakeTest.cmake index 42e2a6ca09..4e70689168 100644 --- a/Tests/RunCMake/CustomRule/RunCMakeTest.cmake +++ b/Tests/RunCMake/CustomRule/RunCMakeTest.cmake @@ -12,6 +12,9 @@ run_cmake(OUTPUT_FILE_SET-BadPattern) run_cmake(OUTPUT_FILE_SET-BadType) run_cmake(OUTPUT_FILE_SET-TypeMismatch) +run_cmake(PatternSelfReference) +run_cmake(PatternCyclicReference) + run_cmake(RuleProperties1) run_cmake(RuleProperties2) diff --git a/Utilities/std/cmext/algorithm b/Utilities/std/cmext/algorithm index 2aa1ec5ff6..6a850eebce 100644 --- a/Utilities/std/cmext/algorithm +++ b/Utilities/std/cmext/algorithm @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -18,7 +19,6 @@ #if defined(__SUNPRO_CC) && defined(__sparc) # include # include -# include #endif namespace cm { diff --git a/bootstrap b/bootstrap index 301c47746d..5faa964ff8 100755 --- a/bootstrap +++ b/bootstrap @@ -464,6 +464,7 @@ CMAKE_CXX_SOURCES="\ cmPlaceholderExpander \ cmPlistParser \ cmRule \ + cmRuleHelper \ cmRulePlaceholderExpander \ cmRuntimeDependencyArchive \ cmScriptGenerator \