GenEx: Fix evaluation of $<CONFIG> on imported targets

The historic implementation of `$<CONFIG>` had some errors that could
result in multiple configurations matching. First, it always considered
the configuration of the consuming target, even if a consumed imported
target selected a different configuration. Second, it matched the entire
list of `MAP_IMPORTED_CONFIG_<CONFIG>` configurations, even if none of
those were actually selected. The latter in particular is redundant at
best, as we also consider the selected configuration of an imported
target, which is the correct configuration to match for imported
targets. Refactor the implementation so that only one configuration is
considered.

Fixes: #23660
Issue: #27022

Help/manual/cmake-policies.7.rst

@@ -98,6 +98,7 @@ Policies Introduced by CMake 4.2
 .. toctree::
    :maxdepth: 1
 
+   CMP0199: $<CONFIG> only matches the configuration of the consumed target. </policy/CMP0199>
    CMP0198: CMAKE_PARENT_LIST_FILE is not defined in CMakeLists.txt. </policy/CMP0198>
 
 Policies Introduced by CMake 4.1

Help/policy/CMP0199.rst

@@ -0,0 +1,93 @@
+CMP0199
+-------
+
+.. versionadded:: 4.2
+
+:genex:`$<CONFIG:cfgs>` only matches the configuration of the consumed target.
+
+Historically, when a :genex:`$<CONFIG:cfgs>` generator expression appeared in
+the properties of an imported target, it would match (that is, evaluate to
+``1``) if any of the ``cfgs`` matched *any* of the following:
+
+1. The selected configuration of the imported target being consumed.
+
+2. The configuration of the consuming target.
+
+3. *Any* of the configurations in the :prop_tgt:`MAP_IMPORTED_CONFIG_<CONFIG>`
+   of the imported target being consumed
+   (where ``<CONFIG>`` is the configuration of the consuming target),
+   *whether or not such configurations are valid for the imported target*.
+
+This could result in expressions which are intended to be mutually exclusive
+being concurrently evaluated.  This would be especially problematic if the
+value of a compile definition is intended to be determined by the
+configuration, as this lack of exclusivity could result in redefinition.
+
+CMake 4.2 and above prefer to consider *only* the configuration of the imported
+target being consumed; that is, (1) in the above list.
+
+This policy provides compatibility with projects that rely on the historical
+behavior.  The ``OLD`` behavior for this policy is to retain the historic
+behavior as described above.  The ``NEW`` behavior is to consider only the
+configuration of the imported target being consumed.
+
+.. note::
+
+  This policy only applies to generator expressions being evaluated as part of
+  the usage requirements of imported targets which are not imported from |CPS|
+  packages.
+
+  For non-imported targets, both the historic and ongoing behavior is to
+  consider only the configuration of the consuming target.  (The selected
+  configuration of a non-imported target is always the active build
+  configuration, which is necessarily the same as the consuming target's
+  configuration.)
+
+  For targets imported from |CPS| packages, the ``NEW`` behavior is used,
+  regardless of the policy setting.
+
+.. |INTRODUCED_IN_CMAKE_VERSION| replace:: 4.2
+.. |WARNS_OR_DOES_NOT_WARN| replace:: warns
+.. include:: include/STANDARD_ADVICE.rst
+
+.. include:: include/DEPRECATED.rst
+
+Examples
+^^^^^^^^
+
+Consider the following imported libraries:
+
+.. code-block:: cmake
+
+  add_library(test1 INTERFACE IMPORTED)
+  set_target_properties(test1 PROPERTIES
+    IMPORTED_CONFIGURATIONS "DEBUG"
+    INTERFACE_COMPILE_DEFINITIONS
+      "$<$<CONFIG:debug>:DEBUG>;$<$<CONFIG:release>:RELEASE>"
+  )
+
+  add_library(test2 INTERFACE IMPORTED)
+  set_target_properties(test2 PROPERTIES
+    IMPORTED_CONFIGURATIONS "TEST"
+    INTERFACE_COMPILE_DEFINITIONS
+      "$<$<CONFIG:debug>:DEBUG>;$<$<CONFIG:example>:EXAMPLE>;$<$<CONFIG:test>:TEST>"
+    MAP_IMPORTED_CONFIG_RELEASE "DEBUG;EXAMPLE;TEST"
+  )
+
+Assume that the consuming project is built in the ``Release`` configuration.
+Under the ``OLD`` policy, a consumer of ``test1`` would see both ``DEBUG``
+and ``RELEASE`` defined; ``$<CONFIG:debug>`` evaluates to ``1`` because the
+selected configuration of ``test1`` is ``DEBUG``, and ``$<CONFIG:release>``
+evaluates to ``1`` because the consumer's configuration is ``Release``
+(keeping in mind that configuration matching is case-insensitive). Likewise,
+a consumer of ``test2`` would see all of ``DEBUG``, ``RELEASE`` and ``TEST``
+defined; ``$<CONFIG:debug>``, ``$<CONFIG:example>`` and ``$<CONFIG:test>`` all
+evaluate to ``1`` because all of these configurations appear in
+``MAP_IMPORTED_CONFIG_RELEASE``.
+
+Under the ``NEW`` policy, when ``test1`` is consumed, only ``$<CONFIG:debug>``
+will evaluate to ``1``. Similarly, when ``test2`` is consumed, only
+``$<CONFIG:test>`` will evaluate to ``1``. Both of these correspond to the
+configuration of the consumed library that is actually selected by CMake.
+
+.. |CPS| replace:: Common Package Specification

Help/release/dev/config-fix-matching.rst

@@ -0,0 +1,5 @@
+config-fix-matching
+-------------------
+
+The :genex:`$<CONFIG:cfgs>` generator expression no longer matches multiple
+configurations.  See policy :policy:`CMP0199`.

Source/cmGeneratorExpressionNode.cxx

@@ -2226,9 +2226,11 @@ static const struct ConfigurationTestNode : public cmGeneratorExpressionNode
     if (parameters.empty()) {
       return configurationNode.Evaluate(parameters, context, content, nullptr);
     }
-    static cmsys::RegularExpression configValidator("^[A-Za-z0-9_]*$");
 
     context->HadContextSensitiveCondition = true;
+
+    // First, validate our arguments.
+    static cmsys::RegularExpression configValidator("^[A-Za-z0-9_]*$");
     bool firstParam = true;
     for (auto const& param : parameters) {
       if (!configValidator.find(param)) {
@@ -2248,41 +2250,109 @@ static const struct ConfigurationTestNode : public cmGeneratorExpressionNode
         context->LG->GetCMakeInstance()->IssueMessage(
           MessageType::WARNING, e.str(), context->Backtrace);
       }
-
       firstParam = false;
-      if (context->Config.empty()) {
-        if (param.empty()) {
+    }
+
+    // Determine the context(s) in which the expression should be evaluated. If
+    // CMPxxxx is NEW, the context is exactly one of the imported target's
+    // selected configuration, if applicable, or the consuming target's
+    // configuration, otherwise.
+    //
+    // If CMPxxxx is OLD, we evaluate first in the context of the consuming
+    // target, then, if the consumed target is imported, we evaluate based on
+    // the mapped configurations (this logic is... problematic; see comment
+    // below), then finally based on the selected configuration of the imported
+    // target.
+    bool const targetIsImported =
+      (context->CurrentTarget && context->CurrentTarget->IsImported());
+    bool const oldPolicy = [&] {
+      if (!targetIsImported) {
+        // For non-imported targets, there is no behavior difference between
+        // the OLD and NEW policy.
+        return false;
+      }
+      cmTarget const* const t = context->CurrentTarget->Target;
+      if (t->GetOrigin() == cmTarget::Origin::Cps) {
+        // Generator expressions appearing on targets imported from CPS should
+        // always be evaluated according to the selected configuration of the
+        // imported target, i.e. the NEW policy.
+        return false;
+      }
+      switch (context->HeadTarget->GetPolicyStatusCMP0199()) {
+        case cmPolicies::WARN:
+          if (context->LG->GetMakefile()->PolicyOptionalWarningEnabled(
+                "CMAKE_POLICY_WARNING_CMP0199")) {
+            std::string const err =
+              cmStrCat(cmPolicies::GetPolicyWarning(cmPolicies::CMP0199),
+                       "\nEvaluation of $<CONFIG> for imported target  \"",
+                       context->CurrentTarget->GetName(), "\", used by \"",
+                       context->HeadTarget->GetName(),
+                       "\", may match multiple configurations.\n");
+            context->LG->GetCMakeInstance()->IssueMessage(
+              MessageType ::AUTHOR_WARNING, err, context->Backtrace);
+          }
+          CM_FALLTHROUGH;
+        case cmPolicies::OLD:
+          return true;
+        case cmPolicies::NEW:
+          return false;
+      }
+
+      // Should be unreachable
+      assert(false);
+      return false;
+    }();
+
+    if (!targetIsImported || oldPolicy) {
+      // Does the consuming target's configuration match any of the arguments?
+      for (auto const& param : parameters) {
+        if (context->Config.empty()) {
+          if (param.empty()) {
+            return "1";
+          }
+        } else if (cmsysString_strcasecmp(param.c_str(),
+                                          context->Config.c_str()) == 0) {
           return "1";
         }
-      } else if (cmsysString_strcasecmp(param.c_str(),
-                                        context->Config.c_str()) == 0) {
-        return "1";
       }
     }
 
-    if (context->CurrentTarget && context->CurrentTarget->IsImported()) {
+    if (targetIsImported) {
       cmValue loc = nullptr;
       cmValue imp = nullptr;
       std::string suffix;
       if (context->CurrentTarget->Target->GetMappedConfig(context->Config, loc,
                                                           imp, suffix)) {
-        // This imported target has an appropriate location
-        // for this (possibly mapped) config.
-        // Check if there is a proper config mapping for the tested config.
-        cmList mappedConfigs;
-        std::string mapProp = cmStrCat(
-          "MAP_IMPORTED_CONFIG_", cmSystemTools::UpperCase(context->Config));
-        if (cmValue mapValue = context->CurrentTarget->GetProperty(mapProp)) {
-          mappedConfigs.assign(cmSystemTools::UpperCase(*mapValue));
+        if (oldPolicy) {
+          // If the target has a MAP_IMPORTED_CONFIG_<CONFIG> property for the
+          // consumer's <CONFIG>, match *any* config in that list, regardless
+          // of whether it's valid or of what GetMappedConfig actually picked.
+          // This will result in $<CONFIG> producing '1' for multiple configs,
+          // and is almost certainly wrong, but it's what CMake did for a very
+          // long time, and... Hyrum's Law.
+          cmList mappedConfigs;
+          std::string mapProp = cmStrCat(
+            "MAP_IMPORTED_CONFIG_", cmSystemTools::UpperCase(context->Config));
+          if (cmValue mapValue =
+                context->CurrentTarget->GetProperty(mapProp)) {
+            mappedConfigs.assign(cmSystemTools::UpperCase(*mapValue));
 
-          for (auto const& param : parameters) {
-            if (cm::contains(mappedConfigs, cmSystemTools::UpperCase(param))) {
-              return "1";
+            for (auto const& param : parameters) {
+              if (cm::contains(mappedConfigs,
+                               cmSystemTools::UpperCase(param))) {
+                return "1";
+              }
             }
+
+            return "0";
           }
-        } else if (!suffix.empty()) {
-          // There is no explicit mapping for the tested config, so use
-          // the configuration of the imported location that was selected.
+        }
+
+        // This imported target has an appropriate location for this (possibly
+        // mapped) config.
+        if (!suffix.empty()) {
+          // Use the (possibly mapped) configuration of the imported location
+          // that was selected.
           for (auto const& param : parameters) {
             if (cmStrCat('_', cmSystemTools::UpperCase(param)) == suffix) {
               return "1";
@@ -2291,6 +2361,7 @@ static const struct ConfigurationTestNode : public cmGeneratorExpressionNode
         }
       }
     }
+
     return "0";
   }
 } configurationTestNode;

Source/cmPolicies.h

@@ -592,7 +592,11 @@ class cmMakefile;
          WARN)                                                                \
   SELECT(POLICY, CMP0198,                                                     \
          "CMAKE_PARENT_LIST_FILE is not defined in CMakeLists.txt.", 4, 2, 0, \
-         WARN)
+         WARN)                                                                \
+  SELECT(                                                                     \
+    POLICY, CMP0199,                                                          \
+    "$<CONFIG:cfgs> only matches the configuration of the consumed target.",  \
+    4, 2, 0, WARN)
 
 #define CM_SELECT_ID(F, A1, A2, A3, A4, A5, A6) F(A1)
 #define CM_FOR_EACH_POLICY_ID(POLICY)                                         \
@@ -640,7 +644,8 @@ class cmMakefile;
   F(CMP0179)                                                                  \
   F(CMP0181)                                                                  \
   F(CMP0182)                                                                  \
-  F(CMP0195)
+  F(CMP0195)                                                                  \
+  F(CMP0199)
 
 #define CM_FOR_EACH_CUSTOM_COMMAND_POLICY(F)                                  \
   F(CMP0116)                                                                  \

Tests/RunCMake/GeneratorExpression/CMP0199-NEW.cmake

@@ -0,0 +1,10 @@
+project(test-CMP0199-NEW C)
+
+cmake_policy(SET CMP0199 NEW)
+
+include(CMP0199-cases.cmake)
+
+# FIXME: CMake currently incorrectly selects the RELEASE configuration. See
+# https://gitlab.kitware.com/cmake/cmake/-/issues/27022.
+do_mapped_config_test(EXPECT_RELEASE)
+do_unique_config_test(EXPECT_DEBUG)

Tests/RunCMake/GeneratorExpression/CMP0199-OLD.cmake

@@ -0,0 +1,8 @@
+project(test-CMP0199-OLD C)
+
+cmake_policy(SET CMP0199 OLD)
+
+include(CMP0199-cases.cmake)
+
+do_mapped_config_test(EXPECT_RELEASE EXPECT_DEBUG EXPECT_TEST)
+do_unique_config_test(EXPECT_RELEASE EXPECT_DEBUG)

Tests/RunCMake/GeneratorExpression/CMP0199-WARN-stderr.txt

@@ -0,0 +1,11 @@
+CMake Warning \(dev\) at CMP0199-WARN\.cmake:[0-9]+ \(target_link_libraries\):
+  Policy CMP0199 is not set: \$<CONFIG:cfgs> only matches the configuration of
+  the consumed target\.  Run "cmake --help-policy CMP0199" for policy details\.
+  Use the cmake_policy command to set the policy and suppress this warning\.
+
+  Evaluation of \$<CONFIG> for imported target "lib_test", used by "exe_test",
+  may match multiple configurations\.
+
+Call Stack \(most recent call first\):
+  CMakeLists\.txt:[0-9]+ \(include\)
+This warning is for project developers\.  Use -Wno-dev to suppress it\.

Tests/RunCMake/GeneratorExpression/CMP0199-WARN.cmake

@@ -0,0 +1,11 @@
+project(test-CMP0199-WARN C)
+
+set(CMAKE_POLICY_WARNING_CMP0199 ON)
+
+add_library(lib_test INTERFACE IMPORTED)
+set_target_properties(lib_test PROPERTIES
+  INTERFACE_COMPILE_DEFINITIONS "$<$<CONFIG:release>:RELEASE>"
+)
+
+add_executable(exe_test configtest.c)
+target_link_libraries(exe_test PRIVATE lib_test)

Tests/RunCMake/GeneratorExpression/CMP0199-cases.cmake

@@ -0,0 +1,32 @@
+# Under CMP0199 OLD, $<CONFIG> matches not just the selected configuration, but
+# every entry in MAP_IMPORTED_CONFIG_<CONFIG>. Under NEW, it should only match
+# the selected configuration.
+function(do_mapped_config_test)
+  add_library(lib_mapped INTERFACE IMPORTED)
+  set_target_properties(lib_mapped PROPERTIES
+    IMPORTED_CONFIGURATIONS "TEST"
+    INTERFACE_COMPILE_DEFINITIONS
+      "$<$<CONFIG:debug>:DEBUG>;$<$<CONFIG:release>:RELEASE>;$<$<CONFIG:test>:TEST>"
+    MAP_IMPORTED_CONFIG_RELEASE "RELEASE;DEBUG;TEST"
+  )
+
+  add_executable(exe_mapped configtest.c)
+  target_compile_definitions(exe_mapped PRIVATE ${ARGN})
+  target_link_libraries(exe_mapped PRIVATE lib_mapped)
+endfunction()
+
+# Under CMake CMP0199 OLD, $<CONFIG> matches both the consumer's configuration
+# AND the selected configuration of the library being consumed. Under NEW, it
+# should only match the selected configuration.
+function(do_unique_config_test)
+  add_library(lib_unique INTERFACE IMPORTED)
+  set_target_properties(lib_unique PROPERTIES
+    IMPORTED_CONFIGURATIONS "DEBUG"
+    INTERFACE_COMPILE_DEFINITIONS
+      "$<$<CONFIG:debug>:DEBUG>;$<$<CONFIG:release>:RELEASE>"
+  )
+
+  add_executable(exe_unique configtest.c)
+  target_compile_definitions(exe_unique PRIVATE ${ARGN})
+  target_link_libraries(exe_unique PRIVATE lib_unique)
+endfunction()

Tests/RunCMake/GeneratorExpression/RunCMakeTest.cmake

@@ -48,6 +48,20 @@ run_cmake(FILTER-InvalidOperator)
 run_cmake(FILTER-Exclude)
 run_cmake(FILTER-Include)
 
+function(run_cmake_build test)
+  set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/${test}-build)
+  if (RunCMake_GENERATOR_IS_MULTI_CONFIG)
+    list(APPEND RunCMake_TEST_OPTIONS -DCMAKE_CONFIGURATION_TYPES=Release)
+  else()
+    list(APPEND RunCMake_TEST_OPTIONS -DCMAKE_BUILD_TYPE=Release)
+  endif()
+
+  run_cmake(${test})
+
+  set(RunCMake_TEST_NO_CLEAN TRUE)
+  run_cmake_command(${test}-build ${CMAKE_COMMAND} --build . --config Release)
+endfunction()
+
 function(run_linker_genex test lang type)
   set(options_args CHECK_RESULT EXECUTE)
   cmake_parse_arguments(PARSE_ARGV 3 RLG "${options_args}" "" "")
@@ -103,6 +117,11 @@ else()
   set(RunCMake_TEST_OPTIONS [==[-DCMAKE_BUILD_TYPE=]==])
 endif()
 run_cmake(CONFIG-empty-entries)
+unset(RunCMake_TEST_OPTIONS)
+
+run_cmake(CMP0199-WARN)
+run_cmake_build(CMP0199-OLD)
+run_cmake_build(CMP0199-NEW)
 
 set(RunCMake_TEST_OPTIONS -DCMAKE_POLICY_DEFAULT_CMP0085:STRING=OLD)
 run_cmake(CMP0085-OLD)

Tests/RunCMake/GeneratorExpression/configtest.c

@@ -0,0 +1,34 @@
+#ifdef EXPECT_DEBUG
+#  ifndef DEBUG
+#    error DEBUG should be defined
+#  endif
+#else
+#  ifdef DEBUG
+#    error DEBUG should not be defined
+#  endif
+#endif
+
+#ifdef EXPECT_RELEASE
+#  ifndef RELEASE
+#    error RELEASE should be defined
+#  endif
+#else
+#  ifdef RELEASE
+#    error RELEASE should not be defined
+#  endif
+#endif
+
+#ifdef EXPECT_TEST
+#  ifndef TEST
+#    error TEST should be defined
+#  endif
+#else
+#  ifdef TEST
+#    error TEST should not be defined
+#  endif
+#endif
+
+int main(void)
+{
+  return 0;
+}

Tests/RunCMake/TargetPolicies/PolicyList-stderr.txt

@@ -47,6 +47,7 @@
    \* CMP0181
    \* CMP0182
    \* CMP0195
+   \* CMP0199
 
 Call Stack \(most recent call first\):
   CMakeLists.txt:3 \(include\)