[clang] Reapply "[Clang][Sema] Fix crash when 'this' is used in a dependent class scope function template specialization that instantiates to a static member function (#87541)" (PR #88311)
Krystian Stasiowski via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 10 12:14:30 PDT 2024
https://github.com/sdkrystian created https://github.com/llvm/llvm-project/pull/88311
Reapplies #87541 and addresses the bug which caused expressions naming overload sets to be incorrectly rebuilt.
>From f6d703151ec06cec140c4d425dc2bbd8b7ac4f93 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Tue, 9 Apr 2024 08:31:52 -0400
Subject: [PATCH] Reapply "[Clang][Sema] Fix crash when 'this' is used in a
dependent class scope function template specialization that instantiates to a
static member function (#87541)"
This patch fixes a crash that happens when '`this`' is referenced
(implicitly or explicitly) in a dependent class scope function template
specialization that instantiates to a static member function. For
example:
```
template<typename T>
struct A
{
template<typename U>
static void f();
template<>
void f<int>()
{
this; // causes crash during instantiation
}
};
template struct A<int>;
```
This happens because during instantiation of the function body,
`Sema::getCurrentThisType` will return a null `QualType` which we
rebuild the `CXXThisExpr` with. A similar problem exists for implicit
class member access expressions in such contexts (which shouldn't really
happen within templates anyways per [class.mfct.non.static]
p2, but changing that is non-trivial). This patch fixes the crash by building
`UnresolvedLookupExpr`s instead of `MemberExpr`s for these implicit
member accesses, which will then be correctly rebuilt as `MemberExpr`s
during instantiation.
---
clang/docs/ReleaseNotes.rst | 2 +
clang/include/clang/Sema/Sema.h | 8 ++-
clang/lib/Sema/SemaExpr.cpp | 5 +-
clang/lib/Sema/SemaExprCXX.cpp | 44 +++++++++-----
clang/lib/Sema/SemaExprMember.cpp | 42 +++++++++----
.../lib/Sema/SemaTemplateInstantiateDecl.cpp | 8 +++
clang/lib/Sema/TreeTransform.h | 44 ++++++++++++--
...ms-function-specialization-class-scope.cpp | 59 ++++++++++++++++++-
8 files changed, 174 insertions(+), 38 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f5359afe1f0999..6bff80ed4d210b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -520,6 +520,8 @@ 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>`_)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 9769d36900664c..f311f9f3743454 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -5439,7 +5439,8 @@ class Sema final : public SemaBase {
ExprResult BuildDeclarationNameExpr(const CXXScopeSpec &SS, LookupResult &R,
bool NeedsADL,
- bool AcceptInvalidDecl = false);
+ bool AcceptInvalidDecl = false,
+ bool NeedUnresolved = false);
ExprResult BuildDeclarationNameExpr(
const CXXScopeSpec &SS, const DeclarationNameInfo &NameInfo, NamedDecl *D,
NamedDecl *FoundD = nullptr,
@@ -6591,7 +6592,10 @@ class Sema final : public SemaBase {
SourceLocation RParenLoc);
//// ActOnCXXThis - Parse 'this' pointer.
- ExprResult ActOnCXXThis(SourceLocation loc);
+ ExprResult ActOnCXXThis(SourceLocation Loc);
+
+ /// Check whether the type of 'this' is valid in the current context.
+ bool CheckCXXThisType(SourceLocation Loc, QualType Type);
/// Build a CXXThisExpr and mark it referenced in the current context.
Expr *BuildCXXThisExpr(SourceLocation Loc, QualType Type, bool IsImplicit);
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 594c11788f4e7e..45acbf197ea6b4 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -3442,10 +3442,11 @@ static bool ShouldLookupResultBeMultiVersionOverload(const LookupResult &R) {
ExprResult Sema::BuildDeclarationNameExpr(const CXXScopeSpec &SS,
LookupResult &R, bool NeedsADL,
- bool AcceptInvalidDecl) {
+ bool AcceptInvalidDecl,
+ bool NeedUnresolved) {
// If this is a single, fully-resolved result and we don't need ADL,
// just build an ordinary singleton decl ref.
- if (!NeedsADL && R.isSingleResult() &&
+ if (!NeedUnresolved && !NeedsADL && R.isSingleResult() &&
!R.getAsSingle<FunctionTemplateDecl>() &&
!ShouldLookupResultBeMultiVersionOverload(R))
return BuildDeclarationNameExpr(SS, R.getLookupNameInfo(), R.getFoundDecl(),
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 7b9b8f149d9edd..9822477260e592 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -1415,26 +1415,42 @@ bool Sema::CheckCXXThisCapture(SourceLocation Loc, const bool Explicit,
}
ExprResult Sema::ActOnCXXThis(SourceLocation Loc) {
- /// 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.
+ // 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.
QualType ThisTy = getCurrentThisType();
- if (ThisTy.isNull()) {
- DeclContext *DC = getFunctionLevelDeclContext();
+ if (CheckCXXThisType(Loc, ThisTy))
+ return ExprError();
- if (const auto *Method = dyn_cast<CXXMethodDecl>(DC);
- Method && Method->isExplicitObjectMemberFunction()) {
- return Diag(Loc, diag::err_invalid_this_use) << 1;
- }
+ return BuildCXXThisExpr(Loc, ThisTy, /*IsImplicit=*/false);
+}
- if (isLambdaCallWithExplicitObjectParameter(CurContext))
- return Diag(Loc, diag::err_invalid_this_use) << 1;
+bool Sema::CheckCXXThisType(SourceLocation Loc, QualType Type) {
+ if (!Type.isNull())
+ return false;
- return Diag(Loc, diag::err_invalid_this_use) << 0;
+ // 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 BuildCXXThisExpr(Loc, ThisTy, /*IsImplicit=*/false);
+ return true;
}
Expr *Sema::BuildCXXThisExpr(SourceLocation Loc, QualType Type,
diff --git a/clang/lib/Sema/SemaExprMember.cpp b/clang/lib/Sema/SemaExprMember.cpp
index 32998ae60eafe2..8cd2288d279cc7 100644
--- a/clang/lib/Sema/SemaExprMember.cpp
+++ b/clang/lib/Sema/SemaExprMember.cpp
@@ -61,6 +61,10 @@ 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,
@@ -91,10 +95,18 @@ static IMAKind ClassifyImplicitMemberAccess(Sema &SemaRef,
DeclContext *DC = SemaRef.getFunctionLevelDeclContext();
- bool isStaticOrExplicitContext =
- SemaRef.CXXThisTypeOverride.isNull() &&
- (!isa<CXXMethodDecl>(DC) || cast<CXXMethodDecl>(DC)->isStatic() ||
- cast<CXXMethodDecl>(DC)->isExplicitObjectMemberFunction());
+ 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();
+ }
+ }
if (R.isUnresolvableResult())
return isStaticOrExplicitContext ? IMA_Unresolved_StaticOrExplicitContext
@@ -123,6 +135,9 @@ 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:
@@ -268,27 +283,30 @@ ExprResult Sema::BuildPossibleImplicitMemberExpr(
const CXXScopeSpec &SS, SourceLocation TemplateKWLoc, LookupResult &R,
const TemplateArgumentListInfo *TemplateArgs, const Scope *S,
UnresolvedLookupExpr *AsULE) {
- switch (ClassifyImplicitMemberAccess(*this, R)) {
+ switch (IMAKind Classification = 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, false,
- S);
-
+ return BuildImplicitMemberExpr(
+ SS, TemplateKWLoc, R, TemplateArgs,
+ /*IsKnownInstance=*/Classification == IMA_Instance, 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, false, TemplateArgs);
- return AsULE ? AsULE : BuildDeclarationNameExpr(SS, R, false);
+ return BuildTemplateIdExpr(SS, TemplateKWLoc, R, /*RequiresADL=*/false,
+ TemplateArgs);
+ return AsULE ? AsULE
+ : BuildDeclarationNameExpr(
+ SS, R, /*NeedsADL=*/false, /*AcceptInvalidDecl=*/false,
+ /*NeedUnresolved=*/Classification == IMA_Dependent);
case IMA_Error_StaticOrExplicitContext:
case IMA_Error_Unrelated:
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 127a432367b95d..8248b10814fea5 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5093,6 +5093,14 @@ 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
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index d4d2fa61d65ea2..6e03ce5863b496 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -794,6 +794,9 @@ class TreeTransform {
ParenExpr *PE, DependentScopeDeclRefExpr *DRE, bool IsAddressOfOperand,
TypeSourceInfo **RecoveryTSI);
+ ExprResult TransformUnresolvedLookupExpr(UnresolvedLookupExpr *E,
+ bool IsAddressOfOperand);
+
StmtResult TransformOMPExecutableDirective(OMPExecutableDirective *S);
// FIXME: We use LLVM_ATTRIBUTE_NOINLINE because inlining causes a ridiculous
@@ -3307,12 +3310,13 @@ class TreeTransform {
/// Build a new C++ "this" expression.
///
- /// By default, builds a new "this" expression without performing any
- /// semantic analysis. Subclasses may override this routine to provide
- /// different behavior.
+ /// By default, performs semantic analysis to build a new "this" expression.
+ /// Subclasses may override this routine to provide different behavior.
ExprResult RebuildCXXThisExpr(SourceLocation ThisLoc,
QualType ThisType,
bool isImplicit) {
+ if (getSema().CheckCXXThisType(ThisLoc, ThisType))
+ return ExprError();
return getSema().BuildCXXThisExpr(ThisLoc, ThisType, isImplicit);
}
@@ -11313,7 +11317,9 @@ template<typename Derived>
ExprResult
TreeTransform<Derived>::TransformAddressOfOperand(Expr *E) {
if (DependentScopeDeclRefExpr *DRE = dyn_cast<DependentScopeDeclRefExpr>(E))
- return getDerived().TransformDependentScopeDeclRefExpr(DRE, true, nullptr);
+ return getDerived().TransformDependentScopeDeclRefExpr(DRE, /*IsAddressOfOperand=*/true, nullptr);
+ else if (UnresolvedLookupExpr *ULE = dyn_cast<UnresolvedLookupExpr>(E))
+ return getDerived().TransformUnresolvedLookupExpr(ULE, /*IsAddressOfOperand=*/true);
else
return getDerived().TransformExpr(E);
}
@@ -13019,10 +13025,17 @@ bool TreeTransform<Derived>::TransformOverloadExprDecls(OverloadExpr *Old,
return false;
}
+template<typename Derived>
+ExprResult
+TreeTransform<Derived>::TransformUnresolvedLookupExpr(UnresolvedLookupExpr *Old) {
+ return TransformUnresolvedLookupExpr(Old, /*IsAddressOfOperand=*/false);
+}
+
template<typename Derived>
ExprResult
TreeTransform<Derived>::TransformUnresolvedLookupExpr(
- UnresolvedLookupExpr *Old) {
+ UnresolvedLookupExpr *Old,
+ bool IsAddressOfOperand) {
LookupResult R(SemaRef, Old->getName(), Old->getNameLoc(),
Sema::LookupOrdinaryName);
@@ -13056,6 +13069,7 @@ TreeTransform<Derived>::TransformUnresolvedLookupExpr(
SourceLocation TemplateKWLoc = Old->getTemplateKeywordLoc();
+ #if 0
// If we have neither explicit template arguments, nor the template keyword,
// it's a normal declaration name or member reference.
if (!Old->hasExplicitTemplateArgs() && !TemplateKWLoc.isValid()) {
@@ -13071,6 +13085,17 @@ TreeTransform<Derived>::TransformUnresolvedLookupExpr(
return getDerived().RebuildDeclarationNameExpr(SS, R, Old->requiresADL());
}
+ #endif
+
+ bool PotentiallyImplicitAccess =
+ R.isClassLookup() && (!IsAddressOfOperand ||
+ (R.isSingleResult() && R.getAsSingle<NamedDecl>()->isCXXInstanceMember()));
+
+ // If we have neither explicit template arguments, nor the template keyword,
+ // it's a normal declaration name or member reference.
+ if (!PotentiallyImplicitAccess && !Old->hasExplicitTemplateArgs() && !TemplateKWLoc.isValid()) {
+ return getDerived().RebuildDeclarationNameExpr(SS, R, Old->requiresADL());
+ }
// If we have template arguments, rebuild them, then rebuild the
// templateid expression.
@@ -13083,6 +13108,15 @@ TreeTransform<Derived>::TransformUnresolvedLookupExpr(
return ExprError();
}
+ // In a C++11 unevaluated context, an UnresolvedLookupExpr might refer to an
+ // instance member. In other contexts, BuildPossibleImplicitMemberExpr will
+ // give a good diagnostic.
+ if (PotentiallyImplicitAccess) {
+ return SemaRef.BuildPossibleImplicitMemberExpr(SS, TemplateKWLoc, R,
+ Old->hasExplicitTemplateArgs() ? &TransArgs : nullptr,
+ /*Scope=*/nullptr);
+ }
+
return getDerived().RebuildTemplateIdExpr(SS, TemplateKWLoc, R,
Old->requiresADL(), &TransArgs);
}
diff --git a/clang/test/SemaTemplate/ms-function-specialization-class-scope.cpp b/clang/test/SemaTemplate/ms-function-specialization-class-scope.cpp
index dcab9bfaeabcb0..879e929b38414a 100644
--- a/clang/test/SemaTemplate/ms-function-specialization-class-scope.cpp
+++ b/clang/test/SemaTemplate/ms-function-specialization-class-scope.cpp
@@ -1,7 +1,6 @@
-// RUN: %clang_cc1 -fms-extensions -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fms-extensions -fdelayed-template-parsing -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fms-extensions -fsyntax-only -Wno-unused-value -verify %s
+// RUN: %clang_cc1 -fms-extensions -fdelayed-template-parsing -fsyntax-only -Wno-unused-value -verify %s
-// expected-no-diagnostics
class A {
public:
template<class U> A(U p) {}
@@ -76,3 +75,57 @@ struct S {
int f<0>(int);
};
}
+
+namespace UsesThis {
+ template<typename T>
+ struct A {
+ int x;
+
+ template<typename U = void>
+ static void f();
+
+ template<>
+ void f<int>() {
+ this->x; // expected-error {{invalid use of 'this' outside of a non-static member function}}
+ x; // expected-error {{invalid use of member 'x' in static member function}}
+ A::x; // expected-error {{invalid use of member 'x' in static member function}}
+ +x; // expected-error {{invalid use of member 'x' in static member function}}
+ +A::x; // expected-error {{invalid use of member 'x' in static member function}}
+ f();
+ f<void>();
+ g(); // expected-error {{call to non-static member function without an object argument}}
+ g<void>(); // expected-error {{call to non-static member function without an object argument}}
+ i(); // expected-error {{call to non-static member function without an object argument}}
+ j();
+ }
+
+ template<typename U = void>
+ void g();
+
+ template<>
+ void g<int>() {
+ this->x;
+ x;
+ A::x;
+ +x;
+ +A::x;
+ f();
+ f<void>();
+ g();
+ g<void>();
+ i();
+ j();
+ }
+
+ template<typename U>
+ static auto h() -> A*;
+
+ template<>
+ auto h<int>() -> decltype(this); // expected-error {{'this' cannot be used in a static member function declaration}}
+
+ void i();
+ static void j();
+ };
+
+ template struct A<int>; // expected-note 2{{in instantiation of}}
+}
More information about the cfe-commits
mailing list