Skip to content

Reapply "[Clang][Sema] Use the correct lookup context when building overloaded 'operator->' in the current instantiation (#104458)" #109422

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Jan 22, 2025
5 changes: 2 additions & 3 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -10608,9 +10608,8 @@ 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);

ExprResult BuildCXXMemberCallExpr(Expr *Exp, NamedDecl *FoundDecl,
CXXConversionDecl *Method,
Expand Down
21 changes: 4 additions & 17 deletions clang/lib/Sema/SemaExprCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7999,18 +7999,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
Expand All @@ -8025,7 +8013,7 @@ ExprResult Sema::ActOnStartCXXMemberReference(Scope *S, Expr *Base,
SmallVector<FunctionDecl*, 8> OperatorArrows;
CTypes.insert(Context.getCanonicalType(BaseType));

while (BaseType->isRecordType()) {
while (BaseType->getAsRecordDecl()) {
if (OperatorArrows.size() >= getLangOpts().ArrowDepth) {
Diag(OpLoc, diag::err_operator_arrow_depth_exceeded)
<< StartingType << getLangOpts().ArrowDepth << Base->getSourceRange();
Expand All @@ -8036,7 +8024,7 @@ ExprResult Sema::ActOnStartCXXMemberReference(Scope *S, Expr *Base,
}

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
Expand Down Expand Up @@ -8100,7 +8088,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 (BaseType->isDependentType() || !BaseType->isRecordType()) {
ObjectType = ParsedType::make(BaseType);
MayBePseudoDestructor = true;
return Base;
Expand All @@ -8111,8 +8099,7 @@ ExprResult Sema::ActOnStartCXXMemberReference(Scope *S, Expr *Base,
// Unlike the object expression in other contexts, *this is not required to
// be of complete type for purposes of class member access (5.2.5) outside
// the member function body.
if (!BaseType->isDependentType() &&
!isThisOutsideMemberFunctionBody(BaseType) &&
if (!isThisOutsideMemberFunctionBody(BaseType) &&
RequireCompleteType(OpLoc, BaseType,
diag::err_incomplete_member_access)) {
return CreateRecoveryExpr(Base->getBeginLoc(), Base->getEndLoc(), {Base});
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Sema/SemaExprMember.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1357,7 +1357,7 @@ static ExprResult LookupMemberExpr(Sema &S, LookupResult &R,
BaseType = Ptr->getPointeeType();
else if (BaseType->isFunctionType())
goto fail;
else if (BaseType->isDependentType())
else if (BaseExpr.get()->isTypeDependent())
BaseType = S.Context.DependentTy;
else if (BaseType->isRecordType()) {
// Recover from arrow accesses to records, e.g.:
Expand Down
20 changes: 15 additions & 5 deletions clang/lib/Sema/SemaOverload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15962,10 +15962,9 @@ 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) {
assert(Base->getType()->getAsRecordDecl() &&
"left-hand side must have class type");

if (checkPlaceholderForOverload(*this, Base))
Expand All @@ -15988,9 +15987,20 @@ 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());
R.suppressAccessDiagnostics();

if (Base->getType()->isDependentType() &&
(!R.empty() || R.wasNotFoundInCurrentInstantiation())) {
DeclarationNameInfo OpNameInfo(OpName, OpLoc);
ExprResult Fn = CreateUnresolvedLookupExpr(
/*NamingClass=*/nullptr, /*NNSLoc=*/NestedNameSpecifierLoc(),
OpNameInfo, R.asUnresolvedSet(), /*PerformADL=*/false);
return CXXOperatorCallExpr::Create(Context, OO_Arrow, Fn.get(), Base,
Context.DependentTy, VK_PRValue, OpLoc,
CurFPFeatureOverrides());
}

for (LookupResult::iterator Oper = R.begin(), OperEnd = R.end();
Oper != OperEnd; ++Oper) {
AddMethodCandidate(Oper.getPair(), Base->getType(), Base->Classify(Context),
Expand Down
5 changes: 3 additions & 2 deletions clang/lib/Sema/TreeTransform.h
Original file line number Diff line number Diff line change
Expand Up @@ -17282,10 +17282,11 @@ 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();
// -> is never a builtin operation.
return SemaRef.BuildOverloadedArrowExpr(nullptr, First, OpLoc);
return getSema().BuildOverloadedArrowExpr(First, OpLoc,
/*NoArrowOperatorFound=*/nullptr);
} else if (Second == nullptr || isPostIncDec) {
if (!First->getType()->isOverloadableType() ||
(Op == OO_Amp && getSema().isQualifiedMemberAccess(First))) {
Expand Down
45 changes: 34 additions & 11 deletions clang/test/CXX/temp/temp.res/temp.dep/temp.dep.type/p4.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 '.'!
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unfortunate, it looks like we're having this problem diagnosing in both phases here, and can get worse with partial instantiations. Typically we'd want to solve this by replacing the expression with a RecoveryExpr or something to suppress it in the instantiated version, can you look into something like that?

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}}
}
};
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -605,3 +607,24 @@ namespace N5 {

template void g(int); // expected-note {{in instantiation of}}
} // namespace N5

namespace N6 {
struct A {
int x;
};

struct B {
A* operator->();
};

struct C {
B y;
};

template<typename T>
struct D : C {
void f() {
y->x;
}
};
} // namespace N6
Loading