Skip to content
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

[Clang][Sema] Don't build CXXDependentScopeMemberExprs for potentially implicit class member access expressions #92318

Merged
merged 5 commits into from
May 20, 2024

Conversation

sdkrystian
Copy link
Member

According to [expr.prim.id.general] p2:

If an id-expression E denotes a non-static non-type member of some class C at a point where the current class is X and

  • E is potentially evaluated or C is X or a base class of X, and
  • E is not the id-expression of a class member access expression, and
  • if E is a qualified-id, E is not the un-parenthesized operand of the unary & operator,

the id-expression is transformed into a class member access expression using (*this) as the object expression.

Consider the following:

struct A
{
    void f0();

    template<typename T>
    void f1();
};

template<typename T>
struct B : T
{
    auto g0() -> decltype(T::f0()); // ok

    auto g1() -> decltype(T::template f1<int>()); // error: call to non-static member function without an object argument
};

template struct B<A>;

Clang incorrectly rejects the call to f1 in the trailing-return-type of g1. Furthermore, the following snippet results in a crash during codegen:

struct A
{
    void f();
};

template<typename T>
struct B : T
{
    template<typename U>
    static void g();
    
    template<>
    void g<int>()
    {
        return T::f(); // crash here
    }
};

template struct B<A>;

This happens because we unconditionally build a CXXDependentScopeMemberExpr (with an implicit object expression) for T::f when parsing the template definition, even though we don't know whether g is an implicit object member function yet.

This patch fixes these issues by instead building DependentScopeDeclRefExprs for such expressions, and only transforming them into implicit class member access expressions during instantiation. Since we implemented the MS "unqualified lookup into dependent bases" extension by building an implicit class member access (and relying on the first component name of the nested-name-specifier to be looked up in the context of the object expression during instantiation), we instead pre-append a fake nested-name-specifier that refers to the injected-class-name of the enclosing class. This patch also refactors Sema::BuildQualifiedDeclarationNameExpr and Sema::BuildQualifiedTemplateIdExpr, streamlining their implementation and removing any redundant checks.

@sdkrystian sdkrystian requested a review from Endilll as a code owner May 15, 2024 21:31
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels May 15, 2024
@llvmbot
Copy link
Member

llvmbot commented May 15, 2024

@llvm/pr-subscribers-clang

Author: Krystian Stasiowski (sdkrystian)

Changes

According to [[expr.prim.id.general] p2](http://eel.is/c++draft/expr.prim.id.general#2):
> If an id-expression E denotes a non-static non-type member of some class C at a point where the current class is X and
> - E is potentially evaluated or C is X or a base class of X, and
> - E is not the id-expression of a class member access expression, and
> - if E is a qualified-id, E is not the un-parenthesized operand of the unary &amp; operator,
>
> the id-expression is transformed into a class member access expression using (*this) as the object expression.

Consider the following:

struct A
{
    void f0();

    template&lt;typename T&gt;
    void f1();
};

template&lt;typename T&gt;
struct B : T
{
    auto g0() -&gt; decltype(T::f0()); // ok

    auto g1() -&gt; decltype(T::template f1&lt;int&gt;()); // error: call to non-static member function without an object argument
};

template struct B&lt;A&gt;;

Clang incorrectly rejects the call to f1 in the trailing-return-type of g1. Furthermore, the following snippet results in a crash during codegen:

struct A
{
    void f();
};

template&lt;typename T&gt;
struct B : T
{
    template&lt;typename U&gt;
    static void g();
    
    template&lt;&gt;
    void g&lt;int&gt;()
    {
        return T::f(); // crash here
    }
};

template struct B&lt;A&gt;;

This happens because we unconditionally build a CXXDependentScopeMemberExpr (with an implicit object expression) for T::f when parsing the template definition, even though we don't know whether g is an implicit object member function yet.

This patch fixes these issues by instead building DependentScopeDeclRefExprs for such expressions, and only transforming them into implicit class member access expressions during instantiation. Since we implemented the MS "unqualified lookup into dependent bases" extension by building an implicit class member access (and relying on the first component name of the nested-name-specifier to be looked up in the context of the object expression during instantiation), we instead pre-append a fake nested-name-specifier that refers to the injected-class-name of the enclosing class. This patch also refactors Sema::BuildQualifiedDeclarationNameExpr and Sema::BuildQualifiedTemplateIdExpr, streamlining their implementation and removing any redundant checks.


Patch is 22.02 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/92318.diff

9 Files Affected:

  • (modified) clang/include/clang/Sema/Sema.h (+5-6)
  • (modified) clang/lib/Sema/SemaCXXScopeSpec.cpp (+8)
  • (modified) clang/lib/Sema/SemaExpr.cpp (+9-22)
  • (modified) clang/lib/Sema/SemaTemplate.cpp (+30-68)
  • (modified) clang/lib/Sema/TreeTransform.h (+3-3)
  • (modified) clang/test/CXX/class/class.mfct/class.mfct.non-static/p3.cpp (+89-2)
  • (modified) clang/test/SemaTemplate/ms-function-specialization-class-scope.cpp (+52)
  • (modified) clang/test/SemaTemplate/ms-lookup-template-base-classes.cpp (+6-6)
  • (modified) clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp (+4-2)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 6a414aa57f32b..779e3134b3067 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -5366,11 +5366,9 @@ class Sema final : public SemaBase {
   bool UseArgumentDependentLookup(const CXXScopeSpec &SS, const LookupResult &R,
                                   bool HasTrailingLParen);
 
-  ExprResult
-  BuildQualifiedDeclarationNameExpr(CXXScopeSpec &SS,
-                                    const DeclarationNameInfo &NameInfo,
-                                    bool IsAddressOfOperand, const Scope *S,
-                                    TypeSourceInfo **RecoveryTSI = nullptr);
+  ExprResult BuildQualifiedDeclarationNameExpr(
+      CXXScopeSpec &SS, const DeclarationNameInfo &NameInfo,
+      bool IsAddressOfOperand, TypeSourceInfo **RecoveryTSI = nullptr);
 
   ExprResult BuildDeclarationNameExpr(const CXXScopeSpec &SS, LookupResult &R,
                                       bool NeedsADL,
@@ -8982,7 +8980,8 @@ class Sema final : public SemaBase {
   ExprResult
   BuildQualifiedTemplateIdExpr(CXXScopeSpec &SS, SourceLocation TemplateKWLoc,
                                const DeclarationNameInfo &NameInfo,
-                               const TemplateArgumentListInfo *TemplateArgs);
+                               const TemplateArgumentListInfo *TemplateArgs,
+                               bool IsAddressOfOperand);
 
   TemplateNameKind ActOnTemplateName(Scope *S, CXXScopeSpec &SS,
                                      SourceLocation TemplateKWLoc,
diff --git a/clang/lib/Sema/SemaCXXScopeSpec.cpp b/clang/lib/Sema/SemaCXXScopeSpec.cpp
index fca5bd131bbc0..c405fbc0aa421 100644
--- a/clang/lib/Sema/SemaCXXScopeSpec.cpp
+++ b/clang/lib/Sema/SemaCXXScopeSpec.cpp
@@ -796,6 +796,14 @@ bool Sema::BuildCXXNestedNameSpecifier(Scope *S, NestedNameSpecInfo &IdInfo,
         Diag(IdInfo.IdentifierLoc,
              diag::ext_undeclared_unqual_id_with_dependent_base)
             << IdInfo.Identifier << ContainingClass;
+        // Fake up a nested-name-specifier that starts with the
+        // injected-class-name of the enclosing class.
+        QualType T = Context.getTypeDeclType(ContainingClass);
+        TypeLocBuilder TLB;
+        TLB.pushTrivial(Context, T, IdInfo.IdentifierLoc);
+        SS.Extend(Context, /*TemplateKWLoc=*/SourceLocation(),
+                  TLB.getTypeLocInContext(Context, T), IdInfo.IdentifierLoc);
+        // Add the identifier to form a dependent name.
         SS.Extend(Context, IdInfo.Identifier, IdInfo.IdentifierLoc,
                   IdInfo.CCLoc);
         return false;
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index ec84798e4ce60..ad6cfb632fd94 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -2946,26 +2946,14 @@ Sema::ActOnIdExpression(Scope *S, CXXScopeSpec &SS,
 /// this path.
 ExprResult Sema::BuildQualifiedDeclarationNameExpr(
     CXXScopeSpec &SS, const DeclarationNameInfo &NameInfo,
-    bool IsAddressOfOperand, const Scope *S, TypeSourceInfo **RecoveryTSI) {
-  if (NameInfo.getName().isDependentName())
-    return BuildDependentDeclRefExpr(SS, /*TemplateKWLoc=*/SourceLocation(),
-                                     NameInfo, /*TemplateArgs=*/nullptr);
-
-  DeclContext *DC = computeDeclContext(SS, false);
-  if (!DC)
-    return BuildDependentDeclRefExpr(SS, /*TemplateKWLoc=*/SourceLocation(),
-                                     NameInfo, /*TemplateArgs=*/nullptr);
-
-  if (RequireCompleteDeclContext(SS, DC))
-    return ExprError();
-
+    bool IsAddressOfOperand, TypeSourceInfo **RecoveryTSI) {
   LookupResult R(*this, NameInfo, LookupOrdinaryName);
-  LookupQualifiedName(R, DC);
+  LookupParsedName(R, /*S=*/nullptr, &SS, /*ObjectType=*/QualType());
 
   if (R.isAmbiguous())
     return ExprError();
 
-  if (R.getResultKind() == LookupResult::NotFoundInCurrentInstantiation)
+  if (R.wasNotFoundInCurrentInstantiation() || SS.isInvalid())
     return BuildDependentDeclRefExpr(SS, /*TemplateKWLoc=*/SourceLocation(),
                                      NameInfo, /*TemplateArgs=*/nullptr);
 
@@ -2974,6 +2962,7 @@ ExprResult Sema::BuildQualifiedDeclarationNameExpr(
     // diagnostic during template instantiation is likely bogus, e.g. if a class
     // is invalid because it's derived from an invalid base class, then missing
     // members were likely supposed to be inherited.
+    DeclContext *DC = computeDeclContext(SS);
     if (const auto *CD = dyn_cast<CXXRecordDecl>(DC))
       if (CD->isInvalidDecl())
         return ExprError();
@@ -3017,16 +3006,14 @@ ExprResult Sema::BuildQualifiedDeclarationNameExpr(
     return ExprEmpty();
   }
 
-  // Defend against this resolving to an implicit member access. We usually
-  // won't get here if this might be a legitimate a class member (we end up in
-  // BuildMemberReferenceExpr instead), but this can be valid if we're forming
-  // a pointer-to-member or in an unevaluated context in C++11.
-  if (!R.empty() && (*R.begin())->isCXXClassMember() && !IsAddressOfOperand)
+  // If necessary, build an implicit class member access.
+  if (isPotentialImplicitMemberAccess(SS, R, IsAddressOfOperand))
     return BuildPossibleImplicitMemberExpr(SS,
                                            /*TemplateKWLoc=*/SourceLocation(),
-                                           R, /*TemplateArgs=*/nullptr, S);
+                                           R, /*TemplateArgs=*/nullptr,
+                                           /*S=*/nullptr);
 
-  return BuildDeclarationNameExpr(SS, R, /* ADL */ false);
+  return BuildDeclarationNameExpr(SS, R, /*ADL=*/false);
 }
 
 /// Cast a base object to a member's actual type.
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index c7aac068e264b..bfdd7398a14a5 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -726,44 +726,18 @@ Sema::ActOnDependentIdExpression(const CXXScopeSpec &SS,
                                  const DeclarationNameInfo &NameInfo,
                                  bool isAddressOfOperand,
                            const TemplateArgumentListInfo *TemplateArgs) {
-  DeclContext *DC = getFunctionLevelDeclContext();
-
-  // 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:
-  //   (...)
-  //   - if that id-expression denotes a non-static data member and it
-  //     appears in an unevaluated operand.
-  //
-  // If this might be the case, form a DependentScopeDeclRefExpr instead of a
-  // CXXDependentScopeMemberExpr. The former can instantiate to either
-  // DeclRefExpr or MemberExpr depending on lookup results, while the latter is
-  // always a MemberExpr.
-  bool MightBeCxx11UnevalField =
-      getLangOpts().CPlusPlus11 && isUnevaluatedContext();
-
-  // Check if the nested name specifier is an enum type.
-  bool IsEnum = false;
-  if (NestedNameSpecifier *NNS = SS.getScopeRep())
-    IsEnum = isa_and_nonnull<EnumType>(NNS->getAsType());
-
-  if (!MightBeCxx11UnevalField && !isAddressOfOperand && !IsEnum &&
-      isa<CXXMethodDecl>(DC) &&
-      cast<CXXMethodDecl>(DC)->isImplicitObjectMemberFunction()) {
-    QualType ThisType =
-        cast<CXXMethodDecl>(DC)->getThisType().getNonReferenceType();
-
-    // Since the 'this' expression is synthesized, we don't need to
-    // perform the double-lookup check.
-    NamedDecl *FirstQualifierInScope = nullptr;
+  if (SS.isEmpty()) {
+    QualType ThisType = getCurrentThisType();
+    if (ThisType.isNull())
+      return ExprError();
 
     return CXXDependentScopeMemberExpr::Create(
-        Context, /*This=*/nullptr, ThisType,
+        Context, /*Base=*/nullptr, ThisType,
         /*IsArrow=*/!Context.getLangOpts().HLSL,
-        /*Op=*/SourceLocation(), SS.getWithLocInContext(Context), TemplateKWLoc,
-        FirstQualifierInScope, NameInfo, TemplateArgs);
+        /*OperatorLoc=*/SourceLocation(),
+        /*QualifierLoc*/ NestedNameSpecifierLoc(), TemplateKWLoc,
+        /*FirstQualifierFoundInScope*/ nullptr, NameInfo, TemplateArgs);
   }
-
   return BuildDependentDeclRefExpr(SS, TemplateKWLoc, NameInfo, TemplateArgs);
 }
 
@@ -772,13 +746,15 @@ Sema::BuildDependentDeclRefExpr(const CXXScopeSpec &SS,
                                 SourceLocation TemplateKWLoc,
                                 const DeclarationNameInfo &NameInfo,
                                 const TemplateArgumentListInfo *TemplateArgs) {
-  // DependentScopeDeclRefExpr::Create requires a valid QualifierLoc
-  NestedNameSpecifierLoc QualifierLoc = SS.getWithLocInContext(Context);
-  if (!QualifierLoc)
-    return ExprError();
+  // DependentScopeDeclRefExpr::Create requires a valid NestedNameSpecifierLoc
+  if (!SS.isValid())
+    return CreateRecoveryExpr(
+        SS.getBeginLoc(),
+        TemplateArgs ? TemplateArgs->getRAngleLoc() : NameInfo.getEndLoc(), {});
 
   return DependentScopeDeclRefExpr::Create(
-      Context, QualifierLoc, TemplateKWLoc, NameInfo, TemplateArgs);
+      Context, SS.getWithLocInContext(Context), TemplateKWLoc, NameInfo,
+      TemplateArgs);
 }
 
 
@@ -5675,50 +5651,36 @@ ExprResult Sema::BuildTemplateIdExpr(const CXXScopeSpec &SS,
 }
 
 // We actually only call this from template instantiation.
-ExprResult
-Sema::BuildQualifiedTemplateIdExpr(CXXScopeSpec &SS,
-                                   SourceLocation TemplateKWLoc,
-                                   const DeclarationNameInfo &NameInfo,
-                             const TemplateArgumentListInfo *TemplateArgs) {
-
+ExprResult Sema::BuildQualifiedTemplateIdExpr(
+    CXXScopeSpec &SS, SourceLocation TemplateKWLoc,
+    const DeclarationNameInfo &NameInfo,
+    const TemplateArgumentListInfo *TemplateArgs, bool IsAddressOfOperand) {
   assert(TemplateArgs || TemplateKWLoc.isValid());
-  DeclContext *DC;
-  if (!(DC = computeDeclContext(SS, false)) ||
-      DC->isDependentContext() ||
-      RequireCompleteDeclContext(SS, DC))
-    return BuildDependentDeclRefExpr(SS, TemplateKWLoc, NameInfo, TemplateArgs);
 
   LookupResult R(*this, NameInfo, LookupOrdinaryName);
-  if (LookupTemplateName(R, (Scope *)nullptr, SS, QualType(),
-                         /*Entering*/ false, TemplateKWLoc))
+  if (LookupTemplateName(R, /*S=*/nullptr, SS, /*ObjectType=*/QualType(),
+                         /*EnteringContext=*/false, TemplateKWLoc))
     return ExprError();
 
   if (R.isAmbiguous())
     return ExprError();
 
+  if (R.wasNotFoundInCurrentInstantiation() || SS.isInvalid())
+    return BuildDependentDeclRefExpr(SS, TemplateKWLoc, NameInfo, TemplateArgs);
+
   if (R.empty()) {
+    DeclContext *DC = computeDeclContext(SS);
     Diag(NameInfo.getLoc(), diag::err_no_member)
       << NameInfo.getName() << DC << SS.getRange();
     return ExprError();
   }
 
-  auto DiagnoseTypeTemplateDecl = [&](TemplateDecl *Temp,
-                                      bool isTypeAliasTemplateDecl) {
-    Diag(NameInfo.getLoc(), diag::err_template_kw_refers_to_type_template)
-        << SS.getScopeRep() << NameInfo.getName().getAsString() << SS.getRange()
-        << isTypeAliasTemplateDecl;
-    Diag(Temp->getLocation(), diag::note_referenced_type_template)
-        << isTypeAliasTemplateDecl;
-    return CreateRecoveryExpr(NameInfo.getBeginLoc(), NameInfo.getEndLoc(), {});
-  };
-
-  if (ClassTemplateDecl *Temp = R.getAsSingle<ClassTemplateDecl>())
-    return DiagnoseTypeTemplateDecl(Temp, false);
-
-  if (TypeAliasTemplateDecl *Temp = R.getAsSingle<TypeAliasTemplateDecl>())
-    return DiagnoseTypeTemplateDecl(Temp, true);
+  // If necessary, build an implicit class member access.
+  if (isPotentialImplicitMemberAccess(SS, R, IsAddressOfOperand))
+    return BuildPossibleImplicitMemberExpr(SS, TemplateKWLoc, R, TemplateArgs,
+                                           /*S=*/nullptr);
 
-  return BuildTemplateIdExpr(SS, TemplateKWLoc, R, /*ADL*/ false, TemplateArgs);
+  return BuildTemplateIdExpr(SS, TemplateKWLoc, R, /*ADL=*/false, TemplateArgs);
 }
 
 /// Form a template name from a name that is syntactically required to name a
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index c039b95293af2..0aeda79aa6fdd 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -3478,11 +3478,11 @@ class TreeTransform {
     SS.Adopt(QualifierLoc);
 
     if (TemplateArgs || TemplateKWLoc.isValid())
-      return getSema().BuildQualifiedTemplateIdExpr(SS, TemplateKWLoc, NameInfo,
-                                                    TemplateArgs);
+      return getSema().BuildQualifiedTemplateIdExpr(
+          SS, TemplateKWLoc, NameInfo, TemplateArgs, IsAddressOfOperand);
 
     return getSema().BuildQualifiedDeclarationNameExpr(
-        SS, NameInfo, IsAddressOfOperand, /*S*/nullptr, RecoveryTSI);
+        SS, NameInfo, IsAddressOfOperand, RecoveryTSI);
   }
 
   /// Build a new template-id expression.
diff --git a/clang/test/CXX/class/class.mfct/class.mfct.non-static/p3.cpp b/clang/test/CXX/class/class.mfct/class.mfct.non-static/p3.cpp
index 9116e7146f812..01fa923dd1715 100644
--- a/clang/test/CXX/class/class.mfct/class.mfct.non-static/p3.cpp
+++ b/clang/test/CXX/class/class.mfct/class.mfct.non-static/p3.cpp
@@ -70,7 +70,7 @@ namespace test2 {
     }
 
     void test1() {
-      B<T>::foo();
+      B<T>::foo(); // expected-error {{call to non-static member function without an object argument}}
     }
 
     static void test2() {
@@ -91,8 +91,95 @@ namespace test2 {
   int test() {
     A<int> a;
     a.test0(); // no instantiation note here, decl is ill-formed
-    a.test1();
+    a.test1(); // expected-note {{in instantiation}}
     a.test2(); // expected-note {{in instantiation}}
     a.test3(); // expected-note {{in instantiation}}
   }
 }
+
+namespace test3 {
+  struct A {
+    void f0();
+
+    template<typename T>
+    void f1();
+
+    static void f2();
+
+    template<typename T>
+    static void f3();
+
+    int x0;
+
+    static constexpr int x1 = 0;
+
+    template<typename T>
+    static constexpr int x2 = 0;
+  };
+
+  template<typename T>
+  struct B : T {
+    auto g0() -> decltype(T::f0());
+
+    auto g1() -> decltype(T::template f1<int>());
+
+    auto g2() -> decltype(T::f2());
+
+    auto g3() -> decltype(T::template f3<int>());
+
+    auto g4() -> decltype(T::x0);
+
+    auto g5() -> decltype(T::x1);
+
+    auto g6() -> decltype(T::template x2<int>);
+
+    decltype(T::f0()) g7(); // expected-error {{call to non-static member function without an object argument}}
+
+    decltype(T::template f1<int>()) g8(); // expected-error {{call to non-static member function without an object argument}}
+
+    decltype(T::f2()) g9();
+
+    decltype(T::template f3<int>()) g10();
+
+    decltype(T::x0) g11();
+
+    decltype(T::x1) g12();
+
+    decltype(T::template x2<int>) g13();
+  };
+
+  template struct B<A>; // expected-note {{in instantiation of}}
+
+  template<typename T>
+  struct C : T {
+    static auto g0() -> decltype(T::f0()); // expected-error {{'this' cannot be implicitly used in a static member function declaration}}
+
+    static auto g1() -> decltype(T::template f1<int>()); // expected-error {{'this' cannot be implicitly used in a static member function declaration}}
+
+    static auto g2() -> decltype(T::f2());
+
+    static auto g3() -> decltype(T::template f3<int>());
+
+    static auto g4() -> decltype(T::x0); // expected-error {{'this' cannot be implicitly used in a static member function declaration}}
+
+    static auto g5() -> decltype(T::x1);
+
+    static auto g6() -> decltype(T::template x2<int>);
+
+    static decltype(T::f0()) g7(); // expected-error {{call to non-static member function without an object argument}}
+
+    static decltype(T::template f1<int>()) g8(); // expected-error {{call to non-static member function without an object argument}}
+
+    static decltype(T::f2()) g9();
+
+    static decltype(T::template f3<int>()) g10();
+
+    static decltype(T::x0) g11();
+
+    static decltype(T::x1) g12();
+
+    static decltype(T::template x2<int>) g13();
+  };
+
+  template struct C<A>; // expected-note {{in instantiation of}}
+}
diff --git a/clang/test/SemaTemplate/ms-function-specialization-class-scope.cpp b/clang/test/SemaTemplate/ms-function-specialization-class-scope.cpp
index c49d2cb2422fa..e1f3ab37ad947 100644
--- a/clang/test/SemaTemplate/ms-function-specialization-class-scope.cpp
+++ b/clang/test/SemaTemplate/ms-function-specialization-class-scope.cpp
@@ -464,6 +464,32 @@ namespace UsesThis {
       g1(x1);
       g1(y0);
       g1(y1);
+
+      T::f0(0);
+      T::f0(z);
+      T::f0(x0);
+      T::f0(x1);
+      T::f0(y0);
+      T::f0(y1);
+      T::g0(0);
+      T::g0(z);
+      T::g0(x0);
+      T::g0(x1);
+      T::g0(y0);
+      T::g0(y1);
+
+      E::f1(0);
+      E::f1(z);
+      E::f1(x0);
+      E::f1(x1);
+      E::f1(y0);
+      E::f1(y1);
+      E::g1(0);
+      E::g1(z);
+      E::g1(x0);
+      E::g1(x1);
+      E::g1(y0);
+      E::g1(y1);
     }
 
     template<>
@@ -519,6 +545,32 @@ namespace UsesThis {
       g1(x1); // expected-error {{invalid use of member 'x1' in static member function}}
       g1(y0);
       g1(y1);
+
+      T::f0(0); // expected-error {{call to non-static member function without an object argument}}
+      T::f0(z); // expected-error {{call to non-static member function without an object argument}}
+      T::f0(x0); // expected-error {{call to non-static member function without an object argument}}
+      T::f0(x1); // expected-error {{call to non-static member function without an object argument}}
+      T::f0(y0); // expected-error {{call to non-static member function without an object argument}}
+      T::f0(y1); // expected-error {{call to non-static member function without an object argument}}
+      T::g0(0);
+      T::g0(z);
+      T::g0(x0); // expected-error {{invalid use of member 'x0' in static member function}}
+      T::g0(x1); // expected-error {{invalid use of member 'x1' in static member function}}
+      T::g0(y0);
+      T::g0(y1);
+
+      E::f1(0); // expected-error {{call to non-static member function without an object argument}}
+      E::f1(z); // expected-error {{call to non-static member function without an object argument}}
+      E::f1(x0); // expected-error {{call to non-static member function without an object argument}}
+      E::f1(x1); // expected-error {{call to non-static member function without an object argument}}
+      E::f1(y0); // expected-error {{call to non-static member function without an object argument}}
+      E::f1(y1); // expected-error {{call to non-static member function without an object argument}}
+      E::g1(0);
+      E::g1(z);
+      E::g1(x0); // expected-error {{invalid use of member 'x0' in static member function}}
+      E::g1(x1); // expected-error {{invalid use of member 'x1' in static member function}}
+      E::g1(y0);
+      E::g1(y1);
     }
   };
 
diff --git a/clang/test/SemaTemplate/ms-lookup-template-base-classes.cpp b/clang/test/SemaTemplate/ms-lookup-template-base-classes.cpp
index 534a5dc9ddc10..547e5945ac6bc 100644
--- a/clang/test/SemaTemplate/ms-lookup-template-base-classes.cpp
+++ b/clang/test/SemaTemplate/ms-lookup-template-base-classes.cpp
@@ -102,7 +102,7 @@ class B : public A<T> {
 };
 template class B<int>; // expected-note {{requested here}}
 
-} 
+}
 
 
 
@@ -111,8 +111,8 @@ namespace lookup_dependent_base_class_default_argument {
 template<class T>
 class A {
 public:
-  static int f1(); // expected-note {{must qualify identifier to find this declaration in dependent base class}} 
-  int f2(); // expected-note {{must qualify identifier to find this declaration in dependent base class}} 
+  static int f1(); // expected-note {{must qualify identifier to find this declaration in dependent base class}}
+  int f2(); // expected-note {{must qualify identifier to find this declaration in dependent base class}}
 };
 
 template<class T>
@@ -137,7 +137,7 @@ namespace lookup_dependent_base_class_friend {
 template <class T>
 class B {
 public:
-  static void g();  // expected-note {{must qualify identifier to find this declaration in dependent base class}} 
+  static void g();  // expected-note {{must qualify identifier to find this declaration in dependent base class}}
 };
 
 template <class T>
@@ -228...
[truncated]

Copy link
Collaborator

@erichkeane erichkeane left a comment

Choose a reason for hiding this comment

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

Seems sensible to me, just need a release note

@sdkrystian sdkrystian force-pushed the dependent-implicit-mem-expr branch from f799624 to 95239f8 Compare May 15, 2024 22:09
Copy link
Contributor

@Endilll Endilll left a comment

Choose a reason for hiding this comment

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

Sema.h changes look good to me.

@sdkrystian sdkrystian force-pushed the dependent-implicit-mem-expr branch 3 times, most recently from 4ee87ce to 862c80d Compare May 17, 2024 21:05
Copy link
Contributor

@mizvekov mizvekov left a comment

Choose a reason for hiding this comment

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

LGTM

@sdkrystian sdkrystian force-pushed the dependent-implicit-mem-expr branch from 862c80d to 99ef993 Compare May 20, 2024 15:16
@sdkrystian sdkrystian force-pushed the dependent-implicit-mem-expr branch from 99ef993 to 4dc8d17 Compare May 20, 2024 17:38
@sdkrystian sdkrystian merged commit fd87d76 into llvm:main May 20, 2024
4 of 5 checks passed
sdkrystian added a commit that referenced this pull request Jul 3, 2024
…nary operator& (#97596)

Currently, `TreeTransform::TransformCXXOperatorCallExpr` calls
`TreeTransform::TransformAddressOfOperand` to transform the first
operand of a `CXXOperatorCallExpr` when its `OverloadOperatorKind` is
`OO_Amp` -- regardless of arity. This results in the first operand of
binary `operator&` being incorrectly transformed as if it was the
operand of the address of operator in cases such as the following:
```
struct A {
  int x;
};

void operator&(A, A);

template<typename T>
struct B {
  int f() {
    return T::x & 1; // invalid reference to 'A::x' is not diagnosed because 'T::x' is incorrectly transformed as if it was the operand of unary operator&
  }
};

template struct B<A>;
```
Prior to #92318 we would build a `CXXDependentScopeMemberExpr` for
`T::x` (as with most dependent qualified names that were not member
qualified names). Since `TreeTransform::TransformAddressOfOperand` only
differs from `TransformExpr` for `DependentScopeDeclRefExpr` and
`UnresolvedLookupExpr` operands, `T::x` was transformed "correctly". Now
that we build a `DependentScopeDeclRefExpr` for `T::x`, it is
incorrectly transformed as if it was the operand of the address of
operator and we fail to diagnose the invalid reference to a non-static
data member. This patch fixes the issue by only calling
`TreeTransform::TransformAddressOfOperand` for `CXXOperatorCallExpr`s
with a single operand. This fixes #97483.
kbluck pushed a commit to kbluck/llvm-project that referenced this pull request Jul 6, 2024
…nary operator& (llvm#97596)

Currently, `TreeTransform::TransformCXXOperatorCallExpr` calls
`TreeTransform::TransformAddressOfOperand` to transform the first
operand of a `CXXOperatorCallExpr` when its `OverloadOperatorKind` is
`OO_Amp` -- regardless of arity. This results in the first operand of
binary `operator&` being incorrectly transformed as if it was the
operand of the address of operator in cases such as the following:
```
struct A {
  int x;
};

void operator&(A, A);

template<typename T>
struct B {
  int f() {
    return T::x & 1; // invalid reference to 'A::x' is not diagnosed because 'T::x' is incorrectly transformed as if it was the operand of unary operator&
  }
};

template struct B<A>;
```
Prior to llvm#92318 we would build a `CXXDependentScopeMemberExpr` for
`T::x` (as with most dependent qualified names that were not member
qualified names). Since `TreeTransform::TransformAddressOfOperand` only
differs from `TransformExpr` for `DependentScopeDeclRefExpr` and
`UnresolvedLookupExpr` operands, `T::x` was transformed "correctly". Now
that we build a `DependentScopeDeclRefExpr` for `T::x`, it is
incorrectly transformed as if it was the operand of the address of
operator and we fail to diagnose the invalid reference to a non-static
data member. This patch fixes the issue by only calling
`TreeTransform::TransformAddressOfOperand` for `CXXOperatorCallExpr`s
with a single operand. This fixes llvm#97483.
searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request Jul 11, 2024
…y implicit class member access expressions (llvm#92318)

According to [expr.prim.id.general] p2:
> If an _id-expression_ `E` denotes a non-static non-type member of some
class `C` at a point where the current class is `X` and
> - `E` is potentially evaluated or `C` is `X` or a base class of `X`,
and
> - `E` is not the _id-expression_ of a class member access expression,
and
> - if `E` is a _qualified-id_, `E` is not the un-parenthesized operand
of the unary `&` operator,
>
> the _id-expression_ is transformed into a class member access
expression using `(*this)` as the object expression.

Consider the following:
```
struct A
{
    void f0();

    template<typename T>
    void f1();
};

template<typename T>
struct B : T
{
    auto g0() -> decltype(T::f0()); // ok

    auto g1() -> decltype(T::template f1<int>()); // error: call to non-static member function without an object argument
};

template struct B<A>;
```

Clang incorrectly rejects the call to `f1` in the _trailing-return-type_
of `g1`. Furthermore, the following snippet results in a crash during
codegen:
```
struct A
{
    void f();
};

template<typename T>
struct B : T
{
    template<typename U>
    static void g();

    template<>
    void g<int>()
    {
        return T::f(); // crash here
    }
};

template struct B<A>;
```
This happens because we unconditionally build a
`CXXDependentScopeMemberExpr` (with an implicit object expression) for
`T::f` when parsing the template definition, even though we don't know
whether `g` is an implicit object member function yet.

This patch fixes these issues by instead building
`DependentScopeDeclRefExpr`s for such expressions, and only transforming
them into implicit class member access expressions during instantiation.
Since we implemented the MS "unqualified lookup into dependent bases"
extension by building an implicit class member access (and relying on
the first component name of the _nested-name-specifier_ to be looked up
in the context of the object expression during instantiation), we
instead pre-append a fake _nested-name-specifier_ that refers to the
injected-class-name of the enclosing class. This patch also refactors
`Sema::BuildQualifiedDeclarationNameExpr` and
`Sema::BuildQualifiedTemplateIdExpr`, streamlining their implementation
and removing any redundant checks.

Change-Id: Ifaaa4aa9c49381e13029ad9266ee3785e2d791d1
searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request Oct 22, 2024
cherry-picked:
8009bbe sdkrystian@gmail.com              Tue Apr 30 14:25:09 2024 -0400 Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent base classes (llvm#84050)" (llvm#90152)
3191e0b sdkrystian@gmail.com              Fri May  3 17:07:52 2024 -0400 [Clang][Sema] Fix template name lookup for operator= (llvm#90999)
62b5b61 sdkrystian@gmail.com              Wed May  8 20:49:59 2024 -0400 [Clang][Sema] Fix lookup of dependent operator= outside of complete-class contexts (llvm#91498)
75ebcbf sdkrystian@gmail.com              Thu May  9 16:34:40 2024 -0400 [Clang][Sema] Revert changes to operator= lookup in templated classes from llvm#91498, llvm#90999, and llvm#90152 (llvm#91620)
596a9c1 sdkrystian@gmail.com              Mon May 13 12:24:46 2024 -0400 [Clang][Sema] Fix bug where operator-> typo corrects in the current instantiation (llvm#91972)
fd87d76 sdkrystian@gmail.com              Mon May 20 13:55:01 2024 -0400 [Clang][Sema] Don't build CXXDependentScopeMemberExprs for potentially implicit class member access expressions (llvm#92318)
e75b58c sdkrystian@gmail.com              Mon May 20 14:50:58 2024 -0400 [Clang][Sema] Do not add implicit 'const' when matching constexpr function template explicit specializations after C++14 (llvm#92449)
bae2c54 serebrennikov.vladislav@gmail.com Mon Jul  1 20:55:57 2024 +0300 [clang][NFC] Move documentation of `Sema` functions into `Sema.h`
e6d305e dzenis@richard.lv                 Mon Sep  4 16:54:42 2023 +0200 Add support of Windows Trace Logging macros

Change-Id: I521b2ebabd7eb9a0df78c577992bfd8508ba44fd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants