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 <sdkryst...@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 <sdkryst...@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@+1 {{call to non-static member function without an object argument}} (void)method; - // expected-error@+1 {{call to non-static member function without an object argument}} + // expected-error@+1 {{must explicitly qualify name of member function when taking its address}} (void)&method; // expected-error@+1 {{call to non-static member function without an object argument}} method(); >From b68ea7b5a4a8b3a3b605e850504dd5fd13363211 Mon Sep 17 00:00:00 2001 From: Krystian Stasiowski <sdkryst...@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 <sdkryst...@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 <sdkryst...@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 { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits