Browse Source

Merge topic 'warn-invalid-experimental'

9036aa10f3 Experimental: provide useful warning for invalid experimental variable values
b05e5403d9 Experimental: Avoid mutating statically allocated data
348ba701fe Tests/RunCMake/cmake_language: Expect experimental warnings more precisely

Acked-by: Kitware Robot <[email protected]>
Acked-by: buildbot <[email protected]>
Merge-request: !10767
Brad King 8 months ago
parent
commit
7deaa4dfb8

+ 29 - 24
Source/cmExperimental.cxx

@@ -7,8 +7,10 @@
 #include <cstddef>
 #include <string>
 
+#include "cmGlobalGenerator.h"
 #include "cmMakefile.h"
 #include "cmMessageType.h"
+#include "cmStringAlgorithms.h"
 #include "cmValue.h"
 
 namespace {
@@ -18,7 +20,7 @@ namespace {
  * Search for other instances to keep the documentation and test suite
  * up-to-date.
  */
-cmExperimental::FeatureData LookupTable[] = {
+cmExperimental::FeatureData const LookupTable[] = {
   // ExportPackageDependencies
   { "ExportPackageDependencies",
     "1942b4fa-b2c5-4546-9385-83f254070067",
@@ -26,8 +28,7 @@ cmExperimental::FeatureData LookupTable[] = {
     "CMake's EXPORT_PACKAGE_DEPENDENCIES support is experimental. It is meant "
     "only for experimentation and feedback to CMake developers.",
     {},
-    cmExperimental::TryCompileCondition::Always,
-    false },
+    cmExperimental::TryCompileCondition::Always },
   // WindowsKernelModeDriver
   { "WindowsKernelModeDriver",
     "9157bf90-2313-44d6-aefa-67cd83c8be7c",
@@ -35,8 +36,7 @@ cmExperimental::FeatureData LookupTable[] = {
     "CMake's Windows kernel-mode driver support is experimental. It is meant "
     "only for experimentation and feedback to CMake developers.",
     {},
-    cmExperimental::TryCompileCondition::Always,
-    false },
+    cmExperimental::TryCompileCondition::Always },
   // CxxImportStd
   { "CxxImportStd",
     "d0edc3af-4c50-42ea-a356-e2862fe7a444",
@@ -44,8 +44,7 @@ cmExperimental::FeatureData LookupTable[] = {
     "CMake's support for `import std;` in C++23 and newer is experimental. It "
     "is meant only for experimentation and feedback to CMake developers.",
     {},
-    cmExperimental::TryCompileCondition::Always,
-    false },
+    cmExperimental::TryCompileCondition::Always },
   // ImportPackageInfo
   { "ImportPackageInfo",
     "e82e467b-f997-4464-8ace-b00808fff261",
@@ -54,8 +53,7 @@ cmExperimental::FeatureData LookupTable[] = {
     "Specification format (via find_package) is experimental. It is meant "
     "only for experimentation and feedback to CMake developers.",
     {},
-    cmExperimental::TryCompileCondition::Always,
-    false },
+    cmExperimental::TryCompileCondition::Always },
   // ExportPackageInfo
   { "ExportPackageInfo",
     "b80be207-778e-46ba-8080-b23bba22639e",
@@ -64,8 +62,7 @@ cmExperimental::FeatureData LookupTable[] = {
     "Specification format is experimental. It is meant only for "
     "experimentation and feedback to CMake developers.",
     {},
-    cmExperimental::TryCompileCondition::Always,
-    false },
+    cmExperimental::TryCompileCondition::Always },
   // ExportBuildDatabase
   { "ExportBuildDatabase",
     "73194a1d-c0b5-41b9-9190-a4512925e192",
@@ -73,8 +70,7 @@ cmExperimental::FeatureData LookupTable[] = {
     "CMake's support for exporting build databases is experimental. It is "
     "meant only for experimentation and feedback to CMake developers.",
     {},
-    cmExperimental::TryCompileCondition::Never,
-    false },
+    cmExperimental::TryCompileCondition::Never },
   // Instrumentation
   { "Instrumentation",
     "a37d1069-1972-4901-b9c9-f194aaf2b6e0",
@@ -82,14 +78,13 @@ cmExperimental::FeatureData LookupTable[] = {
     "CMake's support for collecting instrumentation data is experimental. It "
     "is meant only for experimentation and feedback to CMake developers.",
     {},
-    cmExperimental::TryCompileCondition::Never,
-    false },
+    cmExperimental::TryCompileCondition::Never },
 };
 static_assert(sizeof(LookupTable) / sizeof(LookupTable[0]) ==
                 static_cast<size_t>(cmExperimental::Feature::Sentinel),
               "Experimental feature lookup table mismatch");
 
-cmExperimental::FeatureData& DataForFeature(cmExperimental::Feature f)
+cmExperimental::FeatureData const& DataForFeature(cmExperimental::Feature f)
 {
   assert(f != cmExperimental::Feature::Sentinel);
   return LookupTable[static_cast<size_t>(f)];
@@ -118,16 +113,26 @@ cm::optional<cmExperimental::Feature> cmExperimental::FeatureByName(
 bool cmExperimental::HasSupportEnabled(cmMakefile const& mf, Feature f)
 {
   bool enabled = false;
-  auto& data = ::DataForFeature(f);
+  FeatureData const& data = cmExperimental::DataForFeature(f);
 
-  auto value = mf.GetDefinition(data.Variable);
-  if (value == data.Uuid) {
-    enabled = true;
-  }
+  if (cmValue value = mf.GetDefinition(data.Variable)) {
+    enabled = *value == data.Uuid;
 
-  if (enabled && !data.Warned) {
-    mf.IssueMessage(MessageType::AUTHOR_WARNING, data.Description);
-    data.Warned = true;
+    if (mf.GetGlobalGenerator()->ShouldWarnExperimental(data.Name, *value)) {
+      if (enabled) {
+        mf.IssueMessage(MessageType::AUTHOR_WARNING, data.Description);
+      } else {
+        mf.IssueMessage(
+          MessageType::AUTHOR_WARNING,
+          cmStrCat(
+            data.Variable, " is set to incorrect value\n  ", value, '\n',
+            "See 'Help/dev/experimental.rst' in the source tree of this "
+            "version of CMake for documentation of the experimental feature "
+            "and the corresponding activation value.  This project's code "
+            "may require changes to work with this CMake's version of the "
+            "feature."));
+      }
+    }
   }
 
   return enabled;

+ 6 - 7
Source/cmExperimental.h

@@ -37,13 +37,12 @@ public:
 
   struct FeatureData
   {
-    std::string const Name;
-    std::string const Uuid;
-    std::string const Variable;
-    std::string const Description;
-    std::vector<std::string> const TryCompileVariables;
-    TryCompileCondition const ForwardThroughTryCompile;
-    bool Warned;
+    std::string Name;
+    std::string Uuid;
+    std::string Variable;
+    std::string Description;
+    std::vector<std::string> TryCompileVariables;
+    TryCompileCondition ForwardThroughTryCompile;
   };
 
   static FeatureData const& DataForFeature(Feature f);

+ 9 - 0
Source/cmGlobalGenerator.cxx

@@ -2003,6 +2003,7 @@ void cmGlobalGenerator::ClearGeneratorMembers()
   this->GeneratedFiles.clear();
   this->RuntimeDependencySets.clear();
   this->RuntimeDependencySetsByName.clear();
+  this->WarnedExperimental.clear();
 }
 
 void cmGlobalGenerator::ComputeTargetObjectDirectory(
@@ -3914,3 +3915,11 @@ void cmGlobalGenerator::AddCMakeFilesToRebuild(
                this->InstallScripts.end());
   files.insert(files.end(), this->TestFiles.begin(), this->TestFiles.end());
 }
+
+bool cmGlobalGenerator::ShouldWarnExperimental(cm::string_view featureName,
+                                               cm::string_view featureUuid)
+{
+  return this->WarnedExperimental
+    .emplace(cmStrCat(featureName, '-', featureUuid))
+    .second;
+}

+ 6 - 0
Source/cmGlobalGenerator.h

@@ -16,6 +16,7 @@
 #include <vector>
 
 #include <cm/optional>
+#include <cm/string_view>
 #include <cmext/algorithm>
 #include <cmext/string_view>
 
@@ -681,6 +682,9 @@ public:
     return configs;
   }
 
+  bool ShouldWarnExperimental(cm::string_view featureName,
+                              cm::string_view featureUuid);
+
 protected:
   // for a project collect all its targets by following depend
   // information, and also collect all the targets
@@ -908,6 +912,8 @@ private:
   // track targets to issue CMP0068 warning for.
   std::set<std::string> CMP0068WarnTargets;
 
+  std::unordered_set<std::string> WarnedExperimental;
+
   mutable std::map<cmSourceFile*, std::set<cmGeneratorTarget const*>>
     FilenameTargetDepends;
 

+ 5 - 5
Tests/RunCMake/cmake_language/Experimental/CxxImportStd-set-stderr.txt

@@ -1,6 +1,6 @@
-CMake Warning \(dev\) at Experimental/CxxImportStd-set.cmake:4 \(cmake_language\):
-  CMake's support for `import std;` in C\+\+23 and newer is experimental.  It
-  is meant only for experimentation and feedback to CMake developers.
+^CMake Warning \(dev\) at Experimental/CxxImportStd-set\.cmake:4 \(cmake_language\):
+  CMake's support for `import std;` in C\+\+23 and newer is experimental\.  It
+  is meant only for experimentation and feedback to CMake developers\.
 Call Stack \(most recent call first\):
-  CMakeLists.txt:3 \(include\)
-This warning is for project developers.  Use -Wno-dev to suppress it.
+  CMakeLists\.txt:[0-9]+ \(include\)
+This warning is for project developers\.  Use -Wno-dev to suppress it\.$

+ 25 - 0
Tests/RunCMake/cmake_language/Experimental/CxxImportStd-wrong-stderr.txt

@@ -0,0 +1,25 @@
+^CMake Warning \(dev\) at Experimental/CxxImportStd-wrong\.cmake:4 \(cmake_language\):
+  CMAKE_EXPERIMENTAL_CXX_IMPORT_STD is set to incorrect value
+
+    01234567-0123-0123-0123-0123456789ab
+
+  See 'Help/dev/experimental\.rst' in the source tree of this version of CMake
+  for documentation of the experimental feature and the corresponding
+  activation value\.  This project's code may require changes to work with
+  this CMake's version of the feature\.
+Call Stack \(most recent call first\):
+  CMakeLists\.txt:[0-9]+ \(include\)
+This warning is for project developers\.  Use -Wno-dev to suppress it\.
++
+CMake Warning \(dev\) at Experimental/CxxImportStd-wrong\.cmake:19 \(cmake_language\):
+  CMAKE_EXPERIMENTAL_CXX_IMPORT_STD is set to incorrect value
+
+    76543210-3210-3210-3210-ba9876543210
+
+  See 'Help/dev/experimental\.rst' in the source tree of this version of CMake
+  for documentation of the experimental feature and the corresponding
+  activation value\.  This project's code may require changes to work with
+  this CMake's version of the feature\.
+Call Stack \(most recent call first\):
+  CMakeLists\.txt:[0-9]+ \(include\)
+This warning is for project developers\.  Use -Wno-dev to suppress it\.$

+ 21 - 0
Tests/RunCMake/cmake_language/Experimental/CxxImportStd-wrong.cmake

@@ -0,0 +1,21 @@
+set(CMAKE_EXPERIMENTAL_CXX_IMPORT_STD
+  "01234567-0123-0123-0123-0123456789ab")
+
+cmake_language(GET_EXPERIMENTAL_FEATURE_ENABLED
+  "CxxImportStd"
+  feature_present)
+
+if (NOT feature_present STREQUAL "FALSE")
+  message(FATAL_ERROR
+    "Expected the `CxxImportStd` feature to be disabled.")
+endif ()
+
+# Test if/when warning is repeated.
+cmake_language(GET_EXPERIMENTAL_FEATURE_ENABLED
+  "CxxImportStd"
+  feature_present)
+set(CMAKE_EXPERIMENTAL_CXX_IMPORT_STD
+  "76543210-3210-3210-3210-ba9876543210")
+cmake_language(GET_EXPERIMENTAL_FEATURE_ENABLED
+  "CxxImportStd"
+  feature_present)

+ 5 - 5
Tests/RunCMake/cmake_language/Experimental/ExportPackageDependencies-set-stderr.txt

@@ -1,6 +1,6 @@
-CMake Warning \(dev\) at Experimental/ExportPackageDependencies-set.cmake:4 \(cmake_language\):
-  CMake's EXPORT_PACKAGE_DEPENDENCIES support is experimental.  It is meant
-  only for experimentation and feedback to CMake developers.
+^CMake Warning \(dev\) at Experimental/ExportPackageDependencies-set\.cmake:4 \(cmake_language\):
+  CMake's EXPORT_PACKAGE_DEPENDENCIES support is experimental\.  It is meant
+  only for experimentation and feedback to CMake developers\.
 Call Stack \(most recent call first\):
-  CMakeLists.txt:3 \(include\)
-This warning is for project developers.  Use -Wno-dev to suppress it.
+  CMakeLists\.txt:[0-9]+ \(include\)
+This warning is for project developers\.  Use -Wno-dev to suppress it\.$

+ 25 - 0
Tests/RunCMake/cmake_language/Experimental/ExportPackageDependencies-wrong-stderr.txt

@@ -0,0 +1,25 @@
+^CMake Warning \(dev\) at Experimental/ExportPackageDependencies-wrong\.cmake:4 \(cmake_language\):
+  CMAKE_EXPERIMENTAL_EXPORT_PACKAGE_DEPENDENCIES is set to incorrect value
+
+    01234567-0123-0123-0123-0123456789ab
+
+  See 'Help/dev/experimental\.rst' in the source tree of this version of CMake
+  for documentation of the experimental feature and the corresponding
+  activation value\.  This project's code may require changes to work with
+  this CMake's version of the feature\.
+Call Stack \(most recent call first\):
+  CMakeLists\.txt:[0-9]+ \(include\)
+This warning is for project developers\.  Use -Wno-dev to suppress it\.
++
+CMake Warning \(dev\) at Experimental/ExportPackageDependencies-wrong\.cmake:19 \(cmake_language\):
+  CMAKE_EXPERIMENTAL_EXPORT_PACKAGE_DEPENDENCIES is set to incorrect value
+
+    76543210-3210-3210-3210-ba9876543210
+
+  See 'Help/dev/experimental\.rst' in the source tree of this version of CMake
+  for documentation of the experimental feature and the corresponding
+  activation value\.  This project's code may require changes to work with
+  this CMake's version of the feature\.
+Call Stack \(most recent call first\):
+  CMakeLists\.txt:[0-9]+ \(include\)
+This warning is for project developers\.  Use -Wno-dev to suppress it\.$

+ 21 - 0
Tests/RunCMake/cmake_language/Experimental/ExportPackageDependencies-wrong.cmake

@@ -0,0 +1,21 @@
+set(CMAKE_EXPERIMENTAL_EXPORT_PACKAGE_DEPENDENCIES
+  "01234567-0123-0123-0123-0123456789ab")
+
+cmake_language(GET_EXPERIMENTAL_FEATURE_ENABLED
+  "ExportPackageDependencies"
+  feature_present)
+
+if (NOT feature_present STREQUAL "FALSE")
+  message(FATAL_ERROR
+    "Expected the `ExportPackageDependencies` feature to be disabled.")
+endif ()
+
+# Test if/when warning is repeated.
+cmake_language(GET_EXPERIMENTAL_FEATURE_ENABLED
+  "ExportPackageDependencies"
+  feature_present)
+set(CMAKE_EXPERIMENTAL_EXPORT_PACKAGE_DEPENDENCIES
+  "76543210-3210-3210-3210-ba9876543210")
+cmake_language(GET_EXPERIMENTAL_FEATURE_ENABLED
+  "ExportPackageDependencies"
+  feature_present)

+ 3 - 3
Tests/RunCMake/cmake_language/Experimental/WindowsKernelModeDriver-set-stderr.txt

@@ -1,6 +1,6 @@
-CMake Warning \(dev\) at Experimental/WindowsKernelModeDriver-set.cmake:4 \(cmake_language\):
+^CMake Warning \(dev\) at Experimental/WindowsKernelModeDriver-set.cmake:4 \(cmake_language\):
   CMake's Windows kernel-mode driver support is experimental.  It is meant
   only for experimentation and feedback to CMake developers.
 Call Stack \(most recent call first\):
-  CMakeLists.txt:3 \(include\)
-This warning is for project developers.  Use -Wno-dev to suppress it.
+  CMakeLists.txt:[0-9]+ \(include\)
+This warning is for project developers.  Use -Wno-dev to suppress it.$

+ 25 - 0
Tests/RunCMake/cmake_language/Experimental/WindowsKernelModeDriver-wrong-stderr.txt

@@ -0,0 +1,25 @@
+^CMake Warning \(dev\) at Experimental/WindowsKernelModeDriver-wrong\.cmake:4 \(cmake_language\):
+  CMAKE_EXPERIMENTAL_WINDOWS_KERNEL_MODE_DRIVER is set to incorrect value
+
+    01234567-0123-0123-0123-0123456789ab
+
+  See 'Help/dev/experimental\.rst' in the source tree of this version of CMake
+  for documentation of the experimental feature and the corresponding
+  activation value\.  This project's code may require changes to work with
+  this CMake's version of the feature\.
+Call Stack \(most recent call first\):
+  CMakeLists\.txt:[0-9]+ \(include\)
+This warning is for project developers\.  Use -Wno-dev to suppress it\.
++
+CMake Warning \(dev\) at Experimental/WindowsKernelModeDriver-wrong\.cmake:19 \(cmake_language\):
+  CMAKE_EXPERIMENTAL_WINDOWS_KERNEL_MODE_DRIVER is set to incorrect value
+
+    76543210-3210-3210-3210-ba9876543210
+
+  See 'Help/dev/experimental\.rst' in the source tree of this version of CMake
+  for documentation of the experimental feature and the corresponding
+  activation value\.  This project's code may require changes to work with
+  this CMake's version of the feature\.
+Call Stack \(most recent call first\):
+  CMakeLists\.txt:[0-9]+ \(include\)
+This warning is for project developers\.  Use -Wno-dev to suppress it\.$

+ 21 - 0
Tests/RunCMake/cmake_language/Experimental/WindowsKernelModeDriver-wrong.cmake

@@ -0,0 +1,21 @@
+set(CMAKE_EXPERIMENTAL_WINDOWS_KERNEL_MODE_DRIVER
+  "01234567-0123-0123-0123-0123456789ab")
+
+cmake_language(GET_EXPERIMENTAL_FEATURE_ENABLED
+  "WindowsKernelModeDriver"
+  feature_present)
+
+if (NOT feature_present STREQUAL "FALSE")
+  message(FATAL_ERROR
+    "Expected the `WindowsKernelModeDriver` feature to be disabled.")
+endif ()
+
+# Test if/when warning is repeated.
+cmake_language(GET_EXPERIMENTAL_FEATURE_ENABLED
+  "WindowsKernelModeDriver"
+  feature_present)
+set(CMAKE_EXPERIMENTAL_WINDOWS_KERNEL_MODE_DRIVER
+  "76543210-3210-3210-3210-ba9876543210")
+cmake_language(GET_EXPERIMENTAL_FEATURE_ENABLED
+  "WindowsKernelModeDriver"
+  feature_present)

+ 3 - 0
Tests/RunCMake/cmake_language/RunCMakeTest.cmake

@@ -159,8 +159,11 @@ run_cmake_command(
 
 run_cmake(Experimental/CxxImportStd-set)
 run_cmake(Experimental/CxxImportStd-unset)
+run_cmake(Experimental/CxxImportStd-wrong)
 run_cmake(Experimental/ExportPackageDependencies-set)
 run_cmake(Experimental/ExportPackageDependencies-unset)
+run_cmake(Experimental/ExportPackageDependencies-wrong)
 run_cmake(Experimental/WindowsKernelModeDriver-set)
 run_cmake(Experimental/WindowsKernelModeDriver-unset)
+run_cmake(Experimental/WindowsKernelModeDriver-wrong)
 run_cmake(Experimental/Unknown)