Author: Krystian Stasiowski Date: 2024-09-09T12:06:45-04:00 New Revision: 3cdb30ebbc18fa894d3bd67aebcff76ce7c741ac
URL: https://github.com/llvm/llvm-project/commit/3cdb30ebbc18fa894d3bd67aebcff76ce7c741ac DIFF: https://github.com/llvm/llvm-project/commit/3cdb30ebbc18fa894d3bd67aebcff76ce7c741ac.diff LOG: [Clang][Sema] Use the correct lookup context when building overloaded 'operator->' in the current instantiation (#104458) Currently, clang erroneously rejects the following: ``` struct A { template<typename T> void f(); }; template<typename T> struct B { void g() { (*this)->template f<int>(); // error: no member named 'f' in 'B<T>' } A* operator->(); }; ``` This happens because `Sema::ActOnStartCXXMemberReference` does not adjust the `ObjectType` parameter when `ObjectType` is a dependent type (except when the type is a `PointerType` and the class member access is the `->` form). Since the (possibly adjusted) `ObjectType` parameter (`B<T>` in the above example) is passed to `Parser::ParseOptionalCXXScopeSpecifier`, we end up looking up `f` in `B` rather than `A`. This patch fixes the issue by identifying cases where the type of the object expression `T` is a dependent, non-pointer type and: - `T` is the current instantiation and lookup for `operator->` finds a member of the current instantiation, or - `T` has at least one dependent base case, and `operator->` is not found in the current instantiation and using `ASTContext::DependentTy` as the type of the object expression when the optional _nested-name-specifier_ is parsed. Fixes #104268. Added: Modified: clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaExprCXX.cpp clang/lib/Sema/SemaOverload.cpp clang/lib/Sema/TreeTransform.h clang/test/CXX/temp/temp.res/temp.dep/temp.dep.type/p4.cpp Removed: ################################################################################ diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 68c782a15c6f1b..c1afefd36ce0ce 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -10639,9 +10639,9 @@ class Sema final : public SemaBase { /// BuildOverloadedArrowExpr - Build a call to an overloaded @c operator-> /// (if one exists), where @c Base is an expression of class type and /// @c Member is the name of the member we're trying to find. - ExprResult BuildOverloadedArrowExpr(Scope *S, Expr *Base, - SourceLocation OpLoc, - bool *NoArrowOperatorFound = nullptr); + ExprResult BuildOverloadedArrowExpr(Expr *Base, SourceLocation OpLoc, + bool *NoArrowOperatorFound, + bool &IsDependent); ExprResult BuildCXXMemberCallExpr(Expr *Exp, NamedDecl *FoundDecl, CXXConversionDecl *Method, diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp index 14feafd1e6b17f..3c7eac116e5dc9 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -7966,18 +7966,6 @@ ExprResult Sema::ActOnStartCXXMemberReference(Scope *S, Expr *Base, QualType BaseType = Base->getType(); MayBePseudoDestructor = false; - if (BaseType->isDependentType()) { - // If we have a pointer to a dependent type and are using the -> operator, - // the object type is the type that the pointer points to. We might still - // have enough information about that type to do something useful. - if (OpKind == tok::arrow) - if (const PointerType *Ptr = BaseType->getAs<PointerType>()) - BaseType = Ptr->getPointeeType(); - - ObjectType = ParsedType::make(BaseType); - MayBePseudoDestructor = true; - return Base; - } // C++ [over.match.oper]p8: // [...] When operator->returns, the operator-> is applied to the value @@ -7992,7 +7980,8 @@ ExprResult Sema::ActOnStartCXXMemberReference(Scope *S, Expr *Base, SmallVector<FunctionDecl*, 8> OperatorArrows; CTypes.insert(Context.getCanonicalType(BaseType)); - while (BaseType->isRecordType()) { + while ( + isa<InjectedClassNameType, RecordType>(BaseType.getCanonicalType())) { if (OperatorArrows.size() >= getLangOpts().ArrowDepth) { Diag(OpLoc, diag::err_operator_arrow_depth_exceeded) << StartingType << getLangOpts().ArrowDepth << Base->getSourceRange(); @@ -8002,15 +7991,26 @@ ExprResult Sema::ActOnStartCXXMemberReference(Scope *S, Expr *Base, return ExprError(); } + bool IsDependent; Result = BuildOverloadedArrowExpr( - S, Base, OpLoc, + Base, OpLoc, // When in a template specialization and on the first loop iteration, // potentially give the default diagnostic (with the fixit in a // separate note) instead of having the error reported back to here // and giving a diagnostic with a fixit attached to the error itself. (FirstIteration && CurFD && CurFD->isFunctionTemplateSpecialization()) ? nullptr - : &NoArrowOperatorFound); + : &NoArrowOperatorFound, + IsDependent); + + if (IsDependent) { + // BuildOverloadedArrowExpr sets IsDependent to indicate that we need + // to build a dependent overloaded arrow expression. + assert(BaseType->isDependentType()); + BaseType = Context.DependentTy; + break; + } + if (Result.isInvalid()) { if (NoArrowOperatorFound) { if (FirstIteration) { @@ -8030,6 +8030,7 @@ ExprResult Sema::ActOnStartCXXMemberReference(Scope *S, Expr *Base, } return ExprError(); } + Base = Result.get(); if (CXXOperatorCallExpr *OpCall = dyn_cast<CXXOperatorCallExpr>(Base)) OperatorArrows.push_back(OpCall->getDirectCallee()); @@ -8067,7 +8068,7 @@ ExprResult Sema::ActOnStartCXXMemberReference(Scope *S, Expr *Base, // it's legal for the type to be incomplete if this is a pseudo-destructor // call. We'll do more incomplete-type checks later in the lookup process, // so just skip this check for ObjC types. - if (!BaseType->isRecordType()) { + if (!isa<InjectedClassNameType, RecordType>(BaseType.getCanonicalType())) { ObjectType = ParsedType::make(BaseType); MayBePseudoDestructor = true; return Base; @@ -8085,6 +8086,10 @@ ExprResult Sema::ActOnStartCXXMemberReference(Scope *S, Expr *Base, return CreateRecoveryExpr(Base->getBeginLoc(), Base->getEndLoc(), {Base}); } + // We can't implicitly declare the destructor for a templated class. + if (BaseType->isDependentType()) + MayBePseudoDestructor = true; + // C++ [basic.lookup.classref]p2: // If the id-expression in a class member access (5.2.5) is an // unqualified-id, and the type of the object expression is of a class diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index 861b0a91240b3b..6b91d4898674ec 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -15878,12 +15878,14 @@ Sema::BuildCallToObjectOfClassType(Scope *S, Expr *Obj, return CheckForImmediateInvocation(MaybeBindToTemporary(TheCall), Method); } -ExprResult -Sema::BuildOverloadedArrowExpr(Scope *S, Expr *Base, SourceLocation OpLoc, - bool *NoArrowOperatorFound) { - assert(Base->getType()->isRecordType() && +ExprResult Sema::BuildOverloadedArrowExpr(Expr *Base, SourceLocation OpLoc, + bool *NoArrowOperatorFound, + bool &IsDependent) { + assert(Base->getType()->getAsRecordDecl() && "left-hand side must have class type"); + IsDependent = false; + if (checkPlaceholderForOverload(*this, Base)) return ExprError(); @@ -15904,7 +15906,19 @@ Sema::BuildOverloadedArrowExpr(Scope *S, Expr *Base, SourceLocation OpLoc, return ExprError(); LookupResult R(*this, OpName, OpLoc, LookupOrdinaryName); - LookupQualifiedName(R, Base->getType()->castAs<RecordType>()->getDecl()); + LookupParsedName(R, /*S=*/nullptr, /*SS=*/nullptr, Base->getType()); + + // If the expression is dependent and we either: + // - found a member of the current instantiation named 'operator->', or + // - found nothing, and the lookup context has no dependent base classes + // + // then we should build a dependent class member access expression. + if (R.wasNotFoundInCurrentInstantiation() || + (Base->isTypeDependent() && !R.empty())) { + IsDependent = true; + return Base; + } + R.suppressAccessDiagnostics(); for (LookupResult::iterator Oper = R.begin(), OperEnd = R.end(); diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h index 0daf620b4123e4..235b51a33babd5 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -16583,10 +16583,14 @@ ExprResult TreeTransform<Derived>::RebuildCXXOperatorCallExpr( } else if (Op == OO_Arrow) { // It is possible that the type refers to a RecoveryExpr created earlier // in the tree transformation. - if (First->getType()->isDependentType()) + if (First->containsErrors()) return ExprError(); + bool IsDependent; // -> is never a builtin operation. - return SemaRef.BuildOverloadedArrowExpr(nullptr, First, OpLoc); + ExprResult Result = SemaRef.BuildOverloadedArrowExpr( + First, OpLoc, /*NoArrowOperatorFound=*/nullptr, IsDependent); + assert(!IsDependent); + return Result; } else if (Second == nullptr || isPostIncDec) { if (!First->getType()->isOverloadableType() || (Op == OO_Amp && getSema().isQualifiedMemberAccess(First))) { diff --git a/clang/test/CXX/temp/temp.res/temp.dep/temp.dep.type/p4.cpp b/clang/test/CXX/temp/temp.res/temp.dep/temp.dep.type/p4.cpp index f32f49ef4539a5..89c22a0bc137d9 100644 --- a/clang/test/CXX/temp/temp.res/temp.dep/temp.dep.type/p4.cpp +++ b/clang/test/CXX/temp/temp.res/temp.dep/temp.dep.type/p4.cpp @@ -484,16 +484,19 @@ namespace N4 { template<typename T> struct A { void not_instantiated(A a, A<T> b, T c) { - a->x; - b->x; + a->x; // expected-error {{member reference type 'A<T>' is not a pointer; did you mean to use '.'?}} + b->x; // expected-error {{member reference type 'A<T>' is not a pointer; did you mean to use '.'?}} c->x; } void instantiated(A a, A<T> b, T c) { - a->x; // expected-error {{member reference type 'A<int>' is not a pointer; did you mean to use '.'?}} - // expected-error@-1 {{no member named 'x' in 'N4::A<int>'}} - b->x; // expected-error {{member reference type 'A<int>' is not a pointer; did you mean to use '.'?}} - // expected-error@-1 {{no member named 'x' in 'N4::A<int>'}} + // FIXME: We should only emit a single diagnostic suggesting to use '.'! + a->x; // expected-error {{member reference type 'A<T>' is not a pointer; did you mean to use '.'?}} + // expected-error@-1 {{member reference type 'A<int>' is not a pointer; did you mean to use '.'?}} + // expected-error@-2 {{no member named 'x' in 'N4::A<int>'}} + b->x; // expected-error {{member reference type 'A<T>' is not a pointer; did you mean to use '.'?}} + // expected-error@-1 {{member reference type 'A<int>' is not a pointer; did you mean to use '.'?}} + // expected-error@-2 {{no member named 'x' in 'N4::A<int>'}} c->x; // expected-error {{member reference type 'int' is not a pointer}} } }; @@ -540,11 +543,10 @@ namespace N4 { a->T::f(); a->T::g(); - // FIXME: 'U' should be a dependent name, and its lookup context should be 'a.operator->()'! - a->U::x; // expected-error {{use of undeclared identifier 'U'}} - a->U::y; // expected-error {{use of undeclared identifier 'U'}} - a->U::f(); // expected-error {{use of undeclared identifier 'U'}} - a->U::g(); // expected-error {{use of undeclared identifier 'U'}} + a->U::x; + a->U::y; + a->U::f(); + a->U::g(); } void instantiated(D a) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits