[PATCH] D114769: [C++20] [Modules] [Concepts] Recognize same concepts more precisely in Serialization

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu created this revision.
ChuanqiXu added reviewers: rsmith, aaron.ballman, urnathan, saar.raz.
ChuanqiXu added a project: clang.
ChuanqiXu requested review of this revision.
Herald added a subscriber: cfe-commits.

When we use concept in modules, we would meet a curious error. For example:

  C++
  // foo.h
  #ifndef FOO_H
  #define FOO_H
  
  template< class T >
  concept Range = requires(T& t) {
    t.begin();
  };
  
  namespace workspace {
      struct A {
          public:
              template <Range T>
              using range_type = T;
      };
  }
  
  #endif
  // A.cppm
  module;
  #include "foo.h"
  export module A;
  // B.cppm
  module;
  #include "foo.h"
  export module B;
  import A;

The compiler would tell us that the definition of `workspace::A::range_type` in B.cppm is not the same with the one in A.cppm!
The reason here is that the implementation would judge two concepts is same by their addresses. However, when we use modules, the addresses wouldn't be the same all the time since one is parsed in their TU and another is imported in another TU.
This patch fixes this by using `isSameEntity` to judge the two concepts. I think it should be a good fix.

Test Plan: check-all


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114769

Files:
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/test/Modules/Inputs/concept/A.cppm
  clang/test/Modules/Inputs/concept/foo.h
  clang/test/Modules/concept.cppm


Index: clang/test/Modules/concept.cppm
===================================================================
--- /dev/null
+++ clang/test/Modules/concept.cppm
@@ -0,0 +1,10 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: %clang -std=c++20 %S/Inputs/concept/A.cppm --precompile -o %t/A.pcm
+// RUN: %clang -std=c++20 -fprebuilt-module-path=%t -I%S/Inputs/concept %s -c -Xclang -verify
+// expected-no-diagnostics
+
+module;
+#include "foo.h"
+export module B;
+import A;
Index: clang/test/Modules/Inputs/concept/foo.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/concept/foo.h
@@ -0,0 +1,19 @@
+#ifndef FOO_H
+#define FOO_H
+
+template< class T >
+concept Range = requires(T& t) {
+  t.begin();
+};
+
+namespace workspace {
+    struct A {
+        struct B {
+            public:
+                template <Range T>
+                using range_type = T;
+        };
+    };
+}
+
+#endif
Index: clang/test/Modules/Inputs/concept/A.cppm
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/concept/A.cppm
@@ -0,0 +1,3 @@
+module;
+#include "foo.h"
+export module A;
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===================================================================
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2948,6 +2948,7 @@
 static bool isSameTemplateParameterList(const ASTContext &C,
                                         const TemplateParameterList *X,
                                         const TemplateParameterList *Y);
+static bool isSameEntity(NamedDecl *X, NamedDecl *Y);
 
 /// Determine whether two template parameters are similar enough
 /// that they may be used in declarations of the same template.
@@ -2967,7 +2968,9 @@
     if (!TXTC != !TYTC)
       return false;
     if (TXTC && TYTC) {
-      if (TXTC->getNamedConcept() != TYTC->getNamedConcept())
+      ConceptDecl *CDX = TXTC->getNamedConcept();
+      ConceptDecl *CDY = TYTC->getNamedConcept();
+      if (!isSameEntity(CDX, CDY))
         return false;
       if (TXTC->hasExplicitTemplateArgs() != TYTC->hasExplicitTemplateArgs())
         return false;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D114769.390568.patch
Type: text/x-patch
Size: 2237 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20211130/eaf160a5/attachment-0001.bin>