From a47eef32a30599e01e445e6dd69d4702bceae1bd Mon Sep 17 00:00:00 2001 From: Marc Chevrier Date: Wed, 7 Sep 2022 14:23:21 +0200 Subject: renames method FinalizeTargetCompileInfo() in FinalizeTargetConfiguration(). --- Source/cmGeneratorTarget.cxx | 2 +- Source/cmGlobalGenerator.cxx | 8 ++++---- Source/cmGlobalGenerator.h | 2 +- Source/cmTarget.cxx | 2 +- Source/cmTarget.h | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx index 99afee9..6235a2a 100644 --- a/Source/cmGeneratorTarget.cxx +++ b/Source/cmGeneratorTarget.cxx @@ -8622,7 +8622,7 @@ bool cmGeneratorTarget::AddHeaderSetVerification() verifyTarget->SetProperty("UNITY_BUILD", "OFF"); cm::optional> perConfigCompileDefinitions; - verifyTarget->FinalizeTargetCompileInfo( + verifyTarget->FinalizeTargetConfiguration( this->Makefile->GetCompileDefinitionsEntries(), perConfigCompileDefinitions); diff --git a/Source/cmGlobalGenerator.cxx b/Source/cmGlobalGenerator.cxx index 4feae6d..254220b 100644 --- a/Source/cmGlobalGenerator.cxx +++ b/Source/cmGlobalGenerator.cxx @@ -1501,7 +1501,7 @@ bool cmGlobalGenerator::Compute() if (!this->CheckALLOW_DUPLICATE_CUSTOM_TARGETS()) { return false; } - this->FinalizeTargetCompileInfo(); + this->FinalizeTargetConfiguration(); this->CreateGenerationObjects(); @@ -1825,7 +1825,7 @@ cmGlobalGenerator::CreateMSVC60LinkLineComputer( cm::make_unique(outputConverter, stateDir)); } -void cmGlobalGenerator::FinalizeTargetCompileInfo() +void cmGlobalGenerator::FinalizeTargetConfiguration() { std::vector const langs = this->CMakeInstance->GetState()->GetEnabledLanguages(); @@ -1838,8 +1838,8 @@ void cmGlobalGenerator::FinalizeTargetCompileInfo() for (auto& target : mf->GetTargets()) { cmTarget* t = &target.second; - t->FinalizeTargetCompileInfo(noConfigCompileDefinitions, - perConfigCompileDefinitions); + t->FinalizeTargetConfiguration(noConfigCompileDefinitions, + perConfigCompileDefinitions); } // The standard include directories for each language diff --git a/Source/cmGlobalGenerator.h b/Source/cmGlobalGenerator.h index dcef070..6e3072b 100644 --- a/Source/cmGlobalGenerator.h +++ b/Source/cmGlobalGenerator.h @@ -694,7 +694,7 @@ private: void WriteSummary(); void WriteSummary(cmGeneratorTarget* target); - void FinalizeTargetCompileInfo(); + void FinalizeTargetConfiguration(); virtual void ForceLinkerLanguages(); diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index 62ec344..5838dd2 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -1881,7 +1881,7 @@ void cmTarget::AppendBuildInterfaceIncludes() } } -void cmTarget::FinalizeTargetCompileInfo( +void cmTarget::FinalizeTargetConfiguration( const cmBTStringRange& noConfigCompileDefinitions, cm::optional>& perConfigCompileDefinitions) { diff --git a/Source/cmTarget.h b/Source/cmTarget.h index 37701f0..467c4da 100644 --- a/Source/cmTarget.h +++ b/Source/cmTarget.h @@ -236,7 +236,7 @@ public: void InsertPrecompileHeader(BT const& entry); void AppendBuildInterfaceIncludes(); - void FinalizeTargetCompileInfo( + void FinalizeTargetConfiguration( const cmBTStringRange& noConfigCompileDefinitions, cm::optional>& perConfigCompileDefinitions); -- cgit v0.12 From 985b4c82a64c064e1d22351091206e73ed6fc727 Mon Sep 17 00:00:00 2001 From: Marc Chevrier Date: Wed, 7 Sep 2022 15:20:53 +0200 Subject: Check link libraries properties: fix performances regression Fixes: #23939 --- Source/cmLinkLibrariesCommand.cxx | 2 - Source/cmMakefile.cxx | 25 ------- Source/cmMakefile.h | 1 - Source/cmTarget.cxx | 80 +++++++++++++--------- Source/cmTargetLinkLibrariesCommand.cxx | 3 - .../forbidden-arguments-stderr.txt | 6 +- .../forbidden-arguments-stderr.txt | 4 +- 7 files changed, 51 insertions(+), 70 deletions(-) diff --git a/Source/cmLinkLibrariesCommand.cxx b/Source/cmLinkLibrariesCommand.cxx index ed89e91..2b8f836 100644 --- a/Source/cmLinkLibrariesCommand.cxx +++ b/Source/cmLinkLibrariesCommand.cxx @@ -35,7 +35,5 @@ bool cmLinkLibrariesCommand(std::vector const& args, mf.AppendProperty("LINK_LIBRARIES", *i); } - mf.CheckProperty("LINK_LIBRARIES"); - return true; } diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 628eb1d..469eac3 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -3987,31 +3987,6 @@ std::vector cmMakefile::GetPropertyKeys() const return this->StateSnapshot.GetDirectory().GetPropertyKeys(); } -void cmMakefile::CheckProperty(const std::string& prop) const -{ - // Certain properties need checking. - if (prop == "LINK_LIBRARIES") { - if (cmValue value = this->GetProperty(prop)) { - // Look for internal pattern - static cmsys::RegularExpression linkPattern( - "(^|;)(]*>)(;|$)"); - if (!linkPattern.find(value)) { - return; - } - - // Report an error. - this->IssueMessage( - MessageType::FATAL_ERROR, - cmStrCat("Property ", prop, " contains the invalid item \"", - linkPattern.match(2), "\". The ", prop, - " property may contain the generator-expression \"$\" which may be used to specify how the libraries are " - "linked.")); - } - } -} - cmTarget* cmMakefile::FindLocalNonAliasTarget(const std::string& name) const { auto i = this->Targets.find(name); diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index e7b9716..27838b2 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -794,7 +794,6 @@ public: cmValue GetProperty(const std::string& prop, bool chain) const; bool GetPropertyAsBool(const std::string& prop) const; std::vector GetPropertyKeys() const; - void CheckProperty(const std::string& prop) const; //! Initialize a makefile from its parent void InitializeFromParent(cmMakefile* parent); diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index 5838dd2..cbe5d7d 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -1881,6 +1881,40 @@ void cmTarget::AppendBuildInterfaceIncludes() } } +namespace { +bool CheckLinkLibraryPattern(cm::string_view property, + const std::vector>& value, + cmake* context) +{ + // Look for and internal tags + static cmsys::RegularExpression linkPattern( + "(^|;)(]*>)(;|$)"); + + bool isValid = true; + + for (const auto& item : value) { + if (!linkPattern.find(item.Value)) { + continue; + } + + isValid = false; + + // Report an error. + context->IssueMessage( + MessageType::FATAL_ERROR, + cmStrCat( + "Property ", property, " contains the invalid item \"", + linkPattern.match(2), "\". The ", property, + " property may contain the generator-expression \"$\" which may be used to specify how the libraries are linked."), + item.Backtrace); + } + + return isValid; +} +} + void cmTarget::FinalizeTargetConfiguration( const cmBTStringRange& noConfigCompileDefinitions, cm::optional>& perConfigCompileDefinitions) @@ -1889,6 +1923,18 @@ void cmTarget::FinalizeTargetConfiguration( return; } + if (!CheckLinkLibraryPattern("LINK_LIBRARIES"_s, + this->impl->LinkImplementationPropertyEntries, + this->GetMakefile()->GetCMakeInstance()) || + !CheckLinkLibraryPattern("INTERFACE_LINK_LIBRARIES"_s, + this->impl->LinkInterfacePropertyEntries, + this->GetMakefile()->GetCMakeInstance()) || + !CheckLinkLibraryPattern("INTERFACE_LINK_LIBRARIES_DIRECT"_s, + this->impl->LinkInterfaceDirectPropertyEntries, + this->GetMakefile()->GetCMakeInstance())) { + return; + } + this->AppendBuildInterfaceIncludes(); if (this->GetType() == cmStateEnums::INTERFACE_LIBRARY) { @@ -1969,27 +2015,6 @@ void cmTarget::InsertPrecompileHeader(BT const& entry) } namespace { -void CheckLinkLibraryPattern(const std::string& property, - const std::string& value, cmMakefile* context) -{ - // Look for and internal tags - static cmsys::RegularExpression linkPattern( - "(^|;)(]*>)(;|$)"); - if (!linkPattern.find(value)) { - return; - } - - // Report an error. - context->IssueMessage( - MessageType::FATAL_ERROR, - cmStrCat( - "Property ", property, " contains the invalid item \"", - linkPattern.match(2), "\". The ", property, - " property may contain the generator-expression \"$\" which may be used to specify how the libraries are linked.")); -} - void CheckLINK_INTERFACE_LIBRARIES(const std::string& prop, const std::string& value, cmMakefile* context, bool imported) @@ -2024,13 +2049,6 @@ void CheckLINK_INTERFACE_LIBRARIES(const std::string& prop, } context->IssueMessage(MessageType::FATAL_ERROR, e.str()); } - - CheckLinkLibraryPattern(base, value, context); -} - -void CheckLINK_LIBRARIES(const std::string& value, cmMakefile* context) -{ - CheckLinkLibraryPattern("LINK_LIBRARIES", value, context); } void CheckINTERFACE_LINK_LIBRARIES(const std::string& value, @@ -2051,8 +2069,6 @@ void CheckINTERFACE_LINK_LIBRARIES(const std::string& value, context->IssueMessage(MessageType::FATAL_ERROR, e.str()); } - - CheckLinkLibraryPattern("INTERFACE_LINK_LIBRARIES", value, context); } void CheckIMPORTED_GLOBAL(const cmTarget* target, cmMakefile* context) @@ -2085,10 +2101,6 @@ void cmTarget::CheckProperty(const std::string& prop, if (cmValue value = this->GetProperty(prop)) { CheckLINK_INTERFACE_LIBRARIES(prop, *value, context, true); } - } else if (prop == "LINK_LIBRARIES") { - if (cmValue value = this->GetProperty(prop)) { - CheckLINK_LIBRARIES(*value, context); - } } else if (prop == "INTERFACE_LINK_LIBRARIES") { if (cmValue value = this->GetProperty(prop)) { CheckINTERFACE_LINK_LIBRARIES(*value, context); diff --git a/Source/cmTargetLinkLibrariesCommand.cxx b/Source/cmTargetLinkLibrariesCommand.cxx index ba901d0..af870da 100644 --- a/Source/cmTargetLinkLibrariesCommand.cxx +++ b/Source/cmTargetLinkLibrariesCommand.cxx @@ -379,9 +379,6 @@ bool cmTargetLinkLibrariesCommand(std::vector const& args, target->SetProperty("LINK_INTERFACE_LIBRARIES", ""); } - target->CheckProperty("LINK_LIBRARIES", &mf); - target->CheckProperty("INTERFACE_LINK_LIBRARIES", &mf); - return true; } diff --git a/Tests/RunCMake/GenEx-LINK_GROUP/forbidden-arguments-stderr.txt b/Tests/RunCMake/GenEx-LINK_GROUP/forbidden-arguments-stderr.txt index bae6505..b29303b 100644 --- a/Tests/RunCMake/GenEx-LINK_GROUP/forbidden-arguments-stderr.txt +++ b/Tests/RunCMake/GenEx-LINK_GROUP/forbidden-arguments-stderr.txt @@ -1,6 +1,6 @@ -CMake Error at forbidden-arguments.cmake:[0-9]+ \(link_libraries\): - Property LINK_LIBRARIES contains the invalid item "". The - LINK_LIBRARIES property may contain the generator-expression +CMake Error at forbidden-arguments.cmake:[0-9]+ \(add_library\): + Property LINK_LIBRARIES contains the invalid item "". + The LINK_LIBRARIES property may contain the generator-expression "\$" which may be used to specify how the libraries are linked. Call Stack \(most recent call first\): diff --git a/Tests/RunCMake/GenEx-LINK_LIBRARY/forbidden-arguments-stderr.txt b/Tests/RunCMake/GenEx-LINK_LIBRARY/forbidden-arguments-stderr.txt index 5245dd8..fc8e383 100644 --- a/Tests/RunCMake/GenEx-LINK_LIBRARY/forbidden-arguments-stderr.txt +++ b/Tests/RunCMake/GenEx-LINK_LIBRARY/forbidden-arguments-stderr.txt @@ -1,5 +1,5 @@ -CMake Error at forbidden-arguments.cmake:[0-9]+ \(link_libraries\): - Property LINK_LIBRARIES contains the invalid item "". +CMake Error at forbidden-arguments.cmake:[0-9]+ \(add_library\): + Property LINK_LIBRARIES contains the invalid item "". The LINK_LIBRARIES property may contain the generator-expression "\$" which may be used to specify how the libraries are linked. -- cgit v0.12