[clang] cc1b666 - [Clang] Fix member lookup so that we don't ignore ambiguous lookups in some cases
Shafik Yaghmour via cfe-commits
cfe-commits at lists.llvm.org
Fri Jul 28 15:24:27 PDT 2023
Author: Shafik Yaghmour
Date: 2023-07-28T15:21:57-07:00
New Revision: cc1b6668c57170cd440d321037ced89d6a61a9cb
URL: https://github.com/llvm/llvm-project/commit/cc1b6668c57170cd440d321037ced89d6a61a9cb
DIFF: https://github.com/llvm/llvm-project/commit/cc1b6668c57170cd440d321037ced89d6a61a9cb.diff
LOG: [Clang] Fix member lookup so that we don't ignore ambiguous lookups in some cases
There are some cases during member lookup we are aggressively suppressing
diagnostics when we should just be suppressing access control diagnostic.
In this PR I add the ability to simply suppress access diagnostics while not
suppressing ambiguous lookup diagnostics.
Fixes: https://github.com/llvm/llvm-project/issues/22413
https://github.com/llvm/llvm-project/issues/29942
https://github.com/llvm/llvm-project/issues/35574
https://github.com/llvm/llvm-project/issues/27224
Differential Revision: https://reviews.llvm.org/D155387
Added:
clang/test/CXX/class.derived/class.member.lookup/gh22413.cpp
clang/test/CXX/class.derived/class.member.lookup/p11.cpp
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/Sema/Lookup.h
clang/lib/Sema/SemaOverload.cpp
clang/lib/Sema/SemaTemplate.cpp
clang/test/SemaCXX/arrow-operator.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index de93b15bc1b3c5..4b8a323093ccd5 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -121,6 +121,12 @@ Bug Fixes to C++ Support
a Unicode character whose name contains a ``-``.
(`Fixes #64161 <https://github.com/llvm/llvm-project/issues/64161>_`)
+- Fix cases where we ignore ambiguous name lookup when looking up memebers.
+ (`#22413 <https://github.com/llvm/llvm-project/issues/22413>_`),
+ (`#29942 <https://github.com/llvm/llvm-project/issues/29942>_`),
+ (`#35574 <https://github.com/llvm/llvm-project/issues/35574>_`) and
+ (`#27224 <https://github.com/llvm/llvm-project/issues/27224>_`).
+
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Sema/Lookup.h b/clang/include/clang/Sema/Lookup.h
index 351fa0c6ca0cb9..cbc9b733940a3d 100644
--- a/clang/include/clang/Sema/Lookup.h
+++ b/clang/include/clang/Sema/Lookup.h
@@ -148,20 +148,22 @@ class LookupResult {
: SemaPtr(&SemaRef), NameInfo(NameInfo), LookupKind(LookupKind),
Redecl(Redecl != Sema::NotForRedeclaration),
ExternalRedecl(Redecl == Sema::ForExternalRedeclaration),
- Diagnose(Redecl == Sema::NotForRedeclaration) {
+ DiagnoseAccess(Redecl == Sema::NotForRedeclaration),
+ DiagnoseAmbiguous(Redecl == Sema::NotForRedeclaration) {
configure();
}
// TODO: consider whether this constructor should be restricted to take
// as input a const IdentifierInfo* (instead of Name),
// forcing other cases towards the constructor taking a DNInfo.
- LookupResult(Sema &SemaRef, DeclarationName Name,
- SourceLocation NameLoc, Sema::LookupNameKind LookupKind,
+ LookupResult(Sema &SemaRef, DeclarationName Name, SourceLocation NameLoc,
+ Sema::LookupNameKind LookupKind,
Sema::RedeclarationKind Redecl = Sema::NotForRedeclaration)
: SemaPtr(&SemaRef), NameInfo(Name, NameLoc), LookupKind(LookupKind),
Redecl(Redecl != Sema::NotForRedeclaration),
ExternalRedecl(Redecl == Sema::ForExternalRedeclaration),
- Diagnose(Redecl == Sema::NotForRedeclaration) {
+ DiagnoseAccess(Redecl == Sema::NotForRedeclaration),
+ DiagnoseAmbiguous(Redecl == Sema::NotForRedeclaration) {
configure();
}
@@ -192,12 +194,14 @@ class LookupResult {
Redecl(std::move(Other.Redecl)),
ExternalRedecl(std::move(Other.ExternalRedecl)),
HideTags(std::move(Other.HideTags)),
- Diagnose(std::move(Other.Diagnose)),
+ DiagnoseAccess(std::move(Other.DiagnoseAccess)),
+ DiagnoseAmbiguous(std::move(Other.DiagnoseAmbiguous)),
AllowHidden(std::move(Other.AllowHidden)),
Shadowed(std::move(Other.Shadowed)),
TemplateNameLookup(std::move(Other.TemplateNameLookup)) {
Other.Paths = nullptr;
- Other.Diagnose = false;
+ Other.DiagnoseAccess = false;
+ Other.DiagnoseAmbiguous = false;
}
LookupResult &operator=(LookupResult &&Other) {
@@ -215,17 +219,22 @@ class LookupResult {
Redecl = std::move(Other.Redecl);
ExternalRedecl = std::move(Other.ExternalRedecl);
HideTags = std::move(Other.HideTags);
- Diagnose = std::move(Other.Diagnose);
+ DiagnoseAccess = std::move(Other.DiagnoseAccess);
+ DiagnoseAmbiguous = std::move(Other.DiagnoseAmbiguous);
AllowHidden = std::move(Other.AllowHidden);
Shadowed = std::move(Other.Shadowed);
TemplateNameLookup = std::move(Other.TemplateNameLookup);
Other.Paths = nullptr;
- Other.Diagnose = false;
+ Other.DiagnoseAccess = false;
+ Other.DiagnoseAmbiguous = false;
return *this;
}
~LookupResult() {
- if (Diagnose) diagnose();
+ if (DiagnoseAccess)
+ diagnoseAccess();
+ if (DiagnoseAmbiguous)
+ diagnoseAmbiguous();
if (Paths) deletePaths(Paths);
}
@@ -607,13 +616,20 @@ class LookupResult {
/// Suppress the diagnostics that would normally fire because of this
/// lookup. This happens during (e.g.) redeclaration lookups.
void suppressDiagnostics() {
- Diagnose = false;
+ DiagnoseAccess = false;
+ DiagnoseAmbiguous = false;
}
- /// Determines whether this lookup is suppressing diagnostics.
- bool isSuppressingDiagnostics() const {
- return !Diagnose;
- }
+ /// Suppress the diagnostics that would normally fire because of this
+ /// lookup due to access control violations.
+ void suppressAccessDiagnostics() { DiagnoseAccess = false; }
+
+ /// Determines whether this lookup is suppressing access control diagnostics.
+ bool isSuppressingAccessDiagnostics() const { return !DiagnoseAccess; }
+
+ /// Determines whether this lookup is suppressing ambiguous lookup
+ /// diagnostics.
+ bool isSuppressingAmbiguousDiagnostics() const { return !DiagnoseAmbiguous; }
/// Sets a 'context' source range.
void setContextRange(SourceRange SR) {
@@ -726,11 +742,14 @@ class LookupResult {
}
private:
- void diagnose() {
+ void diagnoseAccess() {
+ if (isClassLookup() && getSema().getLangOpts().AccessControl)
+ getSema().CheckLookupAccess(*this);
+ }
+
+ void diagnoseAmbiguous() {
if (isAmbiguous())
getSema().DiagnoseAmbiguousLookup(*this);
- else if (isClassLookup() && getSema().getLangOpts().AccessControl)
- getSema().CheckLookupAccess(*this);
}
void setAmbiguous(AmbiguityKind AK) {
@@ -776,7 +795,8 @@ class LookupResult {
/// are present
bool HideTags = true;
- bool Diagnose = false;
+ bool DiagnoseAccess = false;
+ bool DiagnoseAmbiguous = false;
/// True if we should allow hidden declarations to be 'visible'.
bool AllowHidden = false;
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index a3d9abb1537789..40a94e13c72950 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -950,7 +950,7 @@ static bool shouldAddReversedEqEq(Sema &S, SourceLocation OpLoc,
LookupResult Members(S, NotEqOp, OpLoc,
Sema::LookupNameKind::LookupMemberName);
S.LookupQualifiedName(Members, RHSRec->getDecl());
- Members.suppressDiagnostics();
+ Members.suppressAccessDiagnostics();
for (NamedDecl *Op : Members)
if (FunctionsCorrespond(S.Context, EqFD, Op->getAsFunction()))
return false;
@@ -961,7 +961,7 @@ static bool shouldAddReversedEqEq(Sema &S, SourceLocation OpLoc,
Sema::LookupNameKind::LookupOperatorName);
S.LookupName(NonMembers,
S.getScopeForContext(EqFD->getEnclosingNamespaceContext()));
- NonMembers.suppressDiagnostics();
+ NonMembers.suppressAccessDiagnostics();
for (NamedDecl *Op : NonMembers) {
auto *FD = Op->getAsFunction();
if(auto* UD = dyn_cast<UsingShadowDecl>(Op))
@@ -7987,7 +7987,7 @@ void Sema::AddMemberOperatorCandidates(OverloadedOperatorKind Op,
LookupResult Operators(*this, OpName, OpLoc, LookupOrdinaryName);
LookupQualifiedName(Operators, T1Rec->getDecl());
- Operators.suppressDiagnostics();
+ Operators.suppressAccessDiagnostics();
for (LookupResult::iterator Oper = Operators.begin(),
OperEnd = Operators.end();
@@ -14983,7 +14983,7 @@ Sema::BuildCallToObjectOfClassType(Scope *S, Expr *Obj,
const auto *Record = Object.get()->getType()->castAs<RecordType>();
LookupResult R(*this, OpName, LParenLoc, LookupOrdinaryName);
LookupQualifiedName(R, Record->getDecl());
- R.suppressDiagnostics();
+ R.suppressAccessDiagnostics();
for (LookupResult::iterator Oper = R.begin(), OperEnd = R.end();
Oper != OperEnd; ++Oper) {
@@ -15080,12 +15080,13 @@ Sema::BuildCallToObjectOfClassType(Scope *S, Expr *Obj,
break;
}
case OR_Ambiguous:
- CandidateSet.NoteCandidates(
- PartialDiagnosticAt(Object.get()->getBeginLoc(),
- PDiag(diag::err_ovl_ambiguous_object_call)
- << Object.get()->getType()
- << Object.get()->getSourceRange()),
- *this, OCD_AmbiguousCandidates, Args);
+ if (!R.isAmbiguous())
+ CandidateSet.NoteCandidates(
+ PartialDiagnosticAt(Object.get()->getBeginLoc(),
+ PDiag(diag::err_ovl_ambiguous_object_call)
+ << Object.get()->getType()
+ << Object.get()->getSourceRange()),
+ *this, OCD_AmbiguousCandidates, Args);
break;
case OR_Deleted:
@@ -15248,7 +15249,7 @@ Sema::BuildOverloadedArrowExpr(Scope *S, Expr *Base, SourceLocation OpLoc,
LookupResult R(*this, OpName, OpLoc, LookupOrdinaryName);
LookupQualifiedName(R, Base->getType()->castAs<RecordType>()->getDecl());
- R.suppressDiagnostics();
+ R.suppressAccessDiagnostics();
for (LookupResult::iterator Oper = R.begin(), OperEnd = R.end();
Oper != OperEnd; ++Oper) {
@@ -15289,11 +15290,12 @@ Sema::BuildOverloadedArrowExpr(Scope *S, Expr *Base, SourceLocation OpLoc,
return ExprError();
}
case OR_Ambiguous:
- CandidateSet.NoteCandidates(
- PartialDiagnosticAt(OpLoc, PDiag(diag::err_ovl_ambiguous_oper_unary)
- << "->" << Base->getType()
- << Base->getSourceRange()),
- *this, OCD_AmbiguousCandidates, Base);
+ if (!R.isAmbiguous())
+ CandidateSet.NoteCandidates(
+ PartialDiagnosticAt(OpLoc, PDiag(diag::err_ovl_ambiguous_oper_unary)
+ << "->" << Base->getType()
+ << Base->getSourceRange()),
+ *this, OCD_AmbiguousCandidates, Base);
return ExprError();
case OR_Deleted:
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index a1f0f5732b2b77..0dd24d17410d34 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -593,7 +593,7 @@ bool Sema::LookupTemplateName(LookupResult &Found,
// postfix-expression and does not name a class template, the name
// found in the class of the object expression is used, otherwise
FoundOuter.clear();
- } else if (!Found.isSuppressingDiagnostics()) {
+ } else if (!Found.isSuppressingAmbiguousDiagnostics()) {
// - if the name found is a class template, it must refer to the same
// entity as the one found in the class of the object expression,
// otherwise the program is ill-formed.
diff --git a/clang/test/CXX/class.derived/class.member.lookup/gh22413.cpp b/clang/test/CXX/class.derived/class.member.lookup/gh22413.cpp
new file mode 100644
index 00000000000000..19fb11783544e5
--- /dev/null
+++ b/clang/test/CXX/class.derived/class.member.lookup/gh22413.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct A {
+ void operator()(int); // expected-note {{member found by ambiguous name lookup}}
+ void f(int); // expected-note {{member found by ambiguous name lookup}}
+};
+struct B {
+ void operator()(); // expected-note {{member found by ambiguous name lookup}}
+ void f() {} // expected-note {{member found by ambiguous name lookup}}
+};
+
+struct C : A, B {};
+
+int f() {
+ C c;
+ c(); // expected-error {{member 'operator()' found in multiple base classes of
diff erent types}}
+ c.f(10); //expected-error {{member 'f' found in multiple base classes of
diff erent types}}
+ return 0;
+}
diff --git a/clang/test/CXX/class.derived/class.member.lookup/p11.cpp b/clang/test/CXX/class.derived/class.member.lookup/p11.cpp
new file mode 100644
index 00000000000000..e0899b227e69bd
--- /dev/null
+++ b/clang/test/CXX/class.derived/class.member.lookup/p11.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct B1 {
+ void f();
+ static void f(int);
+ int i; // expected-note 2{{member found by ambiguous name lookup}}
+};
+struct B2 {
+ void f(double);
+};
+struct I1: B1 { };
+struct I2: B1 { };
+
+struct D: I1, I2, B2 {
+ using B1::f;
+ using B2::f;
+ void g() {
+ f(); // expected-error {{ambiguous conversion from derived class 'D' to base class 'B1'}}
+ f(0); // ok
+ f(0.0); // ok
+ // FIXME next line should be well-formed
+ int B1::* mpB1 = &D::i; // expected-error {{non-static member 'i' found in multiple base-class subobjects of type 'B1'}}
+ int D::* mpD = &D::i; // expected-error {{non-static member 'i' found in multiple base-class subobjects of type 'B1'}}
+ }
+};
diff --git a/clang/test/SemaCXX/arrow-operator.cpp b/clang/test/SemaCXX/arrow-operator.cpp
index c6d2a99251be48..4107e78c91c84d 100644
--- a/clang/test/SemaCXX/arrow-operator.cpp
+++ b/clang/test/SemaCXX/arrow-operator.cpp
@@ -4,11 +4,13 @@ struct T {
};
struct A {
- T* operator->(); // expected-note{{candidate function}}
+ T* operator->();
+ // expected-note at -1 {{member found by ambiguous name lookup}}
};
struct B {
- T* operator->(); // expected-note{{candidate function}}
+ T* operator->();
+ // expected-note at -1 {{member found by ambiguous name lookup}}
};
struct C : A, B {
@@ -19,7 +21,8 @@ struct D : A { };
struct E; // expected-note {{forward declaration of 'E'}}
void f(C &c, D& d, E& e) {
- c->f(); // expected-error{{use of overloaded operator '->' is ambiguous}}
+ c->f();
+ // expected-error at -1 {{member 'operator->' found in multiple base classes of
diff erent types}}
d->f();
e->f(); // expected-error{{incomplete definition of type}}
}
More information about the cfe-commits
mailing list