Skip to content

Commit

Permalink
Revert "Reapply "[Clang][Sema] Fix crash when 'this' is used in a dep…
Browse files Browse the repository at this point in the history
…endent class scope function template specialization that instantiates to a static member function (#87541)" (#88311)"

This reverts commit aa80f3e.

See
#88311 (comment).

There is a fix forward proposed but I am reverting this commit to fix
trunk.
  • Loading branch information
metaflow committed Apr 15, 2024
1 parent 2cc0c21 commit 46131aa
Show file tree
Hide file tree
Showing 9 changed files with 99 additions and 252 deletions.
2 changes: 0 additions & 2 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -529,8 +529,6 @@ Bug Fixes to C++ Support
- Fix an issue caused by not handling invalid cases when substituting into the parameter mapping of a constraint. Fixes (#GH86757).
- Fixed a bug that prevented member function templates of class templates declared with a deduced return type
from being explicitly specialized for a given implicit instantiation of the class template.
- Fixed a crash when ``this`` is used in a dependent class scope function template specialization
that instantiates to a static member function.

- Fix crash when inheriting from a cv-qualified type. Fixes:
(`#35603 <https://github.com/llvm/llvm-project/issues/35603>`_)
Expand Down
16 changes: 4 additions & 12 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -5452,8 +5452,7 @@ class Sema final : public SemaBase {

ExprResult BuildDeclarationNameExpr(const CXXScopeSpec &SS, LookupResult &R,
bool NeedsADL,
bool AcceptInvalidDecl = false,
bool NeedUnresolved = false);
bool AcceptInvalidDecl = false);
ExprResult BuildDeclarationNameExpr(
const CXXScopeSpec &SS, const DeclarationNameInfo &NameInfo, NamedDecl *D,
NamedDecl *FoundD = nullptr,
Expand Down Expand Up @@ -6596,10 +6595,7 @@ class Sema final : public SemaBase {
SourceLocation RParenLoc);

//// ActOnCXXThis - Parse 'this' pointer.
ExprResult ActOnCXXThis(SourceLocation Loc);

/// Check whether the type of 'this' is valid in the current context.
bool CheckCXXThisType(SourceLocation Loc, QualType Type);
ExprResult ActOnCXXThis(SourceLocation loc);

/// Build a CXXThisExpr and mark it referenced in the current context.
Expr *BuildCXXThisExpr(SourceLocation Loc, QualType Type, bool IsImplicit);
Expand Down Expand Up @@ -7022,14 +7018,10 @@ class Sema final : public SemaBase {
///@{

public:
/// Check whether an expression might be an implicit class member access.
bool isPotentialImplicitMemberAccess(const CXXScopeSpec &SS, LookupResult &R,
bool IsAddressOfOperand);

ExprResult BuildPossibleImplicitMemberExpr(
const CXXScopeSpec &SS, SourceLocation TemplateKWLoc, LookupResult &R,
const TemplateArgumentListInfo *TemplateArgs, const Scope *S);

const TemplateArgumentListInfo *TemplateArgs, const Scope *S,
UnresolvedLookupExpr *AsULE = nullptr);
ExprResult
BuildImplicitMemberExpr(const CXXScopeSpec &SS, SourceLocation TemplateKWLoc,
LookupResult &R,
Expand Down
28 changes: 22 additions & 6 deletions clang/lib/Sema/SemaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2917,9 +2917,26 @@ Sema::ActOnIdExpression(Scope *S, CXXScopeSpec &SS,
// to get this right here so that we don't end up making a
// spuriously dependent expression if we're inside a dependent
// instance method.
if (isPotentialImplicitMemberAccess(SS, R, IsAddressOfOperand))
return BuildPossibleImplicitMemberExpr(SS, TemplateKWLoc, R, TemplateArgs,
S);
if (getLangOpts().CPlusPlus && !R.empty() &&
(*R.begin())->isCXXClassMember()) {
bool MightBeImplicitMember;
if (!IsAddressOfOperand)
MightBeImplicitMember = true;
else if (!SS.isEmpty())
MightBeImplicitMember = false;
else if (R.isOverloadedResult())
MightBeImplicitMember = false;
else if (R.isUnresolvableResult())
MightBeImplicitMember = true;
else
MightBeImplicitMember = isa<FieldDecl>(R.getFoundDecl()) ||
isa<IndirectFieldDecl>(R.getFoundDecl()) ||
isa<MSPropertyDecl>(R.getFoundDecl());

if (MightBeImplicitMember)
return BuildPossibleImplicitMemberExpr(SS, TemplateKWLoc,
R, TemplateArgs, S);
}

if (TemplateArgs || TemplateKWLoc.isValid()) {

Expand Down Expand Up @@ -3430,11 +3447,10 @@ static bool ShouldLookupResultBeMultiVersionOverload(const LookupResult &R) {

ExprResult Sema::BuildDeclarationNameExpr(const CXXScopeSpec &SS,
LookupResult &R, bool NeedsADL,
bool AcceptInvalidDecl,
bool NeedUnresolved) {
bool AcceptInvalidDecl) {
// If this is a single, fully-resolved result and we don't need ADL,
// just build an ordinary singleton decl ref.
if (!NeedUnresolved && !NeedsADL && R.isSingleResult() &&
if (!NeedsADL && R.isSingleResult() &&
!R.getAsSingle<FunctionTemplateDecl>() &&
!ShouldLookupResultBeMultiVersionOverload(R))
return BuildDeclarationNameExpr(SS, R.getLookupNameInfo(), R.getFoundDecl(),
Expand Down
61 changes: 29 additions & 32 deletions clang/lib/Sema/SemaExprCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1416,42 +1416,26 @@ bool Sema::CheckCXXThisCapture(SourceLocation Loc, const bool Explicit,
}

ExprResult Sema::ActOnCXXThis(SourceLocation Loc) {
// C++20 [expr.prim.this]p1:
// The keyword this names a pointer to the object for which an
// implicit object member function is invoked or a non-static
// data member's initializer is evaluated.
/// C++ 9.3.2: In the body of a non-static member function, the keyword this
/// is a non-lvalue expression whose value is the address of the object for
/// which the function is called.
QualType ThisTy = getCurrentThisType();

if (CheckCXXThisType(Loc, ThisTy))
return ExprError();
if (ThisTy.isNull()) {
DeclContext *DC = getFunctionLevelDeclContext();

return BuildCXXThisExpr(Loc, ThisTy, /*IsImplicit=*/false);
}
if (const auto *Method = dyn_cast<CXXMethodDecl>(DC);
Method && Method->isExplicitObjectMemberFunction()) {
return Diag(Loc, diag::err_invalid_this_use) << 1;
}

bool Sema::CheckCXXThisType(SourceLocation Loc, QualType Type) {
if (!Type.isNull())
return false;
if (isLambdaCallWithExplicitObjectParameter(CurContext))
return Diag(Loc, diag::err_invalid_this_use) << 1;

// C++20 [expr.prim.this]p3:
// If a declaration declares a member function or member function template
// of a class X, the expression this is a prvalue of type
// "pointer to cv-qualifier-seq X" wherever X is the current class between
// the optional cv-qualifier-seq and the end of the function-definition,
// member-declarator, or declarator. It shall not appear within the
// declaration of either a static member function or an explicit object
// member function of the current class (although its type and value
// category are defined within such member functions as they are within
// an implicit object member function).
DeclContext *DC = getFunctionLevelDeclContext();
if (const auto *Method = dyn_cast<CXXMethodDecl>(DC);
Method && Method->isExplicitObjectMemberFunction()) {
Diag(Loc, diag::err_invalid_this_use) << 1;
} else if (isLambdaCallWithExplicitObjectParameter(CurContext)) {
Diag(Loc, diag::err_invalid_this_use) << 1;
} else {
Diag(Loc, diag::err_invalid_this_use) << 0;
return Diag(Loc, diag::err_invalid_this_use) << 0;
}
return true;

return BuildCXXThisExpr(Loc, ThisTy, /*IsImplicit=*/false);
}

Expr *Sema::BuildCXXThisExpr(SourceLocation Loc, QualType Type,
Expand Down Expand Up @@ -8658,8 +8642,21 @@ static ExprResult attemptRecovery(Sema &SemaRef,

// Detect and handle the case where the decl might be an implicit
// member.
if (SemaRef.isPotentialImplicitMemberAccess(
NewSS, R, Consumer.isAddressOfOperand()))
bool MightBeImplicitMember;
if (!Consumer.isAddressOfOperand())
MightBeImplicitMember = true;
else if (!NewSS.isEmpty())
MightBeImplicitMember = false;
else if (R.isOverloadedResult())
MightBeImplicitMember = false;
else if (R.isUnresolvableResult())
MightBeImplicitMember = true;
else
MightBeImplicitMember = isa<FieldDecl>(ND) ||
isa<IndirectFieldDecl>(ND) ||
isa<MSPropertyDecl>(ND);

if (MightBeImplicitMember)
return SemaRef.BuildPossibleImplicitMemberExpr(
NewSS, /*TemplateKWLoc*/ SourceLocation(), R,
/*TemplateArgs*/ nullptr, /*S*/ nullptr);
Expand Down
63 changes: 14 additions & 49 deletions clang/lib/Sema/SemaExprMember.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,6 @@ enum IMAKind {
/// The reference is a contextually-permitted abstract member reference.
IMA_Abstract,

/// Whether the context is static is dependent on the enclosing template (i.e.
/// in a dependent class scope explicit specialization).
IMA_Dependent,

/// The reference may be to an unresolved using declaration and the
/// context is not an instance method.
IMA_Unresolved_StaticOrExplicitContext,
Expand Down Expand Up @@ -95,18 +91,10 @@ static IMAKind ClassifyImplicitMemberAccess(Sema &SemaRef,

DeclContext *DC = SemaRef.getFunctionLevelDeclContext();

bool couldInstantiateToStatic = false;
bool isStaticOrExplicitContext = SemaRef.CXXThisTypeOverride.isNull();

if (auto *MD = dyn_cast<CXXMethodDecl>(DC)) {
if (MD->isImplicitObjectMemberFunction()) {
isStaticOrExplicitContext = false;
// A dependent class scope function template explicit specialization
// that is neither declared 'static' nor with an explicit object
// parameter could instantiate to a static or non-static member function.
couldInstantiateToStatic = MD->getDependentSpecializationInfo();
}
}
bool isStaticOrExplicitContext =
SemaRef.CXXThisTypeOverride.isNull() &&
(!isa<CXXMethodDecl>(DC) || cast<CXXMethodDecl>(DC)->isStatic() ||
cast<CXXMethodDecl>(DC)->isExplicitObjectMemberFunction());

if (R.isUnresolvableResult())
return isStaticOrExplicitContext ? IMA_Unresolved_StaticOrExplicitContext
Expand Down Expand Up @@ -135,9 +123,6 @@ static IMAKind ClassifyImplicitMemberAccess(Sema &SemaRef,
if (Classes.empty())
return IMA_Static;

if (couldInstantiateToStatic)
return IMA_Dependent;

// C++11 [expr.prim.general]p12:
// An id-expression that denotes a non-static data member or non-static
// member function of a class can only be used:
Expand Down Expand Up @@ -278,52 +263,32 @@ static void diagnoseInstanceReference(Sema &SemaRef,
}
}

bool Sema::isPotentialImplicitMemberAccess(const CXXScopeSpec &SS,
LookupResult &R,
bool IsAddressOfOperand) {
if (!getLangOpts().CPlusPlus)
return false;
else if (R.empty() || !R.begin()->isCXXClassMember())
return false;
else if (!IsAddressOfOperand)
return true;
else if (!SS.isEmpty())
return false;
else if (R.isOverloadedResult())
return false;
else if (R.isUnresolvableResult())
return true;
else
return isa<FieldDecl, IndirectFieldDecl, MSPropertyDecl>(R.getFoundDecl());
}

/// Builds an expression which might be an implicit member expression.
ExprResult Sema::BuildPossibleImplicitMemberExpr(
const CXXScopeSpec &SS, SourceLocation TemplateKWLoc, LookupResult &R,
const TemplateArgumentListInfo *TemplateArgs, const Scope *S) {
switch (IMAKind Classification = ClassifyImplicitMemberAccess(*this, R)) {
const TemplateArgumentListInfo *TemplateArgs, const Scope *S,
UnresolvedLookupExpr *AsULE) {
switch (ClassifyImplicitMemberAccess(*this, R)) {
case IMA_Instance:
return BuildImplicitMemberExpr(SS, TemplateKWLoc, R, TemplateArgs, true, S);

case IMA_Mixed:
case IMA_Mixed_Unrelated:
case IMA_Unresolved:
return BuildImplicitMemberExpr(
SS, TemplateKWLoc, R, TemplateArgs,
/*IsKnownInstance=*/Classification == IMA_Instance, S);
return BuildImplicitMemberExpr(SS, TemplateKWLoc, R, TemplateArgs, false,
S);

case IMA_Field_Uneval_Context:
Diag(R.getNameLoc(), diag::warn_cxx98_compat_non_static_member_use)
<< R.getLookupNameInfo().getName();
[[fallthrough]];
case IMA_Static:
case IMA_Abstract:
case IMA_Dependent:
case IMA_Mixed_StaticOrExplicitContext:
case IMA_Unresolved_StaticOrExplicitContext:
if (TemplateArgs || TemplateKWLoc.isValid())
return BuildTemplateIdExpr(SS, TemplateKWLoc, R, /*RequiresADL=*/false,
TemplateArgs);
return BuildDeclarationNameExpr(
SS, R, /*NeedsADL=*/false, /*AcceptInvalidDecl=*/false,
/*NeedUnresolved=*/Classification == IMA_Dependent);
return BuildTemplateIdExpr(SS, TemplateKWLoc, R, false, TemplateArgs);
return AsULE ? AsULE : BuildDeclarationNameExpr(SS, R, false);

case IMA_Error_StaticOrExplicitContext:
case IMA_Error_Unrelated:
Expand Down
8 changes: 0 additions & 8 deletions clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5097,14 +5097,6 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
EnterExpressionEvaluationContext EvalContext(
*this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated);

Qualifiers ThisTypeQuals;
CXXRecordDecl *ThisContext = nullptr;
if (CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(Function)) {
ThisContext = Method->getParent();
ThisTypeQuals = Method->getMethodQualifiers();
}
CXXThisScopeRAII ThisScope(*this, ThisContext, ThisTypeQuals);

// Introduce a new scope where local variable instantiations will be
// recorded, unless we're actually a member function within a local
// class, in which case we need to merge our results with the parent
Expand Down

0 comments on commit 46131aa

Please sign in to comment.