Merge topic 'check-library-properties-fix-performances-regression' into release-3.24

985b4c82a6 Check link libraries properties: fix performances regression
a47eef32a3 renames method FinalizeTargetCompileInfo() in FinalizeTargetConfiguration().

Acked-by: Kitware Robot <[email protected]>
Merge-request: !7651

Source/cmGeneratorTarget.cxx

@@ -8622,7 +8622,7 @@ bool cmGeneratorTarget::AddHeaderSetVerification()
             verifyTarget->SetProperty("UNITY_BUILD", "OFF");
             cm::optional<std::map<std::string, cmValue>>
               perConfigCompileDefinitions;
-            verifyTarget->FinalizeTargetCompileInfo(
+            verifyTarget->FinalizeTargetConfiguration(
               this->Makefile->GetCompileDefinitionsEntries(),
               perConfigCompileDefinitions);
 

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<cmMSVC60LinkLineComputer>(outputConverter, stateDir));
 }
 
-void cmGlobalGenerator::FinalizeTargetCompileInfo()
+void cmGlobalGenerator::FinalizeTargetConfiguration()
 {
   std::vector<std::string> 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

Source/cmGlobalGenerator.h

@@ -694,7 +694,7 @@ private:
 
   void WriteSummary();
   void WriteSummary(cmGeneratorTarget* target);
-  void FinalizeTargetCompileInfo();
+  void FinalizeTargetConfiguration();
 
   virtual void ForceLinkerLanguages();
 

Source/cmLinkLibrariesCommand.cxx

@@ -35,7 +35,5 @@ bool cmLinkLibrariesCommand(std::vector<std::string> const& args,
     mf.AppendProperty("LINK_LIBRARIES", *i);
   }
 
-  mf.CheckProperty("LINK_LIBRARIES");
-
   return true;
 }

Source/cmMakefile.cxx

@@ -3987,31 +3987,6 @@ std::vector<std::string> 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 <LINK_LIBRARY:> internal pattern
-      static cmsys::RegularExpression linkPattern(
-        "(^|;)(</?LINK_(LIBRARY|GROUP):[^;>]*>)(;|$)");
-      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 \"$<LINK_",
-                 linkPattern.match(3),
-                 ":...>\" 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);

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<std::string> GetPropertyKeys() const;
-  void CheckProperty(const std::string& prop) const;
 
   //! Initialize a makefile from its parent
   void InitializeFromParent(cmMakefile* parent);

Source/cmTarget.cxx

@@ -1881,7 +1881,41 @@ void cmTarget::AppendBuildInterfaceIncludes()
   }
 }
 
-void cmTarget::FinalizeTargetCompileInfo(
+namespace {
+bool CheckLinkLibraryPattern(cm::string_view property,
+                             const std::vector<BT<std::string>>& value,
+                             cmake* context)
+{
+  // Look for <LINK_LIBRARY:> and </LINK_LIBRARY:> internal tags
+  static cmsys::RegularExpression linkPattern(
+    "(^|;)(</?LINK_(LIBRARY|GROUP):[^;>]*>)(;|$)");
+
+  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 \"$<LINK_",
+        linkPattern.match(3),
+        ":...>\" which may be used to specify how the libraries are linked."),
+      item.Backtrace);
+  }
+
+  return isValid;
+}
+}
+
+void cmTarget::FinalizeTargetConfiguration(
   const cmBTStringRange& noConfigCompileDefinitions,
   cm::optional<std::map<std::string, cmValue>>& perConfigCompileDefinitions)
 {
@@ -1889,6 +1923,18 @@ void cmTarget::FinalizeTargetCompileInfo(
     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<std::string> const& entry)
 }
 
 namespace {
-void CheckLinkLibraryPattern(const std::string& property,
-                             const std::string& value, cmMakefile* context)
-{
-  // Look for <LINK_LIBRARY:> and </LINK_LIBRARY:> internal tags
-  static cmsys::RegularExpression linkPattern(
-    "(^|;)(</?LINK_(LIBRARY|GROUP):[^;>]*>)(;|$)");
-  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 \"$<LINK_",
-      linkPattern.match(3),
-      ":...>\" 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);

Source/cmTarget.h

@@ -236,7 +236,7 @@ public:
   void InsertPrecompileHeader(BT<std::string> const& entry);
 
   void AppendBuildInterfaceIncludes();
-  void FinalizeTargetCompileInfo(
+  void FinalizeTargetConfiguration(
     const cmBTStringRange& noConfigCompileDefinitions,
     cm::optional<std::map<std::string, cmValue>>& perConfigCompileDefinitions);
 

Source/cmTargetLinkLibrariesCommand.cxx

@@ -379,9 +379,6 @@ bool cmTargetLinkLibrariesCommand(std::vector<std::string> const& args,
     target->SetProperty("LINK_INTERFACE_LIBRARIES", "");
   }
 
-  target->CheckProperty("LINK_LIBRARIES", &mf);
-  target->CheckProperty("INTERFACE_LINK_LIBRARIES", &mf);
-
   return true;
 }
 

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 "<LINK_GROUP:feat>".  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 "</LINK_GROUP:feat>".
+  The LINK_LIBRARIES property may contain the generator-expression
   "\$<LINK_GROUP:...>" which may be used to specify how the libraries are
   linked.
 Call Stack \(most recent call first\):

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 "<LINK_LIBRARY:feat>".
+CMake Error at forbidden-arguments.cmake:[0-9]+ \(add_library\):
+  Property LINK_LIBRARIES contains the invalid item "</LINK_LIBRARY:feat>".
   The LINK_LIBRARIES property may contain the generator-expression
   "\$<LINK_LIBRARY:...>" which may be used to specify how the libraries are
   linked.