diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 08337e88c21db7bac6c910801ad3d6e0f9a15566..de623ca9b035c005e7415a83a4780be53095f775 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -2986,9 +2986,6 @@ def note_ovl_candidate : Note<"candidate " "%select{none|const|restrict|const and restrict|volatile|const and volatile" "|volatile and restrict|const, volatile, and restrict}4)" "| made ineligible by enable_if}2">; -def ext_ovl_equivalent_internal_linkage_functions_in_modules : ExtWarn< - "ambiguous use of internal linkage function %0 defined in multiple modules">, - InGroup<DiagGroup<"modules-ambiguous-internal-linkage-function">>; def note_ovl_candidate_inherited_constructor : Note<"inherited from here">; def note_ovl_candidate_illegal_constructor : Note< @@ -7869,6 +7866,12 @@ def err_module_self_import : Error< "import of module '%0' appears within same top-level module '%1'">; def err_module_import_in_implementation : Error< "@import of module '%0' in implementation of '%1'; use #import">; + +def ext_equivalent_internal_linkage_decl_in_modules : ExtWarn< + "ambiguous use of internal linkage declaration %0 defined in multiple modules">, + InGroup<DiagGroup<"modules-ambiguous-internal-linkage">>; +def note_equivalent_internal_linkage_decl : Note< + "declared here%select{ in module '%1'|}0">; } let CategoryName = "Coroutines Issue" in { diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index 78dfe59392ab9aaf31ab7cef149e364ebd392566..39b7d8fcb5957d841e95db9a722b87e9682d3dff 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -1386,6 +1386,15 @@ public: hasVisibleDefaultArgument(const NamedDecl *D, llvm::SmallVectorImpl<Module *> *Modules = nullptr); + /// Determine if \p A and \p B are equivalent internal linkage declarations + /// from different modules, and thus an ambiguity error can be downgraded to + /// an extension warning. + bool isEquivalentInternalLinkageDeclaration(const NamedDecl *A, + const NamedDecl *B); + void diagnoseEquivalentInternalLinkageDeclarations( + SourceLocation Loc, const NamedDecl *D, + ArrayRef<const NamedDecl *> Equiv); + bool RequireCompleteType(SourceLocation Loc, QualType T, TypeDiagnoser &Diagnoser); bool RequireCompleteType(SourceLocation Loc, QualType T, diff --git a/lib/Sema/SemaLookup.cpp b/lib/Sema/SemaLookup.cpp index 66cfada9f43431fee4bfccde5bf255f2f482b303..a020dcf14d698dd62ab9e4ecb7012013f7bb7ba8 100644 --- a/lib/Sema/SemaLookup.cpp +++ b/lib/Sema/SemaLookup.cpp @@ -463,8 +463,11 @@ void LookupResult::resolveKind() { llvm::SmallDenseMap<QualType, unsigned, 16> UniqueTypes; bool Ambiguous = false; - bool HasTag = false, HasFunction = false, HasNonFunction = false; + bool HasTag = false, HasFunction = false; bool HasFunctionTemplate = false, HasUnresolved = false; + NamedDecl *HasNonFunction = nullptr; + + llvm::SmallVector<NamedDecl*, 4> EquivalentNonFunctions; unsigned UniqueTagIndex = 0; @@ -533,9 +536,21 @@ void LookupResult::resolveKind() { } else if (isa<FunctionDecl>(D)) { HasFunction = true; } else { - if (HasNonFunction) + if (HasNonFunction) { + // If we're about to create an ambiguity between two declarations that + // are equivalent, but one is an internal linkage declaration from one + // module and the other is an internal linkage declaration from another + // module, just skip it. + if (getSema().isEquivalentInternalLinkageDeclaration(HasNonFunction, + D)) { + EquivalentNonFunctions.push_back(D); + Decls[I] = Decls[--N]; + continue; + } + Ambiguous = true; - HasNonFunction = true; + } + HasNonFunction = D; } I++; } @@ -558,6 +573,12 @@ void LookupResult::resolveKind() { Ambiguous = true; } + // FIXME: This diagnostic should really be delayed until we're done with + // the lookup result, in case the ambiguity is resolved by the caller. + if (!EquivalentNonFunctions.empty() && !Ambiguous) + getSema().diagnoseEquivalentInternalLinkageDeclarations( + getNameLoc(), HasNonFunction, EquivalentNonFunctions); + Decls.set_size(N); if (HasNonFunction && (HasFunction || HasUnresolved)) diff --git a/lib/Sema/SemaOverload.cpp b/lib/Sema/SemaOverload.cpp index 52c0beac9ca69b8f00a63fab8a102b8a2dc1cbce..8ce3c21c3d33d98aa4dc46544a7a598e1fb40d11 100644 --- a/lib/Sema/SemaOverload.cpp +++ b/lib/Sema/SemaOverload.cpp @@ -8575,22 +8575,38 @@ bool clang::isBetterOverloadCandidate(Sema &S, const OverloadCandidate &Cand1, return false; } -/// Determine whether two function declarations are "equivalent" for overload -/// resolution purposes. This applies when the same internal linkage function -/// is defined by two modules (textually including the same header). In such -/// a case, we don't consider the declarations to declare the same entity, but -/// we also don't want lookups with both declarations visible to be ambiguous -/// in some cases (this happens when using a modularized libstdc++). -static bool isEquivalentCompatibleOverload(Sema &S, - const OverloadCandidate &Best, - const OverloadCandidate &Cand) { - return Best.Function && Cand.Function && - Best.Function->getDeclContext()->getRedeclContext()->Equals( - Cand.Function->getDeclContext()->getRedeclContext()) && - S.getOwningModule(Best.Function) != S.getOwningModule(Cand.Function) && - !Best.Function->isExternallyVisible() && - !Cand.Function->isExternallyVisible() && - !S.IsOverload(Best.Function, Cand.Function, /*UsingDecl*/false); +/// Determine whether two declarations are "equivalent" for the purposes of +/// name lookup and overload resolution. This applies when the same internal +/// linkage variable or function is defined by two modules (textually including +/// the same header). In such a case, we don't consider the declarations to +/// declare the same entity, but we also don't want lookups with both +/// declarations visible to be ambiguous in some cases (this happens when using +/// a modularized libstdc++). +bool Sema::isEquivalentInternalLinkageDeclaration(const NamedDecl *A, + const NamedDecl *B) { + return A && B && isa<ValueDecl>(A) && isa<ValueDecl>(B) && + A->getDeclContext()->getRedeclContext()->Equals( + B->getDeclContext()->getRedeclContext()) && + getOwningModule(const_cast<NamedDecl *>(A)) != + getOwningModule(const_cast<NamedDecl *>(B)) && + !A->isExternallyVisible() && !B->isExternallyVisible() && + Context.hasSameType(cast<ValueDecl>(A)->getType(), + cast<ValueDecl>(B)->getType()); +} + +void Sema::diagnoseEquivalentInternalLinkageDeclarations( + SourceLocation Loc, const NamedDecl *D, ArrayRef<const NamedDecl *> Equiv) { + Diag(Loc, diag::ext_equivalent_internal_linkage_decl_in_modules) << D; + + Module *M = getOwningModule(const_cast<NamedDecl*>(D)); + Diag(D->getLocation(), diag::note_equivalent_internal_linkage_decl) + << !M << (M ? M->getFullModuleName() : ""); + + for (auto *E : Equiv) { + Module *M = getOwningModule(const_cast<NamedDecl*>(E)); + Diag(E->getLocation(), diag::note_equivalent_internal_linkage_decl) + << !M << (M ? M->getFullModuleName() : ""); + } } static void NoteFunctionCandidate(Sema &S, OverloadCandidate *Cand, @@ -8623,7 +8639,7 @@ OverloadCandidateSet::BestViableFunction(Sema &S, SourceLocation Loc, if (Best == end()) return OR_No_Viable_Function; - llvm::SmallVector<const OverloadCandidate *, 4> EquivalentCands; + llvm::SmallVector<const NamedDecl *, 4> EquivalentCands; // Make sure that this function is better than every other viable // function. If not, we have an ambiguity. @@ -8632,8 +8648,9 @@ OverloadCandidateSet::BestViableFunction(Sema &S, SourceLocation Loc, Cand != Best && !isBetterOverloadCandidate(S, *Best, *Cand, Loc, UserDefinedConversion)) { - if (isEquivalentCompatibleOverload(S, *Best, *Cand)) { - EquivalentCands.push_back(Cand); + if (S.isEquivalentInternalLinkageDeclaration(Best->Function, + Cand->Function)) { + EquivalentCands.push_back(Cand->Function); continue; } @@ -8648,13 +8665,9 @@ OverloadCandidateSet::BestViableFunction(Sema &S, SourceLocation Loc, S.isFunctionConsideredUnavailable(Best->Function))) return OR_Deleted; - if (!EquivalentCands.empty()) { - S.Diag(Loc, diag::ext_ovl_equivalent_internal_linkage_functions_in_modules) - << Best->Function; - S.NoteOverloadCandidate(Best->Function); - for (auto *Cand : EquivalentCands) - S.NoteOverloadCandidate(Cand->Function); - } + if (!EquivalentCands.empty()) + S.diagnoseEquivalentInternalLinkageDeclarations(Loc, Best->Function, + EquivalentCands); return OR_Success; } diff --git a/test/Modules/Inputs/libstdcxx-ambiguous-internal/a.h b/test/Modules/Inputs/libstdcxx-ambiguous-internal/a.h index e079bd0908eab585a9f0694cd1de80a1e63f943c..0971369913333835fd81efc1d1107582e7c82a6a 100644 --- a/test/Modules/Inputs/libstdcxx-ambiguous-internal/a.h +++ b/test/Modules/Inputs/libstdcxx-ambiguous-internal/a.h @@ -1,4 +1,5 @@ #ifndef A_H #define A_H static inline void f() {} +constexpr int n = 0; #endif diff --git a/test/Modules/Inputs/libstdcxx-ambiguous-internal/d.h b/test/Modules/Inputs/libstdcxx-ambiguous-internal/d.h index 1627bfb99e231636fb50796a157711f0d3f69455..efec99f003d8c72e4f22ffb69fb2f6f4cd2422ed 100644 --- a/test/Modules/Inputs/libstdcxx-ambiguous-internal/d.h +++ b/test/Modules/Inputs/libstdcxx-ambiguous-internal/d.h @@ -1,3 +1,4 @@ #include "b.h" #include "c.h" inline void g() { f(); } +inline int h() { return n; } diff --git a/test/Modules/libstdcxx-ambiguous-internal.cpp b/test/Modules/libstdcxx-ambiguous-internal.cpp index 966b9c9768188de870c8b503364d504b62cb0f8d..784b442eab468f97c1cb1f3c2d01ef6f200a4938 100644 --- a/test/Modules/libstdcxx-ambiguous-internal.cpp +++ b/test/Modules/libstdcxx-ambiguous-internal.cpp @@ -3,5 +3,11 @@ // RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -emit-module -fmodule-name=std -fmodules-cache-path=%t %S/Inputs/libstdcxx-ambiguous-internal/module.modulemap -fmodules-local-submodule-visibility -DAMBIGUOUS 2>&1| FileCheck %s // CHECK-NOT: error -// CHECK: warning: ambiguous use of internal linkage function 'f' defined in multiple modules +// CHECK: warning: ambiguous use of internal linkage declaration 'f' defined in multiple modules +// CHECK: note: declared here in module 'std.C' +// CHECK: note: declared here in module 'std.B' +// CHECK-NOT: error +// CHECK: warning: ambiguous use of internal linkage declaration 'n' defined in multiple modules +// CHECK: note: declared here in module 'std.C' +// CHECK: note: declared here in module 'std.B' // CHECK-NOT: error