jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, vsapsai.
Herald added a project: All.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
When finalizing the result of name lookup that encountered ambiguity, we check
equivalence for declarations with internal linkage. If the declarations are
equivalent, we just produce a warning instead of ambiguous result.
In `Sema::isEquivalentInternalLinkageDeclaration` that implements the check, we
have a special case for constants of anonymous enums. However, it only kicks in
for C++ anonymous enums (since they have `NoLinkage`, meaning they are not
externally visible).
In (Objective)C, constants of anonymous enums are `VisibleNoLinkage`, meaning
they _are_ externally visible, so the special case doesn't apply for them.
This patch renames the function to `isEquivalentNonExternalLinkageDeclaration`
(its documentation already says it handles declarations with "internal/no
linkage") and makes it so that even `VisibleNoLinkage` declarations are not
marked as ambiguous. This is achieved by using `hasExternalFormalLinkage`
instead of `isExternallyVisible` in one of the equivalence checks. Later on, we
don't even emit the warning for such declarations, since they are considered
the same entity.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D121465
Files:
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaLookup.cpp
clang/lib/Sema/SemaOverload.cpp
clang/test/Modules/ambiguous-anonymous-enum-lookup.m.cpp
Index: clang/test/Modules/ambiguous-anonymous-enum-lookup.m.cpp
===================================================================
--- /dev/null
+++ clang/test/Modules/ambiguous-anonymous-enum-lookup.m.cpp
@@ -0,0 +1,40 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache -I %t/include %t/test.cpp -verify
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache -I %t/include %t/test.m -verify
+
+//--- include/textual.h
+#ifndef TEXTUAL_H
+#define TEXTUAL_H
+enum { kAnonymousEnumValue = 0 };
+#endif
+
+//--- include/empty.h
+//--- include/initially_hidden.h
+#include "textual.h"
+
+//--- include/module.modulemap
+module Piecewise {
+ module Empty { header "empty.h" }
+ module InitiallyHidden { header "initially_hidden.h" }
+}
+
+//--- test.cpp
+#include "empty.h"
+#include "textual.h"
+#include "initially_hidden.h"
+
+int testReferencingAnonymousEnumConstant() {
+ return kAnonymousEnumValue; // expected-warning {{ambiguous use of internal linkage declaration 'kAnonymousEnumValue' defined in multiple modules}}
+ // expected-note@textual.h:3 {{declared here}}
+ // expected-note@textual.h:3 {{declared here in module 'Piecewise.InitiallyHidden'}}
+}
+
+//--- test.m
+#include "empty.h"
+#include "textual.h"
+#include "initially_hidden.h"
+
+int testReferencingAnonymousEnumConstant() {
+ return kAnonymousEnumValue; // expected-no-diagnostics
+}
Index: clang/lib/Sema/SemaOverload.cpp
===================================================================
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -9955,19 +9955,19 @@
/// 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) {
+bool Sema::isEquivalentNonExternalLinkageDeclaration(const NamedDecl *A,
+ const NamedDecl *B) {
auto *VA = dyn_cast_or_null<ValueDecl>(A);
auto *VB = dyn_cast_or_null<ValueDecl>(B);
if (!VA || !VB)
return false;
- // The declarations must be declaring the same name as an internal linkage
+ // The declarations must be declaring the same name as an internal/no linkage
// entity in different modules.
if (!VA->getDeclContext()->getRedeclContext()->Equals(
VB->getDeclContext()->getRedeclContext()) ||
getOwningModule(VA) == getOwningModule(VB) ||
- VA->isExternallyVisible() || VB->isExternallyVisible())
+ VA->hasExternalFormalLinkage() || VB->hasExternalFormalLinkage())
return false;
// Check that the declarations appear to be equivalent.
@@ -10093,8 +10093,8 @@
PendingBest.push_back(Cand);
Cand->Best = true;
- if (S.isEquivalentInternalLinkageDeclaration(Cand->Function,
- Curr->Function))
+ if (S.isEquivalentNonExternalLinkageDeclaration(Cand->Function,
+ Curr->Function))
EquivalentCands.push_back(Cand->Function);
else
Best = end();
Index: clang/lib/Sema/SemaLookup.cpp
===================================================================
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -509,6 +509,7 @@
NamedDecl *HasNonFunction = nullptr;
llvm::SmallVector<NamedDecl*, 4> EquivalentNonFunctions;
+ bool ComplainOnEquivalent = false;
unsigned UniqueTagIndex = 0;
@@ -576,11 +577,15 @@
} else {
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)) {
+ // are equivalent, but one is an internal/no linkage declaration from
+ // one module and the other is an internal/no linkage declaration from
+ // another module, just skip it.
+ if (getSema().isEquivalentNonExternalLinkageDeclaration(HasNonFunction,
+ D)) {
+ // Don't complain about declarations that are externally visible.
+ ComplainOnEquivalent |= !HasNonFunction->isExternallyVisible();
+ ComplainOnEquivalent |= !D->isExternallyVisible();
+
EquivalentNonFunctions.push_back(D);
Decls[I] = Decls[--N];
continue;
@@ -616,7 +621,7 @@
// 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)
+ if (!EquivalentNonFunctions.empty() && !Ambiguous && ComplainOnEquivalent)
getSema().diagnoseEquivalentInternalLinkageDeclarations(
getNameLoc(), HasNonFunction, EquivalentNonFunctions);
Index: clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -11908,7 +11908,7 @@
if (UsingShadowDecl *Shadow = dyn_cast<UsingShadowDecl>(*I))
PrevShadow = Shadow;
FoundEquivalentDecl = true;
- } else if (isEquivalentInternalLinkageDeclaration(D, Target)) {
+ } else if (isEquivalentNonExternalLinkageDeclaration(D, Target)) {
// We don't conflict with an existing using shadow decl of an equivalent
// declaration, but we're not a redeclaration of it.
FoundEquivalentDecl = true;
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -2311,11 +2311,11 @@
bool hasVisibleMemberSpecialization(
const NamedDecl *D, llvm::SmallVectorImpl<Module *> *Modules = nullptr);
- /// Determine if \p A and \p B are equivalent internal linkage declarations
+ /// Determine if \p A and \p B are equivalent no/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);
+ /// an extension warning or omitted.
+ bool isEquivalentNonExternalLinkageDeclaration(const NamedDecl *A,
+ const NamedDecl *B);
void diagnoseEquivalentInternalLinkageDeclarations(
SourceLocation Loc, const NamedDecl *D,
ArrayRef<const NamedDecl *> Equiv);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
