[clang] 59179d7 - [Sema] Merge C++20 concept definitions from different modules in same TU
Ilya Biryukov via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 25 05:53:04 PDT 2022
Author: Ilya Biryukov
Date: 2022-07-25T14:43:38+02:00
New Revision: 59179d72b2e3d3b99ebc342374c9c797d526ac5d
URL: https://github.com/llvm/llvm-project/commit/59179d72b2e3d3b99ebc342374c9c797d526ac5d
DIFF: https://github.com/llvm/llvm-project/commit/59179d72b2e3d3b99ebc342374c9c797d526ac5d.diff
LOG: [Sema] Merge C++20 concept definitions from different modules in same TU
Currently the C++20 concepts are only merged in `ASTReader`, i.e. when
coming from different TU. This can causes ambiguious reference errors when
trying to access the same concept that should otherwise be merged.
Please see the added test for an example.
Note that we currently use `ASTContext::isSameEntity` to check for ODR
violations. However, it will not check that concept requirements match.
The same issue holds for mering concepts from different TUs, I added a
FIXME and filed a GH issue to track this:
https://github.com/llvm/llvm-project/issues/56310
Reviewed By: ChuanqiXu
Differential Revision: https://reviews.llvm.org/D128921
Added:
clang/test/Modules/merge-concepts-cxx-modules.cpp
clang/test/Modules/merge-concepts-redefinition-error.cpp
clang/test/Modules/merge-concepts.cpp
Modified:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaTemplate.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c2cabcb04f2f7..6ff5b8de57fd0 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5774,6 +5774,8 @@ def warn_forward_class_redefinition : Warning<
def err_redefinition_
diff erent_typedef : Error<
"%select{typedef|type alias|type alias template}0 "
"redefinition with
diff erent types%
diff { ($ vs $)|}1,2">;
+def err_redefinition_
diff erent_concept : Error<
+ "redefinition of concept %0 with
diff erent template parameters or requirements">;
def err_tag_reference_non_tag : Error<
"%select{non-struct type|non-class type|non-union type|non-enum "
"type|typedef|type alias|template|type alias template|template "
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index de9bde6841f7d..b149c24dea7dc 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -8260,6 +8260,9 @@ class Sema final {
Scope *S, MultiTemplateParamsArg TemplateParameterLists,
IdentifierInfo *Name, SourceLocation NameLoc, Expr *ConstraintExpr);
+ void CheckConceptRedefinition(ConceptDecl *NewDecl, LookupResult &Previous,
+ bool &AddToScope);
+
RequiresExprBodyDecl *
ActOnStartRequiresExpr(SourceLocation RequiresKWLoc,
ArrayRef<ParmVarDecl *> LocalParameters,
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 95c83ebfaeab5..1542a07713fb7 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -20,6 +20,7 @@
#include "clang/AST/TemplateName.h"
#include "clang/AST/TypeVisitor.h"
#include "clang/Basic/Builtins.h"
+#include "clang/Basic/DiagnosticSema.h"
#include "clang/Basic/LangOptions.h"
#include "clang/Basic/PartialDiagnostic.h"
#include "clang/Basic/Stack.h"
@@ -8707,23 +8708,59 @@ Decl *Sema::ActOnConceptDefinition(Scope *S,
// Check for conflicting previous declaration.
DeclarationNameInfo NameInfo(NewDecl->getDeclName(), NameLoc);
LookupResult Previous(*this, NameInfo, LookupOrdinaryName,
- ForVisibleRedeclaration);
+ forRedeclarationInCurContext());
LookupName(Previous, S);
-
FilterLookupForScope(Previous, DC, S, /*ConsiderLinkage=*/false,
/*AllowInlineNamespace*/false);
- if (!Previous.empty()) {
- auto *Old = Previous.getRepresentativeDecl();
- Diag(NameLoc, isa<ConceptDecl>(Old) ? diag::err_redefinition :
- diag::err_redefinition_
diff erent_kind) << NewDecl->getDeclName();
- Diag(Old->getLocation(), diag::note_previous_definition);
- }
+ bool AddToScope = true;
+ CheckConceptRedefinition(NewDecl, Previous, AddToScope);
ActOnDocumentableDecl(NewDecl);
- PushOnScopeChains(NewDecl, S);
+ if (AddToScope)
+ PushOnScopeChains(NewDecl, S);
return NewDecl;
}
+void Sema::CheckConceptRedefinition(ConceptDecl *NewDecl,
+ LookupResult &Previous, bool &AddToScope) {
+ AddToScope = true;
+
+ if (Previous.empty())
+ return;
+
+ auto *OldConcept = dyn_cast<ConceptDecl>(Previous.getRepresentativeDecl());
+ if (!OldConcept) {
+ auto *Old = Previous.getRepresentativeDecl();
+ Diag(NewDecl->getLocation(), diag::err_redefinition_
diff erent_kind)
+ << NewDecl->getDeclName();
+ notePreviousDefinition(Old, NewDecl->getLocation());
+ AddToScope = false;
+ return;
+ }
+ // Check if we can merge with a concept declaration.
+ bool IsSame = Context.isSameEntity(NewDecl, OldConcept);
+ if (!IsSame) {
+ Diag(NewDecl->getLocation(), diag::err_redefinition_
diff erent_concept)
+ << NewDecl->getDeclName();
+ notePreviousDefinition(OldConcept, NewDecl->getLocation());
+ AddToScope = false;
+ return;
+ }
+ if (hasReachableDefinition(OldConcept)) {
+ Diag(NewDecl->getLocation(), diag::err_redefinition)
+ << NewDecl->getDeclName();
+ notePreviousDefinition(OldConcept, NewDecl->getLocation());
+ AddToScope = false;
+ return;
+ }
+ if (!Previous.isSingleResult()) {
+ // FIXME: we should produce an error in case of ambig and failed lookups.
+ // Other decls (e.g. namespaces) also have this shortcoming.
+ return;
+ }
+ Context.setPrimaryMergedDecl(NewDecl, OldConcept);
+}
+
/// \brief Strips various properties off an implicit instantiation
/// that has just been explicitly specialized.
static void StripImplicitInstantiation(NamedDecl *D) {
diff --git a/clang/test/Modules/merge-concepts-cxx-modules.cpp b/clang/test/Modules/merge-concepts-cxx-modules.cpp
new file mode 100644
index 0000000000000..3d4f8435531a8
--- /dev/null
+++ b/clang/test/Modules/merge-concepts-cxx-modules.cpp
@@ -0,0 +1,46 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/same_as.cppm -o %t/same_as.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -fprebuilt-module-path=%t %t/concepts.cppm -o %t/concepts.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -fprebuilt-module-path=%t %t/format.cppm -o %t/format.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/conflicting.cppm -o %t/conflicting.pcm
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/Use.cppm -fsyntax-only -verify
+
+//--- same_as.cppm
+export module same_as;
+export template <class T, class U>
+concept same_as = __is_same(T, U);
+
+//--- concepts.cppm
+export module concepts;
+export import same_as;
+
+export template <class T>
+concept ConflictingConcept = true;
+
+//--- format.cppm
+
+export module format;
+export import concepts;
+export import same_as;
+
+export template <class T> void foo()
+ requires same_as<T, int>
+{}
+
+//--- conflicting.cppm
+export module conflicting;
+export template <class T, class U = int>
+concept ConflictingConcept = true;
+
+//--- Use.cppm
+import format;
+import conflicting;
+
+template <class T> void foo()
+ requires same_as<T, T>
+{}
+ConflictingConcept auto x = 10; // expected-error {{reference to 'ConflictingConcept' is ambiguous}}
+ // expected-note@* 2 {{candidate}}
diff --git a/clang/test/Modules/merge-concepts-redefinition-error.cpp b/clang/test/Modules/merge-concepts-redefinition-error.cpp
new file mode 100644
index 0000000000000..844a4833bf7c2
--- /dev/null
+++ b/clang/test/Modules/merge-concepts-redefinition-error.cpp
@@ -0,0 +1,57 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -xc++ -std=c++20 -fmodules -fmodule-name=library \
+// RUN: -emit-module %t/modules.map \
+// RUN: -o %t/module.pcm \
+// RUN: -verify
+//
+//--- modules.map
+module "library" {
+ export *
+ module "concepts" {
+ export *
+ header "concepts.h"
+ }
+ module "conflicting" {
+ export *
+ header "conflicting.h"
+ }
+}
+
+//--- concepts.h
+#ifndef CONCEPTS_H_
+#define CONCEPTS_H_
+
+template <class T>
+concept ConflictingConcept = true;
+
+template <class T, class U>
+concept same_as = __is_same(T, U);
+
+template<class T> concept truec = true;
+
+int var;
+
+#endif // SAMEAS_CONCEPTS_H
+
+//--- conflicting.h
+#ifndef CONFLICTING_H
+#define CONFLICTING_H
+
+#include "concepts.h"
+
+template <class T, class U = int>
+concept ConflictingConcept = true; // expected-error {{redefinition of concept 'ConflictingConcept' with
diff erent template}}
+ // expected-note@* {{previous definition}}
+
+int same_as; // expected-error {{redefinition of 'same_as' as
diff erent kind of symbol}}
+ // expected-note@* {{previous definition}}
+
+template<class T> concept var = false; // expected-error {{redefinition of 'var' as
diff erent kind of symbol}}
+ // expected-note@* {{previous definition}}
+
+template<class T> concept truec = true; // expected-error {{redefinition of 'truec'}}
+ // expected-note@* {{previous definition}}
+#endif // CONFLICTING_H
diff --git a/clang/test/Modules/merge-concepts.cpp b/clang/test/Modules/merge-concepts.cpp
new file mode 100644
index 0000000000000..9242f27633795
--- /dev/null
+++ b/clang/test/Modules/merge-concepts.cpp
@@ -0,0 +1,65 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -xc++ -std=c++20 -fmodules -fmodule-name=library \
+// RUN: -emit-module %t/modules.map \
+// RUN: -o %t/module.pcm
+//
+//
+// RUN: %clang_cc1 -xc++ -std=c++20 -fmodules -fmodule-file=%t/module.pcm \
+// RUN: -fmodule-map-file=%t/modules.map \
+// RUN: -fsyntax-only -verify %t/use.cpp
+//
+//--- use.cpp
+// expected-no-diagnostics
+
+#include "concepts.h"
+#include "format.h"
+
+template <class T> void foo()
+ requires same_as<T, T>
+{}
+
+//--- modules.map
+module "library" {
+ export *
+ module "concepts" {
+ export *
+ header "concepts.h"
+ }
+ module "format" {
+ export *
+ header "format.h"
+ }
+}
+
+//--- concepts.h
+#ifndef SAMEAS_CONCEPTS_H_
+#define SAMEAS_CONCEPTS_H_
+
+#include "same_as.h"
+
+#endif // SAMEAS_CONCEPTS_H
+
+//--- same_as.h
+#ifndef SAME_AS_H
+#define SAME_AS_H
+
+template <class T, class U>
+concept same_as = __is_same(T, U);
+
+#endif // SAME_AS_H
+
+//--- format.h
+#ifndef FORMAT_H
+#define FORMAT_H
+
+#include "concepts.h"
+#include "same_as.h"
+
+template <class T> void foo()
+ requires same_as<T, int>
+{}
+
+#endif // FORMAT_H
\ No newline at end of file
More information about the cfe-commits
mailing list