diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index de93b15bc1b3c..4b8a323093ccd 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 _`) +- Fix cases where we ignore ambiguous name lookup when looking up memebers. + (`#22413 _`), + (`#29942 _`), + (`#35574 _`) and + (`#27224 _`). + Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/Sema/Lookup.h b/clang/include/clang/Sema/Lookup.h index 351fa0c6ca0cb..cbc9b733940a3 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 a3d9abb153778..40a94e13c7295 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(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(); 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()->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 a1f0f5732b2b7..0dd24d17410d3 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 0000000000000..19fb11783544e --- /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 different types}} + c.f(10); //expected-error {{member 'f' found in multiple base classes of different 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 0000000000000..e0899b227e69b --- /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 c6d2a99251be4..4107e78c91c84 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 different types}} d->f(); e->f(); // expected-error{{incomplete definition of type}} }