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

Chuanqi Xu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 29 22:13:08 PST 2021

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:

  // foo.h
  #ifndef FOO_H
  #define FOO_H
  template< class T >
  concept Range = requires(T& t) {
  namespace workspace {
      struct A {
              template <Range T>
              using range_type = T;
  // A.cppm
  #include "foo.h"
  export module A;
  // B.cppm
  #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

  rG LLVM Github Monorepo



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
+#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;
+        };
+    };
Index: clang/test/Modules/Inputs/concept/A.cppm
--- /dev/null
+++ clang/test/Modules/Inputs/concept/A.cppm
@@ -0,0 +1,3 @@
+#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>

More information about the cfe-commits mailing list