[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
Thu Apr 11 08:02:51 PDT 2024


https://github.com/sdkrystian updated https://github.com/llvm/llvm-project/pull/88311

>From eb389e142b18d1a14d23d9fadea3c503331c2f73 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 1/5] 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                | 53 ++++++++++++++---
 ...ms-function-specialization-class-scope.cpp | 59 ++++++++++++++++++-
 8 files changed, 181 insertions(+), 40 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..c8147b66200271 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,11 @@ 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 +13027,16 @@ bool TreeTransform<Derived>::TransformOverloadExprDecls(OverloadExpr *Old,
   return false;
 }
 
-template<typename Derived>
+template <typename Derived>
+ExprResult TreeTransform<Derived>::TransformUnresolvedLookupExpr(
+    UnresolvedLookupExpr *Old) {
+  return TransformUnresolvedLookupExpr(Old, /*IsAddressOfOperand=*/false);
+}
+
+template <typename Derived>
 ExprResult
-TreeTransform<Derived>::TransformUnresolvedLookupExpr(
-                                                  UnresolvedLookupExpr *Old) {
+TreeTransform<Derived>::TransformUnresolvedLookupExpr(UnresolvedLookupExpr *Old,
+                                                      bool IsAddressOfOperand) {
   LookupResult R(SemaRef, Old->getName(), Old->getNameLoc(),
                  Sema::LookupOrdinaryName);
 
@@ -13056,6 +13070,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 +13086,20 @@ 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 +13112,16 @@ 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}}
+}

>From 4dbbfdd38349b0c9b4ab9d4e7ab8de2a18d9be53 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Thu, 11 Apr 2024 09:34:14 -0400
Subject: [PATCH 2/5] [FOLD]

---
 clang/include/clang/Sema/Sema.h               |  8 +++--
 clang/lib/Sema/SemaExpr.cpp                   |  6 ++++
 clang/lib/Sema/SemaExprCXX.cpp                | 17 ++---------
 clang/lib/Sema/SemaExprMember.cpp             | 29 +++++++++++++++----
 clang/lib/Sema/TreeTransform.h                |  7 +++++
 .../SemaTemplate/instantiate-using-decl.cpp   |  2 +-
 6 files changed, 45 insertions(+), 24 deletions(-)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index f311f9f3743454..2c6bbbefc04957 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -7018,10 +7018,14 @@ class Sema final : public SemaBase {
   ///@{
 
 public:
+  bool isPotentialImplicitMemberAccess(const CXXScopeSpec &SS,
+                                       LookupResult &R,
+                                       bool IsAddressOfOperand);
+
   ExprResult BuildPossibleImplicitMemberExpr(
       const CXXScopeSpec &SS, SourceLocation TemplateKWLoc, LookupResult &R,
-      const TemplateArgumentListInfo *TemplateArgs, const Scope *S,
-      UnresolvedLookupExpr *AsULE = nullptr);
+      const TemplateArgumentListInfo *TemplateArgs, const Scope *S);
+
   ExprResult
   BuildImplicitMemberExpr(const CXXScopeSpec &SS, SourceLocation TemplateKWLoc,
                           LookupResult &R,
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 45acbf197ea6b4..969ff134f9103c 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -2912,6 +2912,7 @@ 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 0
   if (getLangOpts().CPlusPlus && !R.empty() &&
       (*R.begin())->isCXXClassMember()) {
     bool MightBeImplicitMember;
@@ -2932,6 +2933,11 @@ Sema::ActOnIdExpression(Scope *S, CXXScopeSpec &SS,
       return BuildPossibleImplicitMemberExpr(SS, TemplateKWLoc,
                                              R, TemplateArgs, S);
   }
+  #else
+  if (isPotentialImplicitMemberAccess(SS, R, IsAddressOfOperand))
+    return BuildPossibleImplicitMemberExpr(SS, TemplateKWLoc,
+                                           R, TemplateArgs, S);
+  #endif
 
   if (TemplateArgs || TemplateKWLoc.isValid()) {
 
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 9822477260e592..84098a656d571e 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -8643,21 +8643,8 @@ static ExprResult attemptRecovery(Sema &SemaRef,
 
       // Detect and handle the case where the decl might be an implicit
       // member.
-      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)
+      if (SemaRef.isPotentialImplicitMemberAccess(
+          NewSS, R, Consumer.isAddressOfOperand()))
         return SemaRef.BuildPossibleImplicitMemberExpr(
             NewSS, /*TemplateKWLoc*/ SourceLocation(), R,
             /*TemplateArgs*/ nullptr, /*S*/ nullptr);
diff --git a/clang/lib/Sema/SemaExprMember.cpp b/clang/lib/Sema/SemaExprMember.cpp
index 8cd2288d279cc7..eeac753a348979 100644
--- a/clang/lib/Sema/SemaExprMember.cpp
+++ b/clang/lib/Sema/SemaExprMember.cpp
@@ -278,11 +278,29 @@ 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,
-    UnresolvedLookupExpr *AsULE) {
+    const TemplateArgumentListInfo *TemplateArgs, const Scope *S) {
   switch (IMAKind Classification = ClassifyImplicitMemberAccess(*this, R)) {
   case IMA_Instance:
   case IMA_Mixed:
@@ -303,10 +321,9 @@ ExprResult Sema::BuildPossibleImplicitMemberExpr(
     if (TemplateArgs || TemplateKWLoc.isValid())
       return BuildTemplateIdExpr(SS, TemplateKWLoc, R, /*RequiresADL=*/false,
                                  TemplateArgs);
-    return AsULE ? AsULE
-                 : BuildDeclarationNameExpr(
-                       SS, R, /*NeedsADL=*/false, /*AcceptInvalidDecl=*/false,
-                       /*NeedUnresolved=*/Classification == IMA_Dependent);
+    return 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/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index c8147b66200271..e1e2ec51fedc2c 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -13089,10 +13089,17 @@ TreeTransform<Derived>::TransformUnresolvedLookupExpr(UnresolvedLookupExpr *Old,
 #endif
 
   bool PotentiallyImplicitAccess =
+  #if 0
       R.isClassLookup() &&
       (!IsAddressOfOperand ||
        (R.isSingleResult() &&
         R.getAsSingle<NamedDecl>()->isCXXInstanceMember()));
+  #elif 0
+      !IsAddressOfOperand && !R.empty() && R.begin()->isCXXClassMember();
+  #elif 1
+      SemaRef.isPotentialImplicitMemberAccess(SS, R, IsAddressOfOperand);
+  #endif
+
 
   // If we have neither explicit template arguments, nor the template keyword,
   // it's a normal declaration name or member reference.
diff --git a/clang/test/SemaTemplate/instantiate-using-decl.cpp b/clang/test/SemaTemplate/instantiate-using-decl.cpp
index 0bbb3ca9c88c8b..28d83764385131 100644
--- a/clang/test/SemaTemplate/instantiate-using-decl.cpp
+++ b/clang/test/SemaTemplate/instantiate-using-decl.cpp
@@ -121,7 +121,7 @@ template <typename Scalar> struct Derived : Base<Scalar> {
     (void)&field;
     // expected-error at +1 {{call to non-static member function without an object argument}}
     (void)method;
-    // expected-error at +1 {{call to non-static member function without an object argument}}
+    // expected-error at +1 {{must explicitly qualify name of member function when taking its address}}
     (void)&method;
     // expected-error at +1 {{call to non-static member function without an object argument}}
     method();

>From b68ea7b5a4a8b3a3b605e850504dd5fd13363211 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Thu, 11 Apr 2024 09:49:36 -0400
Subject: [PATCH 3/5] [FOLD]

---
 clang/include/clang/Sema/Sema.h |  4 +--
 clang/lib/Sema/SemaExpr.cpp     | 27 ++-------------
 clang/lib/Sema/SemaExprCXX.cpp  |  2 +-
 clang/lib/Sema/TreeTransform.h  | 61 ++++++++-------------------------
 4 files changed, 19 insertions(+), 75 deletions(-)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 2c6bbbefc04957..d328e11cf90514 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -7018,8 +7018,8 @@ class Sema final : public SemaBase {
   ///@{
 
 public:
-  bool isPotentialImplicitMemberAccess(const CXXScopeSpec &SS,
-                                       LookupResult &R,
+  /// Check whether an expression might be an implicit class member access.
+  bool isPotentialImplicitMemberAccess(const CXXScopeSpec &SS, LookupResult &R,
                                        bool IsAddressOfOperand);
 
   ExprResult BuildPossibleImplicitMemberExpr(
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 969ff134f9103c..10e4b8f942799a 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -2912,32 +2912,9 @@ 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 0
-  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);
-  }
-  #else
   if (isPotentialImplicitMemberAccess(SS, R, IsAddressOfOperand))
-    return BuildPossibleImplicitMemberExpr(SS, TemplateKWLoc,
-                                           R, TemplateArgs, S);
-  #endif
+    return BuildPossibleImplicitMemberExpr(SS, TemplateKWLoc, R, TemplateArgs,
+                                           S);
 
   if (TemplateArgs || TemplateKWLoc.isValid()) {
 
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 84098a656d571e..295efce0987ed5 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -8644,7 +8644,7 @@ 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()))
+              NewSS, R, Consumer.isAddressOfOperand()))
         return SemaRef.BuildPossibleImplicitMemberExpr(
             NewSS, /*TemplateKWLoc*/ SourceLocation(), R,
             /*TemplateArgs*/ nullptr, /*S*/ nullptr);
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index e1e2ec51fedc2c..7651843b06e3ab 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -13068,48 +13068,8 @@ TreeTransform<Derived>::TransformUnresolvedLookupExpr(UnresolvedLookupExpr *Old,
     R.setNamingClass(NamingClass);
   }
 
+  // Rebuild the template arguments, if any.
   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()) {
-    NamedDecl *D = R.getAsSingle<NamedDecl>();
-    // In a C++11 unevaluated context, an UnresolvedLookupExpr might refer to an
-    // instance member. In other contexts, BuildPossibleImplicitMemberExpr will
-    // give a good diagnostic.
-    if (D && D->isCXXInstanceMember()) {
-      return SemaRef.BuildPossibleImplicitMemberExpr(SS, TemplateKWLoc, R,
-                                                     /*TemplateArgs=*/nullptr,
-                                                     /*Scope=*/nullptr);
-    }
-
-    return getDerived().RebuildDeclarationNameExpr(SS, R, Old->requiresADL());
-  }
-#endif
-
-  bool PotentiallyImplicitAccess =
-  #if 0
-      R.isClassLookup() &&
-      (!IsAddressOfOperand ||
-       (R.isSingleResult() &&
-        R.getAsSingle<NamedDecl>()->isCXXInstanceMember()));
-  #elif 0
-      !IsAddressOfOperand && !R.empty() && R.begin()->isCXXClassMember();
-  #elif 1
-      SemaRef.isPotentialImplicitMemberAccess(SS, R, IsAddressOfOperand);
-  #endif
-
-
-  // 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.
   TemplateArgumentListInfo TransArgs(Old->getLAngleLoc(), Old->getRAngleLoc());
   if (Old->hasExplicitTemplateArgs() &&
       getDerived().TransformTemplateArguments(Old->getTemplateArgs(),
@@ -13119,16 +13079,23 @@ TreeTransform<Derived>::TransformUnresolvedLookupExpr(UnresolvedLookupExpr *Old,
     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) {
+  // An UnresolvedLookupExpr can refer to a class member. This occurs e.g. when
+  // a non-static data member is named in an unevaluated operand, or when
+  // a member is named in a dependent class scope function template explicit
+  // specialization that is neither declared static nor with an explicit object
+  // parameter.
+  if (SemaRef.isPotentialImplicitMemberAccess(SS, R, IsAddressOfOperand))
     return SemaRef.BuildPossibleImplicitMemberExpr(
         SS, TemplateKWLoc, R,
         Old->hasExplicitTemplateArgs() ? &TransArgs : nullptr,
-        /*Scope=*/nullptr);
-  }
+        /*S=*/nullptr);
+
+  // 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())
+    return getDerived().RebuildDeclarationNameExpr(SS, R, Old->requiresADL());
 
+  // If we have template arguments, then rebuild the template-id expression.
   return getDerived().RebuildTemplateIdExpr(SS, TemplateKWLoc, R,
                                             Old->requiresADL(), &TransArgs);
 }

>From d118499d8005fcc9fe4890de67ebe03a76deb985 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Thu, 11 Apr 2024 10:29:06 -0400
Subject: [PATCH 4/5] [FOLD] add test causing regression

---
 .../ms-function-specialization-class-scope.cpp | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/clang/test/SemaTemplate/ms-function-specialization-class-scope.cpp b/clang/test/SemaTemplate/ms-function-specialization-class-scope.cpp
index 879e929b38414a..e75c569bd69773 100644
--- a/clang/test/SemaTemplate/ms-function-specialization-class-scope.cpp
+++ b/clang/test/SemaTemplate/ms-function-specialization-class-scope.cpp
@@ -128,4 +128,22 @@ namespace UsesThis {
   };
 
   template struct A<int>; // expected-note 2{{in instantiation of}}
+
+  template <typename T>
+  struct Foo {
+    template <typename X>
+    int bar(X x) {
+      return 0;
+    }
+
+    template <>
+    int bar(int x) {
+      return bar(5.0); // ok
+    }
+  };
+
+  void call() {
+    Foo<double> f;
+    f.bar(1);
+  }
 }

>From 3a5c4a578ce5c5a336e946d04c3c0a042c1f1538 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Thu, 11 Apr 2024 11:02:33 -0400
Subject: [PATCH 5/5] [FOLD] add tests for address of operator

---
 .../ms-function-specialization-class-scope.cpp     | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/clang/test/SemaTemplate/ms-function-specialization-class-scope.cpp b/clang/test/SemaTemplate/ms-function-specialization-class-scope.cpp
index e75c569bd69773..8ede30645a83a2 100644
--- a/clang/test/SemaTemplate/ms-function-specialization-class-scope.cpp
+++ b/clang/test/SemaTemplate/ms-function-specialization-class-scope.cpp
@@ -91,12 +91,18 @@ namespace UsesThis {
       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}}
+      &x; // expected-error {{invalid use of member 'x' in static member function}}
+      &A::x;
       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();
+      &i; // expected-error 2{{must explicitly qualify name of member function when taking its address}}
+      &j;
+      &A::i;
+      &A::j;
     }
 
     template<typename U = void>
@@ -109,12 +115,18 @@ namespace UsesThis {
       A::x;
       +x;
       +A::x;
+      &x;
+      &A::x;
       f();
       f<void>();
       g();
       g<void>();
       i();
       j();
+      &i; // expected-error 2{{must explicitly qualify name of member function when taking its address}}
+      &j;
+      &A::i;
+      &A::j;
     }
 
     template<typename U>
@@ -127,7 +139,7 @@ namespace UsesThis {
     static void j();
   };
 
-  template struct A<int>; // expected-note 2{{in instantiation of}}
+  template struct A<int>; // expected-note 3{{in instantiation of}}
 
   template <typename T>
   struct Foo {



More information about the cfe-commits mailing list