ilya-biryukov updated this revision to Diff 442623.
ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added a comment.
- Update code to match how typedefs behave
- remove leftover test from previous version
- Add test for C++20 modules, rewrite original test with split-file
- use isReachable, do not call makeMergedDefinitionVisible
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128921/new/
https://reviews.llvm.org/D128921
Files:
clang/include/clang/Sema/Sema.h
clang/lib/AST/ASTContext.cpp
clang/lib/Sema/SemaTemplate.cpp
clang/test/Modules/merge-concepts-cxx-modules.cpp
clang/test/Modules/merge-concepts.cpp
Index: clang/test/Modules/merge-concepts.cpp
===================================================================
--- /dev/null
+++ clang/test/Modules/merge-concepts.cpp
@@ -0,0 +1,83 @@
+// 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
+
+#include "concepts.h"
+#include "conflicting.h"
+#include "format.h"
+
+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}}
+
+//--- modules.map
+module "library" {
+ export *
+ module "concepts" {
+ export *
+ header "concepts.h"
+ }
+ module "format" {
+ export *
+ header "format.h"
+ }
+ module "conflicting" {
+ export *
+ header "conflicting.h"
+ }
+}
+
+//--- concepts.h
+#ifndef SAMEAS_CONCEPTS_H_
+#define SAMEAS_CONCEPTS_H_
+
+#include "same_as.h"
+
+template <class T>
+concept ConflictingConcept = true;
+
+#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
+
+//--- conflicting.h
+#ifndef CONFLICTING_H
+#define CONFLICTING_H
+
+template <class T, class U = int>
+concept ConflictingConcept = true;
+
+#endif // CONFLICTING_H
Index: clang/test/Modules/merge-concepts-cxx-modules.cpp
===================================================================
--- /dev/null
+++ 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}}
Index: clang/lib/Sema/SemaTemplate.cpp
===================================================================
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -8656,23 +8656,52 @@
// 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_different_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;
+ // Check if there is a concept declaration we can merge with.
+ auto *PrevDef =
+ Previous.isSingleResult() ? Previous.getAsSingle<ConceptDecl>() : nullptr;
+ if (PrevDef && !hasReachableDefinition(PrevDef) &&
+ Context.isSameEntity(NewDecl, PrevDef)) {
+ Context.setPrimaryMergedDecl(NewDecl, PrevDef);
+ AddToScope = false;
+ return;
+ }
+ // Filter out non-visible declaration to avoid spurious redefinition errors.
+ auto F = Previous.makeFilter();
+ while (F.hasNext()) {
+ if (!isReachable(F.next())) {
+ F.erase();
+ }
+ }
+ F.done();
+ // We report redefinition if the lookup is not empty after filters.
+ if (Previous.empty())
+ return;
+ auto *Old = Previous.getRepresentativeDecl();
+ Diag(NewDecl->getLocation(), isa<ConceptDecl>(Old)
+ ? diag::err_redefinition
+ : diag::err_redefinition_different_kind)
+ << NewDecl->getDeclName();
+ Diag(Old->getLocation(), diag::note_previous_definition);
+}
+
/// \brief Strips various properties off an implicit instantiation
/// that has just been explicitly specialized.
static void StripImplicitInstantiation(NamedDecl *D) {
Index: clang/lib/AST/ASTContext.cpp
===================================================================
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -6524,6 +6524,7 @@
// and patterns match.
if (const auto *TemplateX = dyn_cast<TemplateDecl>(X)) {
const auto *TemplateY = cast<TemplateDecl>(Y);
+ // FIXME: for C++20 concepts, check their requirements are the same.
return isSameEntity(TemplateX->getTemplatedDecl(),
TemplateY->getTemplatedDecl()) &&
isSameTemplateParameterList(TemplateX->getTemplateParameters(),
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -8247,6 +8247,9 @@
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,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
