Skip to content

Commit 3e6ea8a

Browse files
ilya-biryukovtstellar
authored andcommitted
[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 (cherry picked from commit 42f87bb)
1 parent 207f96e commit 3e6ea8a

File tree

3 files changed

+31
-7
lines changed

3 files changed

+31
-7
lines changed

clang/include/clang/AST/DeclTemplate.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3287,8 +3287,12 @@ class ConceptDecl : public TemplateDecl, public Mergeable<ConceptDecl> {
32873287
return isa<TemplateTypeParmDecl>(getTemplateParameters()->getParam(0));
32883288
}
32893289

3290-
ConceptDecl *getCanonicalDecl() override { return getFirstDecl(); }
3291-
const ConceptDecl *getCanonicalDecl() const { return getFirstDecl(); }
3290+
ConceptDecl *getCanonicalDecl() override {
3291+
return cast<ConceptDecl>(getPrimaryMergedDecl(this));
3292+
}
3293+
const ConceptDecl *getCanonicalDecl() const {
3294+
return const_cast<ConceptDecl *>(this)->getCanonicalDecl();
3295+
}
32923296

32933297
// Implement isa/cast/dyncast/etc.
32943298
static bool classof(const Decl *D) { return classofKind(D->getKind()); }

clang/lib/Sema/SemaTemplate.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8758,7 +8758,8 @@ void Sema::CheckConceptRedefinition(ConceptDecl *NewDecl,
87588758
// Other decls (e.g. namespaces) also have this shortcoming.
87598759
return;
87608760
}
8761-
Context.setPrimaryMergedDecl(NewDecl, OldConcept);
8761+
// We unwrap canonical decl late to check for module visibility.
8762+
Context.setPrimaryMergedDecl(NewDecl, OldConcept->getCanonicalDecl());
87628763
}
87638764

87648765
/// \brief Strips various properties off an implicit instantiation

clang/test/Modules/merge-concepts.cpp

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@ module "library" {
2828
export *
2929
header "concepts.h"
3030
}
31+
module "compare" {
32+
export *
33+
header "compare.h"
34+
}
3135
module "format" {
3236
export *
3337
header "format.h"
@@ -47,19 +51,34 @@ module "library" {
4751
#define SAME_AS_H
4852

4953
template <class T, class U>
50-
concept same_as = __is_same(T, U);
54+
concept same_as_impl = __is_same(T, U);
5155

56+
template <class T, class U>
57+
concept same_as = same_as_impl<T, U> && same_as_impl<U, T>;
5258
#endif // SAME_AS_H
5359

60+
61+
//--- compare.h
62+
#ifndef COMPARE_H
63+
#define COMPARE_H
64+
65+
#include "same_as.h"
66+
#include "concepts.h"
67+
68+
template <class T> void foo()
69+
requires same_as<T, int>
70+
{}
71+
#endif // COMPARE_H
72+
5473
//--- format.h
5574
#ifndef FORMAT_H
5675
#define FORMAT_H
5776

58-
#include "concepts.h"
5977
#include "same_as.h"
78+
#include "concepts.h"
6079

61-
template <class T> void foo()
80+
template <class T> void bar()
6281
requires same_as<T, int>
6382
{}
6483

65-
#endif // FORMAT_H
84+
#endif // FORMAT_H

0 commit comments

Comments
 (0)