[clang] 42f87bb - [Sema] Return primary merged decl as canonical for concepts

[Ilya Biryukov via cfe-commits]

Author: Ilya Biryukov
Date: 2022-07-27T12:31:20+02:00
New Revision: 42f87bb62d0719848842da60d2a8180b9e4d7c52

URL: https://github.com/llvm/llvm-project/commit/42f87bb62d0719848842da60d2a8180b9e4d7c52
DIFF: https://github.com/llvm/llvm-project/commit/42f87bb62d0719848842da60d2a8180b9e4d7c52.diff

LOG: [Sema] Return primary merged decl as canonical for concepts

Otherwise we get invalid results for ODR checks. See changed test for an
example: despite the fact that we merge the first concept, its **uses**
were considered different by `Profile`, leading to redefinition errors.

After this change, canonical decl for a concept can come from a
different module and may not be visible. This behavior looks suspicious,
but does not break any tests. We might want to add a mechanism to make
the canonical concept declaration visible if we find code that relies on
this invariant.

Additionally make sure we always merge with the canonical declaration to
avoid chains of merged concepts being reported as redefinitions. An
example was added to the test.

Also change the order of includes in the test. Importing a moduralized
header before its textual part causes the include guard macro to be
exported and the corresponding `#include` becomes a no-op.

Reviewed By: ChuanqiXu

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

Added: 
    

Modified: 
    clang/include/clang/AST/DeclTemplate.h
    clang/lib/Sema/SemaTemplate.cpp
    clang/test/Modules/merge-concepts.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/AST/DeclTemplate.h b/clang/include/clang/AST/DeclTemplate.h
index 725bb0bced9c5..baed5ca22fa75 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -3287,8 +3287,12 @@ class ConceptDecl : public TemplateDecl, public Mergeable<ConceptDecl> {
     return isa<TemplateTypeParmDecl>(getTemplateParameters()->getParam(0));
   }
 
-  ConceptDecl *getCanonicalDecl() override { return getFirstDecl(); }
-  const ConceptDecl *getCanonicalDecl() const { return getFirstDecl(); }
+  ConceptDecl *getCanonicalDecl() override {
+    return cast<ConceptDecl>(getPrimaryMergedDecl(this));
+  }
+  const ConceptDecl *getCanonicalDecl() const {
+    return const_cast<ConceptDecl *>(this)->getCanonicalDecl();
+  }
 
   // Implement isa/cast/dyncast/etc.
   static bool classof(const Decl *D) { return classofKind(D->getKind()); }

diff  --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 8f887ce7829fa..c0bec60f737c8 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -8754,7 +8754,8 @@ void Sema::CheckConceptRedefinition(ConceptDecl *NewDecl,
     //        Other decls (e.g. namespaces) also have this shortcoming.
     return;
   }
-  Context.setPrimaryMergedDecl(NewDecl, OldConcept);
+  // We unwrap canonical decl late to check for module visibility.
+  Context.setPrimaryMergedDecl(NewDecl, OldConcept->getCanonicalDecl());
 }
 
 /// \brief Strips various properties off an implicit instantiation

diff  --git a/clang/test/Modules/merge-concepts.cpp b/clang/test/Modules/merge-concepts.cpp
index 9242f27633795..c774157dd85b4 100644
--- a/clang/test/Modules/merge-concepts.cpp
+++ b/clang/test/Modules/merge-concepts.cpp
@@ -28,6 +28,10 @@ module "library" {
 		export *
 		header "concepts.h"
 	}
+	module "compare" {
+		export *
+		header "compare.h"
+	}
 	module "format" {
 		export *
 		header "format.h"
@@ -47,19 +51,34 @@ module "library" {
 #define SAME_AS_H
 
 template <class T, class U>
-concept same_as = __is_same(T, U);
+concept same_as_impl = __is_same(T, U);
 
+template <class T, class U>
+concept same_as = same_as_impl<T, U> && same_as_impl<U, T>;
 #endif // SAME_AS_H
 
+
+//--- compare.h
+#ifndef COMPARE_H
+#define COMPARE_H
+
+#include "same_as.h"
+#include "concepts.h"
+
+template <class T> void foo()
+  requires same_as<T, int>
+{}
+#endif // COMPARE_H
+
 //--- format.h
 #ifndef FORMAT_H
 #define FORMAT_H
 
-#include "concepts.h"
 #include "same_as.h"
+#include "concepts.h"
 
-template <class T> void foo()
+template <class T> void bar()
   requires same_as<T, int>
 {}
 
-#endif // FORMAT_H
\ No newline at end of file
+#endif // FORMAT_H