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@-1 {{member found by ambiguous name lookup}} }; struct B { - T* operator->(); // expected-note{{candidate function}} + T* operator->(); + // expected-note@-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@-1 {{member 'operator->' found in multiple base classes of diff erent types}} d->f(); e->f(); // expected-error{{incomplete definition of type}} } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits