https://github.com/zyn0217 created https://github.com/llvm/llvm-project/pull/157062
We previously employed a TreeTransform to perform a task that should have been achieved by RAV. The TreeTransform approach was a bit wasteful, as we discarded the transform result and incurred some incorrect semantic analysis. Fixes https://github.com/llvm/llvm-project/issues/156225 >From fb991a4dc845327f9cf1f53c0943407b4ea0ca78 Mon Sep 17 00:00:00 2001 From: Younan Zhang <zyn7...@gmail.com> Date: Fri, 5 Sep 2025 17:55:56 +0800 Subject: [PATCH] [Clang] Convert ConstraintRefersToContainingTemplateChecker to using RAV We previously employed a TreeTransform to perform a task that should have been achieved by RAV. The TreeTransform approach was a bit wasteful, as we discarded the transform result and incurred some incorrect semantic analysis. --- clang/docs/ReleaseNotes.rst | 1 + clang/lib/AST/DynamicRecursiveASTVisitor.cpp | 2 +- clang/lib/Sema/SemaTemplate.cpp | 85 ++++++++++++-------- clang/test/SemaTemplate/concepts-friends.cpp | 18 +++++ 4 files changed, 73 insertions(+), 33 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 7311610a05396..e412e3f127369 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -337,6 +337,7 @@ Bug Fixes to C++ Support - Fix the parsing of variadic member functions when the ellipis immediately follows a default argument.(#GH153445) - Fixed a bug that caused ``this`` captured by value in a lambda with a dependent explicit object parameter to not be instantiated properly. (#GH154054) +- Fixed a bug where our ``member-like constrained friend`` checking caused an incorrect analysis of lambda captures. (#GH156225) Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/lib/AST/DynamicRecursiveASTVisitor.cpp b/clang/lib/AST/DynamicRecursiveASTVisitor.cpp index 8821cd332e918..6d7925b437b04 100644 --- a/clang/lib/AST/DynamicRecursiveASTVisitor.cpp +++ b/clang/lib/AST/DynamicRecursiveASTVisitor.cpp @@ -87,7 +87,7 @@ using namespace clang; // ends up executing RAV's implementation because we used a qualified // function call. // -// End result: RAV::TraverseCallExpr() is executed, +// End result: RAV::TraverseCallExpr() is executed. namespace { template <bool Const> struct Impl : RecursiveASTVisitor<Impl<Const>> { DynamicRecursiveASTVisitorBase<Const> &Visitor; diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp index 3d8416ac7dc1b..d96f14c84094b 100644 --- a/clang/lib/Sema/SemaTemplate.cpp +++ b/clang/lib/Sema/SemaTemplate.cpp @@ -1716,79 +1716,100 @@ NamedDecl *Sema::ActOnTemplateTemplateParameter( namespace { class ConstraintRefersToContainingTemplateChecker - : public TreeTransform<ConstraintRefersToContainingTemplateChecker> { + : public ConstDynamicRecursiveASTVisitor { + using inherited = ConstDynamicRecursiveASTVisitor; + Sema &SemaRef; bool Result = false; const FunctionDecl *Friend = nullptr; unsigned TemplateDepth = 0; // Check a record-decl that we've seen to see if it is a lexical parent of the // Friend, likely because it was referred to without its template arguments. - void CheckIfContainingRecord(const CXXRecordDecl *CheckingRD) { + bool CheckIfContainingRecord(const CXXRecordDecl *CheckingRD) { CheckingRD = CheckingRD->getMostRecentDecl(); if (!CheckingRD->isTemplated()) - return; + return true; for (const DeclContext *DC = Friend->getLexicalDeclContext(); DC && !DC->isFileContext(); DC = DC->getParent()) if (const auto *RD = dyn_cast<CXXRecordDecl>(DC)) - if (CheckingRD == RD->getMostRecentDecl()) + if (CheckingRD == RD->getMostRecentDecl()) { Result = true; + return false; + } + + return true; } - void CheckNonTypeTemplateParmDecl(NonTypeTemplateParmDecl *D) { + bool CheckNonTypeTemplateParmDecl(const NonTypeTemplateParmDecl *D) { if (D->getDepth() < TemplateDepth) Result = true; // Necessary because the type of the NTTP might be what refers to the parent // constriant. - TransformType(D->getType()); + return TraverseType(D->getType()); } public: - using inherited = TreeTransform<ConstraintRefersToContainingTemplateChecker>; - ConstraintRefersToContainingTemplateChecker(Sema &SemaRef, const FunctionDecl *Friend, unsigned TemplateDepth) - : inherited(SemaRef), Friend(Friend), TemplateDepth(TemplateDepth) {} + : SemaRef(SemaRef), Friend(Friend), TemplateDepth(TemplateDepth) {} + bool getResult() const { return Result; } // This should be the only template parm type that we have to deal with. - // SubstTempalteTypeParmPack, SubstNonTypeTemplateParmPack, and + // SubstTemplateTypeParmPack, SubstNonTypeTemplateParmPack, and // FunctionParmPackExpr are all partially substituted, which cannot happen // with concepts at this point in translation. - using inherited::TransformTemplateTypeParmType; - QualType TransformTemplateTypeParmType(TypeLocBuilder &TLB, - TemplateTypeParmTypeLoc TL, bool) { - if (TL.getDecl()->getDepth() < TemplateDepth) + bool VisitTemplateTypeParmType(const TemplateTypeParmType *Type) override { + if (Type->getDecl()->getDepth() < TemplateDepth) { Result = true; - return inherited::TransformTemplateTypeParmType( - TLB, TL, - /*SuppressObjCLifetime=*/false); + return false; + } + return true; } - Decl *TransformDecl(SourceLocation Loc, Decl *D) { - if (!D) - return D; + bool TraverseDeclRefExpr(const DeclRefExpr *E) override { + return TraverseDecl(E->getDecl()); + } + + bool TraverseTypedefType(const TypedefType *TT, + bool /*TraverseQualifier*/) override { + return TraverseType(TT->desugar()); + } + + bool TraverseTypeLoc(TypeLoc TL, bool TraverseQualifier) override { + // We don't care about TypeLocs. So traverse Types instead. + return TraverseType(TL.getType(), TraverseQualifier); + } + + bool VisitTagType(const TagType *T) override { + return TraverseDecl(T->getOriginalDecl()); + } + + bool TraverseDecl(const Decl *D) override { + assert(D); // FIXME : This is possibly an incomplete list, but it is unclear what other // Decl kinds could be used to refer to the template parameters. This is a // best guess so far based on examples currently available, but the // unreachable should catch future instances/cases. if (auto *TD = dyn_cast<TypedefNameDecl>(D)) - TransformType(TD->getUnderlyingType()); - else if (auto *NTTPD = dyn_cast<NonTypeTemplateParmDecl>(D)) - CheckNonTypeTemplateParmDecl(NTTPD); - else if (auto *VD = dyn_cast<ValueDecl>(D)) - TransformType(VD->getType()); - else if (auto *TD = dyn_cast<TemplateDecl>(D)) - TransformTemplateParameterList(TD->getTemplateParameters()); - else if (auto *RD = dyn_cast<CXXRecordDecl>(D)) - CheckIfContainingRecord(RD); - else if (isa<NamedDecl>(D)) { + return TraverseType(TD->getUnderlyingType()); + if (auto *NTTPD = dyn_cast<NonTypeTemplateParmDecl>(D)) + return CheckNonTypeTemplateParmDecl(NTTPD); + if (auto *VD = dyn_cast<ValueDecl>(D)) + return TraverseType(VD->getType()); + if (isa<TemplateDecl>(D)) + return true; + if (auto *RD = dyn_cast<CXXRecordDecl>(D)) + return CheckIfContainingRecord(RD); + + if (isa<NamedDecl, RequiresExprBodyDecl>(D)) { // No direct types to visit here I believe. } else llvm_unreachable("Don't know how to handle this declaration type yet"); - return D; + return true; } }; } // namespace @@ -1799,7 +1820,7 @@ bool Sema::ConstraintExpressionDependsOnEnclosingTemplate( assert(Friend->getFriendObjectKind() && "Only works on a friend"); ConstraintRefersToContainingTemplateChecker Checker(*this, Friend, TemplateDepth); - Checker.TransformExpr(const_cast<Expr *>(Constraint)); + Checker.TraverseStmt(Constraint); return Checker.getResult(); } diff --git a/clang/test/SemaTemplate/concepts-friends.cpp b/clang/test/SemaTemplate/concepts-friends.cpp index d05be423a8cfc..11287aa773b1b 100644 --- a/clang/test/SemaTemplate/concepts-friends.cpp +++ b/clang/test/SemaTemplate/concepts-friends.cpp @@ -548,3 +548,21 @@ Template<void, 4> f{}; static_assert(+Template<float, 5>{} == 5); } // namespace GH78101 + +namespace GH156225 { + +struct Test { + template <class T> + friend constexpr bool foo() + requires([] { + bool flags[1]; + for (bool x : flags) + return false; + return true; + }()) + { + return {}; + } +}; + +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits