ilya-biryukov created this revision.
ilya-biryukov added a reviewer: erichkeane.
Herald added a project: All.
ilya-biryukov requested review of this revision.
Herald added a project: clang.
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
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D128921
Files:
clang/lib/AST/ASTContext.cpp
clang/lib/Sema/SemaTemplate.cpp
clang/test/Modules/Inputs/merge-concepts/concepts.h
clang/test/Modules/Inputs/merge-concepts/format.h
clang/test/Modules/Inputs/merge-concepts/modules.map
clang/test/Modules/Inputs/merge-concepts/same_as.h
clang/test/Modules/merge-concepts.m
Index: clang/test/Modules/merge-concepts.m
===================================================================
--- /dev/null
+++ clang/test/Modules/merge-concepts.m
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t.dir
+// RUN: mkdir %t.dir
+//
+// RUN: %clang_cc1 -xc++ -std=c++20 -fmodules -fmodule-name=library \
+// RUN: -emit-module %S/Inputs/merge-concepts/modules.map \
+// RUN: -o %t.dir/module.pcm
+//
+// Note that this file merely checks the module compiles. The contents of this
+// file are never read by the compiler.
Index: clang/test/Modules/Inputs/merge-concepts/same_as.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/merge-concepts/same_as.h
@@ -0,0 +1,7 @@
+#ifndef SAME_AS_H
+#define SAME_AS_H
+
+template <class T, class U>
+concept same_as = __is_same(T, U);
+
+#endif // SAME_AS_H
Index: clang/test/Modules/Inputs/merge-concepts/modules.map
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/merge-concepts/modules.map
@@ -0,0 +1,11 @@
+module "library" {
+ export *
+ module "concepts" {
+ export *
+ header "concepts.h"
+ }
+ module "format" {
+ export *
+ header "format.h"
+ }
+}
Index: clang/test/Modules/Inputs/merge-concepts/format.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/merge-concepts/format.h
@@ -0,0 +1,11 @@
+#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
Index: clang/test/Modules/Inputs/merge-concepts/concepts.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/merge-concepts/concepts.h
@@ -0,0 +1,6 @@
+#ifndef SAMEAS_CONCEPTS_H_
+#define SAMEAS_CONCEPTS_H_
+
+#include "same_as.h"
+
+#endif // SAMEAS_CONCEPTS_H
Index: clang/lib/Sema/SemaTemplate.cpp
===================================================================
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -8655,12 +8655,21 @@
// 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.isSingleResult() ? Previous.getAsSingle<ConceptDecl>() : nullptr;
+ bool AddToScope = true;
+ // Check for redefinition and merge with decl from other module if needed.
+ if (Old && !hasVisibleDefinition(Old) && Context.isSameEntity(NewDecl, Old)) {
+ Context.setPrimaryMergedDecl(NewDecl, Old);
+ makeMergedDefinitionVisible(Old);
+ AddToScope = false;
+ } else if (!Previous.empty()) {
auto *Old = Previous.getRepresentativeDecl();
Diag(NameLoc, isa<ConceptDecl>(Old) ? diag::err_redefinition :
diag::err_redefinition_different_kind) << NewDecl->getDeclName();
@@ -8668,7 +8677,8 @@
}
ActOnDocumentableDecl(NewDecl);
- PushOnScopeChains(NewDecl, S);
+ if (AddToScope)
+ PushOnScopeChains(NewDecl, S);
return NewDecl;
}
Index: clang/lib/AST/ASTContext.cpp
===================================================================
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -6512,6 +6512,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(),
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
