llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Krystian Stasiowski (sdkrystian) <details> <summary>Changes</summary> Consider the following: ```cpp template<typename T> struct A { auto f() { return this->x; } }; ``` Although `A` has no dependent base classes and the lookup context for `x` is the current instantiation, we currently do not diagnose the absence of a member `x` until `A<T>::f` is instantiated. This patch moves the point of diagnosis for such expressions to occur at the point of definition. Note that this PR is a bit of a work in progress. Although it passes all tests, some aspects (e.g. calling `LookupResult::setNotFoundInCurrentInstantiation` in `LookupMemberExprInRecord`, using `Type::getAsRecordDecl` to determine whether a type names the current instantiation, and using `computeDeclContext` to determine whether a nested-name-specifier names the current instantiation) are a bit "sketchy" and may need some work. I also need to add more tests, e.g. for using declarations, or class member access expressions naming members of nested classes which are explicitly specialized for a given implicitly instantiated specialization of the enclosing class template. --- Patch is 23.23 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/84050.diff 11 Files Affected: - (modified) clang/lib/AST/Expr.cpp (+1-1) - (modified) clang/lib/Sema/SemaExpr.cpp (+3-1) - (modified) clang/lib/Sema/SemaExprMember.cpp (+80-52) - (modified) clang/lib/Sema/SemaOverload.cpp (+3) - (modified) clang/lib/Sema/TreeTransform.h (+20) - (modified) clang/test/AST/HLSL/this-reference-template.hlsl (+1-1) - (added) clang/test/CXX/temp/temp.res/temp.dep/temp.dep.type/p4.cpp (+110) - (modified) clang/test/CodeGenCXX/mangle.cpp (-8) - (modified) clang/test/Index/annotate-nested-name-specifier.cpp (+2-2) - (modified) clang/test/SemaTemplate/instantiate-function-1.cpp (+7-7) - (modified) clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp (+2-2) ``````````diff diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp index b4de2155adcebd..643c938d63c654 100644 --- a/clang/lib/AST/Expr.cpp +++ b/clang/lib/AST/Expr.cpp @@ -103,7 +103,7 @@ const Expr *Expr::skipRValueSubobjectAdjustments( } } else if (const auto *ME = dyn_cast<MemberExpr>(E)) { if (!ME->isArrow()) { - assert(ME->getBase()->getType()->isRecordType()); + assert(ME->getBase()->getType()->getAsRecordDecl()); if (const auto *Field = dyn_cast<FieldDecl>(ME->getMemberDecl())) { if (!Field->isBitField() && !Field->getType()->isReferenceType()) { E = ME->getBase(); diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 47bb263f56aade..e071c3e580b9d8 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -666,7 +666,9 @@ ExprResult Sema::DefaultLvalueConversion(Expr *E) { // expressions of certain types in C++. if (getLangOpts().CPlusPlus && (E->getType() == Context.OverloadTy || - T->isDependentType() || + // FIXME: This is a hack! We want the lvalue-to-rvalue conversion applied + // to pointer types even if the pointee type is dependent. + (T->isDependentType() && !T->isPointerType()) || T->isRecordType())) return E; diff --git a/clang/lib/Sema/SemaExprMember.cpp b/clang/lib/Sema/SemaExprMember.cpp index 32998ae60eafe2..eb87e0edda0128 100644 --- a/clang/lib/Sema/SemaExprMember.cpp +++ b/clang/lib/Sema/SemaExprMember.cpp @@ -624,8 +624,8 @@ namespace { // classes, one of its base classes. class RecordMemberExprValidatorCCC final : public CorrectionCandidateCallback { public: - explicit RecordMemberExprValidatorCCC(const RecordType *RTy) - : Record(RTy->getDecl()) { + explicit RecordMemberExprValidatorCCC(QualType RTy) + : Record(RTy->getAsRecordDecl()) { // Don't add bare keywords to the consumer since they will always fail // validation by virtue of not being associated with any decls. WantTypeSpecifiers = false; @@ -671,33 +671,54 @@ class RecordMemberExprValidatorCCC final : public CorrectionCandidateCallback { static bool LookupMemberExprInRecord(Sema &SemaRef, LookupResult &R, Expr *BaseExpr, - const RecordType *RTy, + QualType RTy, SourceLocation OpLoc, bool IsArrow, CXXScopeSpec &SS, bool HasTemplateArgs, SourceLocation TemplateKWLoc, TypoExpr *&TE) { + DeclContext *DC = SemaRef.computeDeclContext(RTy); + // If the object expression is dependent and isn't the current instantiation, + // lookup will not find anything and we must defer until instantiation. + if (!DC) { + R.setNotFoundInCurrentInstantiation(); + return false; + } + + // FIXME: Should this use Name.isDependentName()? + if (DeclarationName Name = R.getLookupName(); + Name.getNameKind() == DeclarationName::CXXConversionFunctionName && + Name.getCXXNameType()->isDependentType()) { + R.setNotFoundInCurrentInstantiation(); + return false; + } + SourceRange BaseRange = BaseExpr ? BaseExpr->getSourceRange() : SourceRange(); - RecordDecl *RDecl = RTy->getDecl(); - if (!SemaRef.isThisOutsideMemberFunctionBody(QualType(RTy, 0)) && - SemaRef.RequireCompleteType(OpLoc, QualType(RTy, 0), + if (!RTy->isDependentType() && + !SemaRef.isThisOutsideMemberFunctionBody(RTy) && + SemaRef.RequireCompleteType(OpLoc, RTy, diag::err_typecheck_incomplete_tag, BaseRange)) return true; if (HasTemplateArgs || TemplateKWLoc.isValid()) { // LookupTemplateName doesn't expect these both to exist simultaneously. - QualType ObjectType = SS.isSet() ? QualType() : QualType(RTy, 0); + QualType ObjectType = SS.isSet() ? QualType() : RTy; bool MOUS; return SemaRef.LookupTemplateName(R, nullptr, SS, ObjectType, false, MOUS, TemplateKWLoc); } - DeclContext *DC = RDecl; if (SS.isSet()) { // If the member name was a qualified-id, look into the // nested-name-specifier. DC = SemaRef.computeDeclContext(SS, false); + // We tried to look into a dependent context that is not the current + // instantiation. Defer lookup until instantiation. + if (!DC) { + R.setNotFoundInCurrentInstantiation(); + return false; + } if (SemaRef.RequireCompleteDeclContext(SS, DC)) { SemaRef.Diag(SS.getRange().getEnd(), diag::err_typecheck_incomplete_tag) @@ -717,7 +738,7 @@ static bool LookupMemberExprInRecord(Sema &SemaRef, LookupResult &R, // The record definition is complete, now look up the member. SemaRef.LookupQualifiedName(R, DC, SS); - if (!R.empty()) + if (!R.empty() || R.wasNotFoundInCurrentInstantiation()) return false; DeclarationName Typo = R.getLookupName(); @@ -781,14 +802,6 @@ Sema::BuildMemberReferenceExpr(Expr *Base, QualType BaseType, const TemplateArgumentListInfo *TemplateArgs, const Scope *S, ActOnMemberAccessExtraArgs *ExtraArgs) { - if (BaseType->isDependentType() || - (SS.isSet() && isDependentScopeSpecifier(SS)) || - NameInfo.getName().isDependentName()) - return ActOnDependentMemberExpr(Base, BaseType, - IsArrow, OpLoc, - SS, TemplateKWLoc, FirstQualifierInScope, - NameInfo, TemplateArgs); - LookupResult R(*this, NameInfo, LookupMemberName); // Implicit member accesses. @@ -797,7 +810,7 @@ Sema::BuildMemberReferenceExpr(Expr *Base, QualType BaseType, QualType RecordTy = BaseType; if (IsArrow) RecordTy = RecordTy->castAs<PointerType>()->getPointeeType(); if (LookupMemberExprInRecord( - *this, R, nullptr, RecordTy->castAs<RecordType>(), OpLoc, IsArrow, + *this, R, nullptr, RecordTy, OpLoc, IsArrow, SS, TemplateArgs != nullptr, TemplateKWLoc, TE)) return ExprError(); if (TE) @@ -990,6 +1003,15 @@ Sema::BuildMemberReferenceExpr(Expr *BaseExpr, QualType BaseExprType, const Scope *S, bool SuppressQualifierCheck, ActOnMemberAccessExtraArgs *ExtraArgs) { + + if (R.wasNotFoundInCurrentInstantiation() || + (SS.isValid() && !computeDeclContext(SS, false))) { + return ActOnDependentMemberExpr(BaseExpr, BaseExprType, + IsArrow, OpLoc, + SS, TemplateKWLoc, FirstQualifierInScope, + R.getLookupNameInfo(), TemplateArgs); + } + QualType BaseType = BaseExprType; if (IsArrow) { assert(BaseType->isPointerType()); @@ -1028,11 +1050,11 @@ Sema::BuildMemberReferenceExpr(Expr *BaseExpr, QualType BaseExprType, // Rederive where we looked up. DeclContext *DC = (SS.isSet() ? computeDeclContext(SS, false) - : BaseType->castAs<RecordType>()->getDecl()); + : BaseType->getAsRecordDecl()); if (ExtraArgs) { ExprResult RetryExpr; - if (!IsArrow && BaseExpr) { + if (!IsArrow && BaseExpr && !BaseExpr->isTypeDependent()) { SFINAETrap Trap(*this, true); ParsedType ObjectType; bool MayBePseudoDestructor = false; @@ -1055,9 +1077,16 @@ Sema::BuildMemberReferenceExpr(Expr *BaseExpr, QualType BaseExprType, } } - Diag(R.getNameLoc(), diag::err_no_member) - << MemberName << DC - << (BaseExpr ? BaseExpr->getSourceRange() : SourceRange()); + if(DC) { + Diag(R.getNameLoc(), diag::err_no_member) + << MemberName << DC + << (BaseExpr ? BaseExpr->getSourceRange() : SourceRange()); + } else { + // FIXME: Is this needed? + Diag(R.getNameLoc(), diag::err_no_member) + << MemberName << BaseExprType + << (BaseExpr ? BaseExpr->getSourceRange() : SourceRange()); + } return ExprError(); } @@ -1287,7 +1316,10 @@ static ExprResult LookupMemberExpr(Sema &S, LookupResult &R, return ExprError(); QualType BaseType = BaseExpr.get()->getType(); + + #if 0 assert(!BaseType->isDependentType()); + #endif DeclarationName MemberName = R.getLookupName(); SourceLocation MemberLoc = R.getNameLoc(); @@ -1299,29 +1331,32 @@ static ExprResult LookupMemberExpr(Sema &S, LookupResult &R, if (IsArrow) { if (const PointerType *Ptr = BaseType->getAs<PointerType>()) BaseType = Ptr->getPointeeType(); - else if (const ObjCObjectPointerType *Ptr + else if (!BaseType->isDependentType()) { + if (const ObjCObjectPointerType *Ptr = BaseType->getAs<ObjCObjectPointerType>()) BaseType = Ptr->getPointeeType(); - else if (BaseType->isRecordType()) { - // Recover from arrow accesses to records, e.g.: - // struct MyRecord foo; - // foo->bar - // This is actually well-formed in C++ if MyRecord has an - // overloaded operator->, but that should have been dealt with - // by now--or a diagnostic message already issued if a problem - // was encountered while looking for the overloaded operator->. - if (!S.getLangOpts().CPlusPlus) { - S.Diag(OpLoc, diag::err_typecheck_member_reference_suggestion) - << BaseType << int(IsArrow) << BaseExpr.get()->getSourceRange() - << FixItHint::CreateReplacement(OpLoc, "."); + else if (BaseType->isRecordType()) { + // Recover from arrow accesses to records, e.g.: + // struct MyRecord foo; + // foo->bar + // This is actually well-formed in C++ if MyRecord has an + // overloaded operator->, but that should have been dealt with + // by now--or a diagnostic message already issued if a problem + // was encountered while looking for the overloaded operator->. + if (!S.getLangOpts().CPlusPlus) { + S.Diag(OpLoc, diag::err_typecheck_member_reference_suggestion) + << BaseType << int(IsArrow) << BaseExpr.get()->getSourceRange() + << FixItHint::CreateReplacement(OpLoc, "."); + } + IsArrow = false; + } else if (BaseType->isFunctionType()) { + goto fail; + } else { + S.Diag(MemberLoc, diag::err_typecheck_member_reference_arrow) + << BaseType << BaseExpr.get()->getSourceRange(); + return ExprError(); } - IsArrow = false; - } else if (BaseType->isFunctionType()) { - goto fail; - } else { - S.Diag(MemberLoc, diag::err_typecheck_member_reference_arrow) - << BaseType << BaseExpr.get()->getSourceRange(); - return ExprError(); + } } @@ -1341,9 +1376,9 @@ static ExprResult LookupMemberExpr(Sema &S, LookupResult &R, } // Handle field access to simple records. - if (const RecordType *RTy = BaseType->getAs<RecordType>()) { + if (BaseType->getAsRecordDecl() || BaseType->isDependentType()) { TypoExpr *TE = nullptr; - if (LookupMemberExprInRecord(S, R, BaseExpr.get(), RTy, OpLoc, IsArrow, SS, + if (LookupMemberExprInRecord(S, R, BaseExpr.get(), BaseType, OpLoc, IsArrow, SS, HasTemplateArgs, TemplateKWLoc, TE)) return ExprError(); @@ -1781,13 +1816,6 @@ ExprResult Sema::ActOnMemberAccessExpr(Scope *S, Expr *Base, if (Result.isInvalid()) return ExprError(); Base = Result.get(); - if (Base->getType()->isDependentType() || Name.isDependentName() || - isDependentScopeSpecifier(SS)) { - return ActOnDependentMemberExpr(Base, Base->getType(), IsArrow, OpLoc, SS, - TemplateKWLoc, FirstQualifierInScope, - NameInfo, TemplateArgs); - } - ActOnMemberAccessExtraArgs ExtraArgs = {S, Id, ObjCImpDecl}; ExprResult Res = BuildMemberReferenceExpr( Base, Base->getType(), OpLoc, IsArrow, SS, TemplateKWLoc, diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index 7d38043890ca20..7218553bcd77f8 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -5744,7 +5744,10 @@ static ImplicitConversionSequence TryObjectArgumentInitialization( return ICS; } + // FIXME: Should this check getAsRecordDecl instead? + #if 0 assert(FromType->isRecordType()); + #endif QualType ClassType = S.Context.getTypeDeclType(ActingContext); // C++98 [class.dtor]p2: diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h index 7389a48fe56fcc..27d3588aaa9082 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -12930,6 +12930,26 @@ bool TreeTransform<Derived>::TransformOverloadExprDecls(OverloadExpr *Old, // Resolve a kind, but don't do any further analysis. If it's // ambiguous, the callee needs to deal with it. R.resolveKind(); + + if (Old->hasTemplateKeyword() && !R.empty()) { + NamedDecl *FoundDecl = R.getRepresentativeDecl()->getUnderlyingDecl(); + getSema().FilterAcceptableTemplateNames(R, + /*AllowFunctionTemplates=*/true, + /*AllowDependent=*/true); + if (R.empty()) { + // If a 'template' keyword was used, a lookup that finds only non-template + // names is an error. + getSema().Diag(R.getNameLoc(), diag::err_template_kw_refers_to_non_template) + << R.getLookupName() + << Old->getQualifierLoc().getSourceRange() + << Old->hasTemplateKeyword() + << Old->getTemplateKeywordLoc(); + getSema().Diag(FoundDecl->getLocation(), diag::note_template_kw_refers_to_non_template) + << R.getLookupName(); + return true; + } + } + return false; } diff --git a/clang/test/AST/HLSL/this-reference-template.hlsl b/clang/test/AST/HLSL/this-reference-template.hlsl index 60e057986ebf80..d427e73044b788 100644 --- a/clang/test/AST/HLSL/this-reference-template.hlsl +++ b/clang/test/AST/HLSL/this-reference-template.hlsl @@ -24,7 +24,7 @@ void main() { // CHECK: -CXXMethodDecl 0x{{[0-9A-Fa-f]+}} <line:8:3, line:10:3> line:8:5 getFirst 'K ()' implicit-inline // CHECK-NEXT:-CompoundStmt 0x{{[0-9A-Fa-f]+}} <col:16, line:10:3> // CHECK-NEXT:-ReturnStmt 0x{{[0-9A-Fa-f]+}} <line:9:4, col:16> -// CHECK-NEXT:-CXXDependentScopeMemberExpr 0x{{[0-9A-Fa-f]+}} <col:11, col:16> '<dependent type>' lvalue .First +// CHECK-NEXT:-MemberExpr 0x{{[0-9A-Fa-f]+}} <col:11, col:16> 'K' lvalue .First 0x{{[0-9A-Fa-f]+}} // CHECK-NEXT:-CXXThisExpr 0x{{[0-9A-Fa-f]+}} <col:11> 'Pair<K, V>' lvalue this // CHECK-NEXT:-CXXMethodDecl 0x{{[0-9A-Fa-f]+}} <line:12:3, line:14:3> line:12:5 getSecond 'V ()' implicit-inline // CHECK-NEXT:-CompoundStmt 0x{{[0-9A-Fa-f]+}} <col:17, line:14:3> diff --git a/clang/test/CXX/temp/temp.res/temp.dep/temp.dep.type/p4.cpp b/clang/test/CXX/temp/temp.res/temp.dep/temp.dep.type/p4.cpp new file mode 100644 index 00000000000000..7b36e3ad3b0131 --- /dev/null +++ b/clang/test/CXX/temp/temp.res/temp.dep/temp.dep.type/p4.cpp @@ -0,0 +1,110 @@ +// RUN: %clang_cc1 -Wno-unused-value -verify %s + +namespace N0 { + template<typename T> + struct A { + int x; + void f(); + using X = int; + + void not_instantiated(A *a, A &b) { + x; + f(); + new X; + + this->x; + this->f(); + this->A::x; + this->A::f(); + + a->x; + a->f(); + a->A::x; + a->A::f(); + + (*this).x; + (*this).f(); + (*this).A::x; + (*this).A::f(); + + b.x; + b.f(); + b.A::x; + b.A::f(); + + A::x; + A::f(); + new A::X; + + y; // expected-error{{use of undeclared identifier 'y'}} + g(); // expected-error{{use of undeclared identifier 'g'}} + new Y; // expected-error{{unknown type name 'Y'}} + + this->y; // expected-error{{no member named 'y' in 'A<T>'}} + this->g(); // expected-error{{no member named 'g' in 'A<T>'}} + this->A::y; // expected-error{{no member named 'y' in 'A<T>'}} + this->A::g(); // expected-error{{no member named 'g' in 'A<T>'}} + + a->y; // expected-error{{no member named 'y' in 'A<T>'}} + a->g(); // expected-error{{no member named 'g' in 'A<T>'}} + a->A::y; // expected-error{{no member named 'y' in 'A<T>'}} + a->A::g(); // expected-error{{no member named 'g' in 'A<T>'}} + + // FIXME: An overloaded unary 'operator*' is built for these + // even though the operand is a pointer (to a dependent type). + // Type::isOverloadableType should return false for such cases. + (*this).y; + (*this).g(); + (*this).A::y; + (*this).A::g(); + + b.y; // expected-error{{no member named 'y' in 'A<T>'}} + b.g(); // expected-error{{no member named 'g' in 'A<T>'}} + b.A::y; // expected-error{{no member named 'y' in 'A<T>'}} + b.A::g(); // expected-error{{no member named 'g' in 'A<T>'}} + + A::y; // expected-error{{no member named 'y' in 'A<T>'}} + A::g(); // expected-error{{no member named 'g' in 'A<T>'}} + new A::Y; // expected-error{{no type named 'Y' in 'A<T>'}} + } + }; +} // namespace N0 + +namespace N1 { + struct A { + template<int I> + void f(); + }; + + template<typename T> + struct B { + template<int I> + void f(); + + A x; + A g(); + + void not_instantiated(B *a, B &b) { + f<0>(); + this->f<0>(); + a->f<0>(); + // FIXME: This should not require 'template'! + (*this).f<0>(); // expected-error{{missing 'template' keyword prior to dependent template name 'f'}} + b.f<0>(); + + x.f<0>(); + this->x.f<0>(); + a->x.f<0>(); + // FIXME: This should not require 'template'! + (*this).x.f<0>(); // expected-error{{missing 'template' keyword prior to dependent template name 'f'}} + b.x.f<0>(); + + // FIXME: None of these should require 'template'! + g().f<0>(); // expected-error{{missing 'template' keyword prior to dependent template name 'f'}} + this->g().f<0>(); // expected-error{{missing 'template' keyword prior to dependent template name 'f'}} + a->g().f<0>(); // expected-error{{missing 'template' keyword prior to dependent template name 'f'}} + (*this).g().f<0>(); // expected-error{{missing 'template' keyword prior to dependent template name 'f'}} + b.g().f<0>(); // expected-error{{missing 'template' keyword prior to dependent template name 'f'}} + } + }; +} // namespace N1 diff --git a/clang/test/CodeGenCXX/mangle.cpp b/clang/test/CodeGenCXX/mangle.cpp index 31467d943840e0..d0800af55c87e8 100644 --- a/clang/test/CodeGenCXX/mangle.cpp +++ b/clang/test/CodeGenCXX/mangle.cpp @@ -1032,10 +1032,6 @@ namespace test51 { template <typename T> decltype(S1<T>().~S1<T>(), S1<T>().~S1<T>()) fun4() {}; template <typename T> - decltype(S1<int>().~S1<T>()) fun5(){}; - template <template <typename T> class U> - decltype(S1<int>().~U<int>()) fun6(){}; - template <typename T> decltype(E().E::~T()) fun7() {} template <template <typename> class U> decltype(X<int>::Y().U<int>::Y::~Y()) fun8() {} @@ -1047,10 +1043,6 @@ namespace test51 { // CHECK-LABEL: @_ZN6test514fun3I2S1IiEiEEDTcldtcvS1_IT0_E_EdnT_EEv template void fun4<int>(); // CHECK-LABEL: @_ZN6test514fun4IiEEDTcmcldtcv2S1IT_E_Edn2S1IS2_EEcldtcvS3__Edn2S1IS2_EEEv - template void fun5<int>(); - // CHECK-LABEL: @_ZN6test514fun5IiEEDTcldtcv2S1IiE_Edn2S1IT_EEEv - template void fun6<S1>(); - // CHECK-LABEL: @_ZN6test514fun6I2S1EEDTcldtcvS1_IiE_EdnT_IiEEEv template void fun7<E>(); // CHECK-LABEL: @_ZN6test514fun7INS_1EEEEDTcldtcvS1__Esr1EEdnT_EEv template void fun8<X>(); diff --git a/clang/test/Index/annotate-nested-name-specifier.cpp b/clang/test/Index/annotate-nested-name-specifier.cpp index a7338db6b05b77..3181497258407f 100644 --- a/clang/test/Index/annotate-nested-name-specifier.cpp +++ b/clang/test/Index/annotate-nested-name-specifier.cpp @@ -132,7 +132,7 @@ struct X8 { struct X9 : X8 { typedef X8 inherited; - void f() { + void f() { inherited::f(); } }; @@ -29... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/84050 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits