[clang] 4b95a5a - [Modules] Add ODR Check for concepts

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2022-07-12T23:45:53+08:00
New Revision: 4b95a5a772530f78326941f26e5cb2c33212460f

URL: https://github.com/llvm/llvm-project/commit/4b95a5a772530f78326941f26e5cb2c33212460f
DIFF: https://github.com/llvm/llvm-project/commit/4b95a5a772530f78326941f26e5cb2c33212460f.diff

LOG: [Modules] Add ODR Check for concepts

Closing https://github.com/llvm/llvm-project/issues/56310

Previously we don't check the contents of concept so it might merge
inconsistent results.

Important Note: this patch might break existing code but the behavior
might be right.

Reviewed By: erichkeane

Differential Revision: https://reviews.llvm.org/D129104

Added: 
    clang/test/Modules/concept_differ.cpp
    clang/test/Modules/concept_differ.cppm

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/lib/AST/ASTContext.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f5aecff0d2647..2898a6c1d9339 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -183,6 +183,9 @@ Bug Fixes
   initializer is not allowed this is now diagnosed as an error.
 - Clang now correctly emits symbols for implicitly instantiated constexpr
   template function. Fixes `Issue 55560 <https://github.com/llvm/llvm-project/issues/55560>`_.
+- Clang now checks ODR violations when merging concepts from 
diff erent modules.
+  Note that this may possibly break existing code, and is done so intentionally.
+  Fixes `Issue 56310 <https://github.com/llvm/llvm-project/issues/56310>`_.
 
 Improvements to Clang's diagnostics
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 748fa6ebf1d6c..aa29ce7f13840 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -6561,6 +6561,20 @@ bool ASTContext::isSameEntity(const NamedDecl *X, const NamedDecl *Y) const {
   // and patterns match.
   if (const auto *TemplateX = dyn_cast<TemplateDecl>(X)) {
     const auto *TemplateY = cast<TemplateDecl>(Y);
+
+    // ConceptDecl wouldn't be the same if their constraint expression 
diff ers.
+    if (const auto *ConceptX = dyn_cast<ConceptDecl>(X)) {
+      const auto *ConceptY = cast<ConceptDecl>(Y);
+      const Expr *XCE = ConceptX->getConstraintExpr();
+      const Expr *YCE = ConceptY->getConstraintExpr();
+      assert(XCE && YCE && "ConceptDecl without constraint expression?");
+      llvm::FoldingSetNodeID XID, YID;
+      XCE->Profile(XID, *this, /*Canonical=*/true);
+      YCE->Profile(YID, *this, /*Canonical=*/true);
+      if (XID != YID)
+        return false;
+    }
+
     return isSameEntity(TemplateX->getTemplatedDecl(),
                         TemplateY->getTemplatedDecl()) &&
            isSameTemplateParameterList(TemplateX->getTemplateParameters(),

diff  --git a/clang/test/Modules/concept_
diff er.cpp b/clang/test/Modules/concept_
diff er.cpp
new file mode 100644
index 0000000000000..23c7d4c5ecf9a
--- /dev/null
+++ b/clang/test/Modules/concept_
diff er.cpp
@@ -0,0 +1,35 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -x c++ -std=c++20 -fmodules -fmodules-cache-path=%t -fmodule-map-file=%t/module.map %t/foo.cpp -verify
+
+//--- module.map
+module "foo" {
+  export * 
+  header "foo.h"
+}
+module "bar" {
+  export * 
+  header "bar.h"
+}
+
+//--- foo.h
+template <class T>
+concept A = true;
+
+//--- bar.h
+template <class T>
+concept A = false;
+
+//--- foo.cpp
+#include "bar.h"
+#include "foo.h"
+
+template <class T> void foo() requires A<T> {}  // expected-error 1+{{reference to 'A' is ambiguous}}
+                                                // expected-note@* 1+{{candidate found by name lookup}}
+
+int main() {
+  foo<int>();
+  return 0;
+}

diff  --git a/clang/test/Modules/concept_
diff er.cppm b/clang/test/Modules/concept_
diff er.cppm
new file mode 100644
index 0000000000000..ccb29d26e53d1
--- /dev/null
+++ b/clang/test/Modules/concept_
diff er.cppm
@@ -0,0 +1,39 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -x c++ -std=c++20 %t/A.cppm -I%t -emit-module-interface -o %t/A.pcm
+// RUN: %clang_cc1 -x c++ -std=c++20 %t/B.cppm -I%t -emit-module-interface -o %t/B.pcm
+// RUN: %clang_cc1 -x c++ -std=c++20 -fprebuilt-module-path=%t %t/foo.cpp -verify
+
+//--- foo.h
+template <class T>
+concept A = true;
+
+//--- bar.h
+template <class T>
+concept A = false;
+
+//--- A.cppm
+module;
+#include "foo.h"
+export module A;
+export using ::A;
+
+//--- B.cppm
+module;
+#include "bar.h"
+export module B;
+export using ::A;
+
+//--- foo.cpp
+import A;
+import B;
+
+template <class T> void foo() requires A<T> {}  // expected-error 1+{{reference to 'A' is ambiguous}}
+                                                // expected-note@* 1+{{candidate found by name lookup}}
+
+int main() {
+  foo<int>();
+  return 0;
+}