llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Ambrose Leeb (Sirraide) <details> <summary>Changes</summary> This fixes a number of problems around expansion statements, most of which arise from the fact that we check if `CurContext` is a `FunctionDecl` (which it isn't inside of an expansion statement) and then complain that we're not inside a function (even though we are). I also added a helper to `DeclContext` to check if we're in a function/block/ObjC method while ignoring any intervening expansion statements, as well as few to cast a `DeclContext` to a `FunctionDecl` (also while ignoring expansion statements). --- Patch is 23.40 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/217110.diff 14 Files Affected: - (modified) clang/docs/ReleaseNotes.md (+6) - (modified) clang/include/clang/AST/DeclBase.h (+24) - (modified) clang/include/clang/AST/DeclCXX.h (+1-1) - (modified) clang/lib/AST/ByteCode/Interp.h (+2-2) - (modified) clang/lib/AST/Decl.cpp (+2-2) - (modified) clang/lib/AST/ExprConstant.cpp (+2-2) - (modified) clang/lib/Sema/SemaChecking.cpp (+3-2) - (modified) clang/lib/Sema/SemaCoroutine.cpp (+7-9) - (modified) clang/lib/Sema/SemaDecl.cpp (+6-5) - (modified) clang/lib/Sema/SemaDeclCXX.cpp (+24-6) - (modified) clang/lib/Sema/SemaExpr.cpp (+2-1) - (modified) clang/lib/Sema/SemaType.cpp (+1-1) - (added) clang/test/SemaCXX/cxx2c-expansion-stmts-warnings.cpp (+16) - (modified) clang/test/SemaCXX/cxx2c-expansion-stmts.cpp (+200) ``````````diff diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index e4a6f72f8fec5..ea7b7ff9391d7 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -476,6 +476,12 @@ features cannot lower the translation-unit ABI level; - Fixed merging of lambdas across modules in the case where neither lambda is imported from an AST file. (#GH214560) +- Fixed a number issues arising from the fact that Clang considered the body of + an expansion statement to not be inside a function in some contexts. Several + constructs that were previously incorrectly rejected inside expansion statements + (e.g. `thread_local` variables, `va_start`, and `co_await`/`co_yield`/`co_return`) + are now accepted, and vice versa. + #### Bug Fixes to AST Handling - Fixed a non-deterministic ordering of unused local typedefs that made diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h index 9d233be282dbb..0e0c99cec389b 100644 --- a/clang/include/clang/AST/DeclBase.h +++ b/clang/include/clang/AST/DeclBase.h @@ -2188,6 +2188,30 @@ class DeclContext { } } + /// Test whether we're directly inside a function or method, but ignoring + /// any intervening expansion statements. + bool isInsideFunctionOrMethod() const { + return getEnclosingNonExpansionStatementContext()->isFunctionOrMethod(); + } + + /// Cast this to a FunctionDecl if it is one, ignoring any intervening + /// expansion statements. Returns nullptr if this is not a function. + FunctionDecl *getAsFunctionDecl() { + return dyn_cast<FunctionDecl>(getEnclosingNonExpansionStatementContext()); + } + + const FunctionDecl *getAsFunctionDecl() const { + return dyn_cast<FunctionDecl>(getEnclosingNonExpansionStatementContext()); + } + + FunctionDecl *castAsFunctionDecl() { + return cast<FunctionDecl>(getEnclosingNonExpansionStatementContext()); + } + + const FunctionDecl *castAsFunctionDecl() const { + return cast<FunctionDecl>(getEnclosingNonExpansionStatementContext()); + } + /// Test whether the context supports looking up names. bool isLookupContext() const { return !isFunctionOrMethod() && getDeclKind() != Decl::LinkageSpec && diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h index a42884be71d68..13969828029df 100644 --- a/clang/include/clang/AST/DeclCXX.h +++ b/clang/include/clang/AST/DeclCXX.h @@ -1574,7 +1574,7 @@ class CXXRecordDecl : public RecordDecl { if (const auto *RD = dyn_cast<CXXRecordDecl>(getDeclContext())) return RD->isLocalClass(); - return dyn_cast<FunctionDecl>(getDeclContext()); + return getDeclContext()->getAsFunctionDecl(); } FunctionDecl *isLocalClass() { diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h index 054fba2c87c45..f45174e1c86f9 100644 --- a/clang/lib/AST/ByteCode/Interp.h +++ b/clang/lib/AST/ByteCode/Interp.h @@ -2748,8 +2748,8 @@ inline bool SubPtr(InterpState &S, CodePtr OpPC, uint32_t ElemSize) { return false; } - if (LHSAddrExpr->getLabel()->getDeclContext() != - RHSAddrExpr->getLabel()->getDeclContext()) + if (LHSAddrExpr->getLabel()->getDeclContext()->getAsFunctionDecl() != + RHSAddrExpr->getLabel()->getDeclContext()->getAsFunctionDecl()) return Invalid(S, OpPC); S.Stk.push<T>(LHSAddrExpr, RHSAddrExpr); diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index 152c621bc1ef4..15a2fc40bd887 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -1106,7 +1106,7 @@ bool NamedDecl::isPlaceholderVar(const LangOptions &LangOpts) const { if (isa<FieldDecl>(this)) return true; if (const auto *IFD = dyn_cast<IndirectFieldDecl>(this)) { - if (!getDeclContext()->isFunctionOrMethod() && + if (!getDeclContext()->isInsideFunctionOrMethod() && !getDeclContext()->isRecord()) return false; const VarDecl *VD = IFD->getVarDecl(); @@ -1121,7 +1121,7 @@ bool NamedDecl::isPlaceholderVar(const LangOptions &LangOpts) const { return VD->getStorageDuration() == StorageDuration::SD_Automatic; } if (const auto *BD = dyn_cast<BindingDecl>(this); - BD && getDeclContext()->isFunctionOrMethod()) { + BD && getDeclContext()->isInsideFunctionOrMethod()) { const VarDecl *VD = BD->getHoldingVar(); return !VD || VD->getStorageDuration() == StorageDuration::SD_Automatic; } diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index 480d5119a5363..67cffe0fc46b8 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -19007,8 +19007,8 @@ bool DataRecursiveIntBinOpEvaluator:: if (!LHSAddrExpr || !RHSAddrExpr) return false; // Make sure both labels come from the same function. - if (LHSAddrExpr->getLabel()->getDeclContext() != - RHSAddrExpr->getLabel()->getDeclContext()) + if (LHSAddrExpr->getLabel()->getDeclContext()->getAsFunctionDecl() != + RHSAddrExpr->getLabel()->getDeclContext()->getAsFunctionDecl()) return false; Result = APValue(LHSAddrExpr, RHSAddrExpr); return true; diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index f2f38c84dc5f8..17a04c391b169 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -6180,7 +6180,8 @@ static bool checkVAStartIsInVariadicFunction(Sema &S, Expr *Fn, // and get its parameter list. bool IsVariadic = false; ArrayRef<ParmVarDecl *> Params; - DeclContext *Caller = S.CurContext; + DeclContext *Caller = + S.CurContext->getEnclosingNonExpansionStatementContext(); if (auto *Block = dyn_cast<BlockDecl>(Caller)) { IsVariadic = Block->isVariadic(); Params = Block->parameters(); @@ -7828,7 +7829,7 @@ static bool CheckMissingFormatAttribute( if (S->getDiagnostics().isIgnored(diag::warn_missing_format_attribute, Loc)) return false; - DeclContext *DC = S->CurContext; + DeclContext *DC = S->CurContext->getEnclosingNonExpansionStatementContext(); if (!isa<ObjCMethodDecl>(DC) && !isa<FunctionDecl>(DC) && !isa<BlockDecl>(DC)) return false; Decl *Caller = cast<Decl>(DC)->getCanonicalDecl(); diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp index 48ee5cc0b0836..7879f55f091ca 100644 --- a/clang/lib/Sema/SemaCoroutine.cpp +++ b/clang/lib/Sema/SemaCoroutine.cpp @@ -186,7 +186,7 @@ static bool isValidCoroutineContext(Sema &S, SourceLocation Loc, // appear in a default argument." But the diagnostic QoI here could be // improved to inform the user that default arguments specifically are not // allowed. - auto *FD = dyn_cast<FunctionDecl>(S.CurContext); + auto FD = S.CurContext->getAsFunctionDecl(); if (!FD) { S.Diag(Loc, isa<ObjCMethodDecl>(S.CurContext) ? diag::err_coroutine_objc_method @@ -464,8 +464,7 @@ static ExprResult buildPromiseCall(Sema &S, VarDecl *Promise, } VarDecl *Sema::buildCoroutinePromise(SourceLocation Loc) { - assert(isa<FunctionDecl>(CurContext) && "not in a function scope"); - auto *FD = cast<FunctionDecl>(CurContext); + auto *FD = CurContext->castAsFunctionDecl(); bool IsThisDependentType = [&] { if (const auto *MD = dyn_cast_if_present<CXXMethodDecl>(FD)) return MD->isImplicitObjectMemberFunction() && @@ -573,7 +572,7 @@ static FunctionScopeInfo *checkCoroutineContext(Sema &S, SourceLocation Loc, if (!isValidCoroutineContext(S, Loc, Keyword)) return nullptr; - assert(isa<FunctionDecl>(S.CurContext) && "not in a function scope"); + assert(S.CurContext->getAsFunctionDecl() && "not in a function scope"); auto *ScopeInfo = S.getCurFunction(); assert(ScopeInfo && "missing function scope for function"); @@ -620,7 +619,7 @@ static void checkNoThrow(Sema &S, const Stmt *E, // potentially-throwing ([except.spec]). // // First time seeing an error, emit the error message. - S.Diag(cast<FunctionDecl>(S.CurContext)->getLocation(), + S.Diag(S.CurContext->castAsFunctionDecl()->getLocation(), diag::err_coroutine_promise_final_suspend_requires_nothrow); } ThrowingDecls.insert(D); @@ -691,7 +690,7 @@ bool Sema::ActOnCoroutineBodyStart(Scope *SC, SourceLocation KWLoc, // Ignore previous expr evaluation contexts. EnterExpressionEvaluationContextForFunction PotentiallyEvaluated( *this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated, - dyn_cast_or_null<FunctionDecl>(CurContext)); + CurContext->getAsFunctionDecl()); if (!checkCoroutineContext(*this, KWLoc, Keyword)) return false; @@ -716,7 +715,7 @@ bool Sema::ActOnCoroutineBodyStart(Scope *SC, SourceLocation KWLoc, ScopeInfo->setNeedsCoroutineSuspends(false); - auto *Fn = cast<FunctionDecl>(CurContext); + auto *Fn = CurContext->castAsFunctionDecl(); SourceLocation Loc = Fn->getLocation(); // Build the initial suspend point auto buildSuspends = [&](StringRef Name) mutable -> StmtResult { @@ -1968,8 +1967,7 @@ static VarDecl *buildVarDecl(Sema &S, SourceLocation Loc, QualType Type, // Build statements that move coroutine function parameters to the coroutine // frame, and store them on the function scope info. bool Sema::buildCoroutineParameterMoves(SourceLocation Loc) { - assert(isa<FunctionDecl>(CurContext) && "not in a function scope"); - auto *FD = cast<FunctionDecl>(CurContext); + auto *FD = CurContext->castAsFunctionDecl(); auto *ScopeInfo = getCurFunction(); if (!ScopeInfo->CoroutineParameterMoves.empty()) diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index d87710d3cf140..d8997c41e2ad8 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -2054,7 +2054,7 @@ static bool ShouldDiagnoseUnusedDecl(const LangOptions &LangOpts, // Except for labels, we only care about unused decls that are local to // functions. - bool WithinFunction = D->getDeclContext()->isFunctionOrMethod(); + bool WithinFunction = D->getDeclContext()->isInsideFunctionOrMethod(); if (const auto *R = dyn_cast<CXXRecordDecl>(D->getDeclContext())) // For dependent types, the diagnostic is deferred. WithinFunction = @@ -6410,6 +6410,7 @@ bool Sema::diagnoseQualifiedDeclaration(CXXScopeSpec &SS, DeclContext *DC, // declaration. For a template-id, we perform the checks in // CheckTemplateSpecializationScope. if (!Cur->Encloses(DC) && !(TemplateId || IsMemberSpecialization)) { + Cur = Cur->getEnclosingNonExpansionStatementContext(); if (Cur->isRecord()) Diag(Loc, diag::err_member_qualification) << Name << SS.getRange(); @@ -8115,7 +8116,7 @@ NamedDecl *Sema::ActOnVariableDeclarator( if (!getLangOpts().CPlusPlus) { Diag(D.getDeclSpec().getInlineSpecLoc(), diag::err_inline_non_function) << 0; - } else if (CurContext->isFunctionOrMethod()) { + } else if (CurContext->isInsideFunctionOrMethod()) { // 'inline' is not allowed on block scope variable declaration. Diag(D.getDeclSpec().getInlineSpecLoc(), diag::err_inline_declaration_block_scope) << Name @@ -8153,7 +8154,7 @@ NamedDecl *Sema::ActOnVariableDeclarator( if (NewVD->hasLocalStorage() && (SCSpec != DeclSpec::SCS_unspecified || TSCS != DeclSpec::TSCS_thread_local || - !DC->isFunctionOrMethod())) + !DC->isInsideFunctionOrMethod())) Diag(D.getDeclSpec().getThreadStorageClassSpecLoc(), diag::err_thread_non_global) << DeclSpec::getSpecifierName(TSCS); @@ -9592,7 +9593,7 @@ static StorageClass getFunctionStorageClass(Sema &SemaRef, Declarator &D) { return SC_None; return SC_Extern; case DeclSpec::SCS_static: { - if (SemaRef.CurContext->getRedeclContext()->isFunctionOrMethod()) { + if (SemaRef.CurContext->getRedeclContext()->isInsideFunctionOrMethod()) { // C99 6.7.1p5: // The declaration of an identifier for a function that has // block scope shall have no explicit storage-class specifier @@ -10423,7 +10424,7 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC, // The inline specifier shall not appear on a block scope function // declaration. if (isInline && !NewFD->isInvalidDecl()) { - if (CurContext->isFunctionOrMethod()) { + if (CurContext->isInsideFunctionOrMethod()) { // 'inline' is not allowed on block scope function declaration. Diag(D.getDeclSpec().getInlineSpecLoc(), diag::err_inline_declaration_block_scope) << Name diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index dd95f9220bb9d..df3cdba48619b 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -640,7 +640,9 @@ bool Sema::MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old, << (New->getTemplateSpecializationKind() ==TSK_ExplicitSpecialization) << New->getDeclName() << NewParam->getDefaultArgRange(); - } else if (New->getDeclContext()->isDependentContext()) { + } else if (New->getDeclContext() + ->getEnclosingNonExpansionStatementContext() + ->isDependentContext()) { // C++ [dcl.fct.default]p6 (DR217): // Default arguments for a member function of a class template shall // be specified on the initial declaration of the member function @@ -2068,9 +2070,6 @@ static bool CheckConstexprDeclStmt(Sema &SemaRef, const FunctionDecl *Dcl, // - using-enum-declaration continue; - case Decl::CXXExpansionStmt: - continue; - case Decl::Typedef: case Decl::TypeAlias: { // - typedef declarations and alias-declarations that do not define @@ -2257,15 +2256,34 @@ CheckConstexprFunctionStmt(Sema &SemaRef, const FunctionDecl *Dcl, Stmt *S, // - null statements, return true; - case Stmt::DeclStmtClass: + case Stmt::DeclStmtClass: { + auto *DS = cast<DeclStmt>(S); + + // Expansion statement 'declarations' have substatements, so we need to + // handle them separately. + if (DS->isSingleDecl()) { + if (auto *ESD = dyn_cast<CXXExpansionStmtDecl>(DS->getSingleDecl())) { + // Don't check unexpanded expansion statements. + if (!ESD->getInstantiations()) + return true; + for (auto *BodyIt : ESD->getInstantiations()->getInstantiations()) { + if (!CheckConstexprFunctionStmt(SemaRef, Dcl, BodyIt, ReturnStmts, + Cxx1yLoc, Cxx2aLoc, Cxx2bLoc, Kind)) + return false; + } + return true; + } + } + // - static_assert-declarations // - using-declarations, // - using-directives, // - typedef declarations and alias-declarations that do not define // classes or enumerations, - if (!CheckConstexprDeclStmt(SemaRef, Dcl, cast<DeclStmt>(S), Cxx1yLoc, Kind)) + if (!CheckConstexprDeclStmt(SemaRef, Dcl, DS, Cxx1yLoc, Kind)) return false; return true; + } case Stmt::ReturnStmtClass: // - and exactly one return statement; diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index f25829ae676dc..3333993a4566e 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -19335,7 +19335,8 @@ void Sema::MarkCaptureUsedInEnclosingContext(ValueDecl *Capture, static void diagnoseUncapturableValueReferenceOrBinding(Sema &S, SourceLocation loc, ValueDecl *var) { - DeclContext *VarDC = var->getDeclContext(); + DeclContext *VarDC = + var->getDeclContext()->getEnclosingNonExpansionStatementContext(); // If the parameter still belongs to the translation unit, then // we're actually just using one parameter in the declaration of diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp index f9033ecb48581..0c5becf5379f7 100644 --- a/clang/lib/Sema/SemaType.cpp +++ b/clang/lib/Sema/SemaType.cpp @@ -3551,7 +3551,7 @@ static void warnAboutAmbiguousFunction(Sema &S, Declarator &D, // doesn't have a storage class (such as 'extern') specified. if (!D.isFunctionDeclarator() || D.getFunctionDefinitionKind() != FunctionDefinitionKind::Declaration || - !S.CurContext->isFunctionOrMethod() || + !S.CurContext->isInsideFunctionOrMethod() || D.getDeclSpec().getStorageClassSpec() != DeclSpec::SCS_unspecified) return; diff --git a/clang/test/SemaCXX/cxx2c-expansion-stmts-warnings.cpp b/clang/test/SemaCXX/cxx2c-expansion-stmts-warnings.cpp new file mode 100644 index 0000000000000..6ef468f2cd1fb --- /dev/null +++ b/clang/test/SemaCXX/cxx2c-expansion-stmts-warnings.cpp @@ -0,0 +1,16 @@ +// RUN: %clang_cc1 %s -std=c++2c -fsyntax-only -verify=expected,old-interp +// RUN: %clang_cc1 %s -std=c++2c -fsyntax-only -verify=expected,new-interp -fexperimental-new-constant-interpreter + +// Test that checks for warnings that should be emitted in expansion statements, +// but which are suppressed if we saw an error (which is why they're in a separate +// file). + +#pragma GCC diagnostic warning "-Wunused-variable" +#pragma GCC diagnostic warning "-Wunused-local-typedefs" +void unused() { + template for (int init_stmt; int expansion_var : {0}) { // expected-warning {{unused variable 'init_stmt'}} expected-warning {{unused variable 'expansion_var'}} + int unused_var; // expected-warning {{unused variable 'unused_var'}} + using unused_type = int; // expected-warning {{unused type alias 'unused_type'}} + typedef int unused_typedef; // expected-warning {{unused typedef 'unused_typedef'}} + } +} diff --git a/clang/test/SemaCXX/cxx2c-expansion-stmts.cpp b/clang/test/SemaCXX/cxx2c-expansion-stmts.cpp index dd450a8f1b76c..44189ff31dae8 100644 --- a/clang/test/SemaCXX/cxx2c-expansion-stmts.cpp +++ b/clang/test/SemaCXX/cxx2c-expansion-stmts.cpp @@ -1607,3 +1607,203 @@ T tf() { template long tf<long>(); } + +// Boilerplate needed for tests involving coroutines +namespace std { +template <class... Args> +struct void_t_imp { + using type = void; +}; +template <class... Args> +using void_t = typename void_t_imp<Args...>::type; + +template <class T, class = void> +struct traits_sfinae_base {}; + +template <class T> +struct traits_sfinae_base<T, void_t<typename T::promise_type>> { + using promise_type = typename T::promise_type; +}; + +template <class Ret, class... Args> +struct coroutine_traits : public traits_sfinae_base<Ret> {}; + +template <class PromiseType = void> +struct coroutine_handle { + static coroutine_handle from_address(void *) noexcept; + static coroutine_handle from_promise(PromiseType &promise); +}; +template <> +struct coroutine_handle<void> { + template <class PromiseType> + coroutine_handle(coroutine_handle<PromiseType>) noexcept; + static coroutine_handle from_address(void *) noexcept; + template <class PromiseType> + static coroutine_handle from_promise(PromiseType &promise); +}; + +struct suspend_always { + bool await_ready() noexcept { return false; } + template <typename F> + void await_suspend(F) noexcept; + void await_resume() noexcept {} +}; + +struct suspend_never { + bool await_ready() noexcept { return true; } + template <typename F> + void await_suspend(F) noexcept; + void await_resume() noexcept {} +}; +} // namespace std + +struct task { + struct promise_type { + task get_return_object() { return {}; } + std::suspend_never initial_suspend() noexcept { return {}; } + std::suspend_never final_suspend() noexcept { return {}; } + void return_void() {} + std::suspend_never yield_value(int) { return {}; } + void unhandled_exception() {} + }; +}; + +namespace decl_context_issues { +void local_class() { + template for (int x : {0}) { + struct Local { + template <class T> // expected-error {{templates cannot be declared inside of a local class}} + void member(T) {} + }; + + template for (int y : {1}) { + struct Nested { + template <class T> // expected-error {{templates cannot be declared inside of a local class}} + void member(T) {} + }; + } + } + + template for (int x : {}) { + struct DiscardedLocal { + template <class T> // expected-error {{templates cannot be declared inside of a local ... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/217110 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
