[llvm-branch-commits] [clang] [clang] correctly handle +/- features when matching modules (PR #187624)

[Florian Mayer via llvm-branch-commits]

https://github.com/fmayer updated https://github.com/llvm/llvm-project/pull/187624

>From cc0cccbce3d1a7a7ace63ddfe30e7313735209a0 Mon Sep 17 00:00:00 2001
From: Florian Mayer <fmayer at google.com>
Date: Fri, 20 Mar 2026 10:59:11 -0700
Subject: [PATCH] improve

Created using spr 1.3.7
---
 clang/lib/Serialization/ASTReader.cpp        | 56 ++++++++++----------
 clang/test/Modules/merge-target-features.cpp |  5 +-
 2 files changed, 31 insertions(+), 30 deletions(-)

diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 6738c19880b11..a696f4fe23312 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -454,6 +454,28 @@ static bool checkCodegenOptions(const CodeGenOptions &CGOpts,
   return false;
 }
 
+static std::set<StringRef>
+accumulateFeaturesAsWritten(const std::vector<std::string> &FeaturesAsWritten) {
+  std::set<StringRef> Out;
+  for (StringRef Name : FeaturesAsWritten) {
+    if (Name.size() < 2)
+      continue;
+    StringRef Feature = Name.substr(1);
+    if (Name[0] == '+') {
+      Out.emplace(Name);
+      std::string Inverse = (std::string("-") + Feature).str();
+      if (auto It = Out.find(Inverse); It != Out.end())
+        Out.erase(It);
+    } else if (Name[0] == '-') {
+      Out.emplace(Name);
+      std::string Inverse = (std::string("+") + Feature).str();
+      if (auto It = Out.find(Inverse); It != Out.end())
+        Out.erase(It);
+    }
+  }
+  return Out;
+}
+
 /// Compare the given set of target options against an existing set of
 /// target options.
 ///
@@ -489,30 +511,10 @@ static bool checkTargetOptions(const TargetOptions &TargetOpts,
 #undef CHECK_TARGET_OPT
 
   // Compare feature sets.
-  std::set<StringRef> ExistingFeatures;
-  std::set<StringRef> ReadFeatures;
-
-  for (StringRef Name : ExistingTargetOpts.FeaturesAsWritten) {
-    if (Name.size() < 2)
-      continue;
-    StringRef Feature = Name.substr(1);
-    if (Name[0] == '+')
-      ExistingFeatures.emplace(Feature);
-    else if (Name[0] == '-')
-      if (auto It = ExistingFeatures.find(Feature);
-          It != ExistingFeatures.end())
-        ExistingFeatures.erase(Feature);
-  }
-  for (StringRef Name : TargetOpts.FeaturesAsWritten) {
-    if (Name.size() < 2)
-      continue;
-    StringRef Feature = Name.substr(1);
-    if (Name[0] == '+')
-      ReadFeatures.emplace(Feature);
-    else if (Name[0] == '-')
-      if (auto It = ReadFeatures.find(Feature); It != ReadFeatures.end())
-        ReadFeatures.erase(Feature);
-  }
+  std::set<StringRef> ExistingFeatures =
+      accumulateFeaturesAsWritten(ExistingTargetOpts.FeaturesAsWritten);
+  std::set<StringRef> ReadFeatures =
+      accumulateFeaturesAsWritten(TargetOpts.FeaturesAsWritten);
 
   // We compute the set difference in both directions explicitly so that we can
   // diagnose the differences differently.
@@ -532,12 +534,10 @@ static bool checkTargetOptions(const TargetOptions &TargetOpts,
   if (Diags) {
     for (StringRef Feature : UnmatchedReadFeatures)
       Diags->Report(diag::err_ast_file_targetopt_feature_mismatch)
-          << /* is-existing-feature */ false << ModuleFilename
-          << (std::string("+") + Feature.str());
+          << /* is-existing-feature */ false << ModuleFilename << Feature;
     for (StringRef Feature : UnmatchedExistingFeatures)
       Diags->Report(diag::err_ast_file_targetopt_feature_mismatch)
-          << /* is-existing-feature */ true << ModuleFilename
-          << (std::string("+") + Feature.str());
+          << /* is-existing-feature */ true << ModuleFilename << Feature;
   }
 
   return !UnmatchedReadFeatures.empty() || !UnmatchedExistingFeatures.empty();
diff --git a/clang/test/Modules/merge-target-features.cpp b/clang/test/Modules/merge-target-features.cpp
index f662c36dade4e..c3678cbea11cb 100644
--- a/clang/test/Modules/merge-target-features.cpp
+++ b/clang/test/Modules/merge-target-features.cpp
@@ -100,7 +100,7 @@
 // RUN:   | FileCheck --allow-empty --check-prefix=DOUBLE %s
 // DOUBLE-NOT: error:
 //
-// RUN: %clang_cc1 -fmodules -x c++ -fmodules-cache-path=%t \
+// RUN: not %clang_cc1 -fmodules -x c++ -fmodules-cache-path=%t \
 // RUN:   -iquote Inputs/merge-target-features \
 // RUN:   -fno-implicit-modules \
 // RUN:   -fmodule-map-file-home-is-cwd \
@@ -110,7 +110,8 @@
 // RUN:   -target-cpu i386 \
 // RUN:   -fsyntax-only merge-target-features.cpp 2>&1 \
 // RUN:   | FileCheck --allow-empty --check-prefix=PLUSMINUS %s
-// PLUSMINUS-NOT: error:
+// PLUSMINUS: error: precompiled file '{{.*}}foo-plusminus.pcm' was compiled with the target feature '-sse2' but the current translation unit is not
+// PLUSMINUS: error: {{.*}} configuration mismatch
 //
 // RUN: %clang_cc1 -fmodules -x c++ -fmodules-cache-path=%t \
 // RUN:   -iquote Inputs/merge-target-features \