ilya-biryukov updated this revision to Diff 446883.
ilya-biryukov added a comment.
- Update diagnostics per Richard's suggestions, add tests
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128921/new/
https://reviews.llvm.org/D128921
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
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-redefinition-error.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,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
Index: clang/test/Modules/merge-concepts-redefinition-error.cpp
===================================================================
--- /dev/null
+++ 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 different template}}
+ // expected-note@* {{previous definition}}
+
+int same_as; // expected-error {{redefinition of 'same_as' as different kind of symbol}}
+ // expected-note@* {{previous definition}}
+
+template<class T> concept var = false; // expected-error {{redefinition of 'var' as different 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
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
@@ -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 @@
// 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;
+
+ auto *OldConcept = dyn_cast<ConceptDecl>(Previous.getRepresentativeDecl());
+ if (!OldConcept) {
+ auto *Old = Previous.getRepresentativeDecl();
+ Diag(NewDecl->getLocation(), diag::err_redefinition_different_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_different_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) {
Index: clang/lib/AST/ASTContext.cpp
===================================================================
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -6561,6 +6561,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
@@ -8255,6 +8255,8 @@
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,
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5770,6 +5770,8 @@
def err_redefinition_different_typedef : Error<
"%select{typedef|type alias|type alias template}0 "
"redefinition with different types%diff{ ($ vs $)|}1,2">;
+def err_redefinition_different_concept : Error<
+ "redefinition of concept %0 with different 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 "
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
