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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
