[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
This commit is contained in:
Ilya Biryukov 2022-07-25 12:42:42 +02:00
parent aff68f5ad6
commit 59179d72b2
6 changed files with 219 additions and 9 deletions

View File

@ -5774,6 +5774,8 @@ def warn_forward_class_redefinition : Warning<
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 "

View File

@ -8260,6 +8260,9 @@ public:
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,

View File

@ -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_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) {

View File

@ -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}}

View File

@ -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

View File

@ -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