llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-coroutines Author: Ambrose Leeb (Sirraide) <details> <summary>Changes</summary> This reverts 7c44c505cd7688a8994779f1751e0f9b4b95ab4c and relands #<!-- -->217110. There were two tests that needed updating: one because a warning that had previously been erroneously suppressed (because we thought we weren’t in a function) now works properly, and another because it crashes due to an unrelated bug. I’ve filed #<!-- -->223003 for the latter and moved the crash into a separate XFAIL test so it can be reenabled when the bug is fixed. The plan was to backport this to the 23 release branch, so still no release note. --- Patch is 25.76 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/223005.diff 14 Files Affected: - (modified) clang/include/clang/AST/DeclBase.h (+26) - (modified) clang/include/clang/AST/DeclCXX.h (+1-1) - (modified) clang/lib/AST/Decl.cpp (+1-3) - (modified) clang/lib/Sema/SemaChecking.cpp (+3-2) - (modified) clang/lib/Sema/SemaCoroutine.cpp (+7-9) - (modified) clang/lib/Sema/SemaDecl.cpp (+2-1) - (modified) clang/lib/Sema/SemaDeclCXX.cpp (+24-6) - (modified) clang/lib/Sema/SemaExpr.cpp (+2-1) - (modified) clang/test/Parser/cxx2c-expansion-statements.cpp (+2-2) - (modified) clang/test/SemaCXX/cxx2c-expansion-stmts-control-flow.cpp (+1-1) - (added) clang/test/SemaCXX/cxx2c-expansion-stmts-warnings.cpp (+16) - (modified) clang/test/SemaCXX/cxx2c-expansion-stmts.cpp (+200) - (modified) clang/test/SemaCXX/expansion-statements-local-extern-decls.cpp (+4-22) - (added) clang/test/SemaCXX/expansion-statements-local-extern-if-constexpr.cpp (+19) ``````````diff diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h index 9d233be282dbb..a067d87e92a84 100644 --- a/clang/include/clang/AST/DeclBase.h +++ b/clang/include/clang/AST/DeclBase.h @@ -2175,12 +2175,16 @@ class DeclContext { } } + /// Returns true if this DeclContext is a function, Objective-C method, + /// or block, or a DeclContext that can only occur in or is conceptually + /// treated like a function. bool isFunctionOrMethod() const { switch (getDeclKind()) { case Decl::Block: case Decl::Captured: case Decl::ObjCMethod: case Decl::TopLevelStmt: + case Decl::CXXExpansionStmt: return true; default: return getDeclKind() >= Decl::firstFunction && @@ -2188,6 +2192,28 @@ class DeclContext { } } + /// Cast this to a FunctionDecl if it is one, ignoring any intervening + /// expansion statements. Returns nullptr if this is not a function. + /// + /// In particular, this will return nullptr if the *nearest* enclosing + /// DeclContext that is not an expansion statement is something other + /// than a function (e.g. a CXXRecordDecl, even if it is a local class). + FunctionDecl *getEnclosingFunction() { + return dyn_cast<FunctionDecl>(getEnclosingNonExpansionStatementContext()); + } + + const FunctionDecl *getEnclosingFunction() const { + return dyn_cast<FunctionDecl>(getEnclosingNonExpansionStatementContext()); + } + + FunctionDecl *castEnclosingFunction() { + return cast<FunctionDecl>(getEnclosingNonExpansionStatementContext()); + } + + const FunctionDecl *castEnclosingFunction() 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 afe46fae1bceb..ff2223070dc15 100644 --- a/clang/include/clang/AST/DeclCXX.h +++ b/clang/include/clang/AST/DeclCXX.h @@ -1578,7 +1578,7 @@ class CXXRecordDecl : public RecordDecl { if (const auto *RD = dyn_cast<CXXRecordDecl>(getDeclContext())) return RD->isLocalClass(); - return dyn_cast<FunctionDecl>(getDeclContext()); + return getDeclContext()->getEnclosingFunction(); } FunctionDecl *isLocalClass() { diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index d1d296dd60d14..18808942128e6 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -1567,9 +1567,7 @@ LinkageInfo LinkageComputer::computeLVForDecl(const NamedDecl *D, // one such matching entity, the program is ill-formed. Otherwise, // if no matching entity is found, the block scope entity receives // external linkage. - if (D->getDeclContext() - ->getEnclosingNonExpansionStatementContext() - ->isFunctionOrMethod()) + if (D->getDeclContext()->isFunctionOrMethod()) return getLVForLocalDecl(D, computation); // C++ [basic.link]p6: diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 25996ddbc0558..ee7af4a68a84b 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -6186,7 +6186,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(); @@ -7841,7 +7842,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 d22260b887415..d68a9ec2d8f35 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->getEnclosingFunction(); if (!FD) { S.Diag(Loc, isa<ObjCMethodDecl>(S.CurContext) ? diag::err_coroutine_objc_method @@ -470,8 +470,7 @@ static void markCoroutineParametersReferenced(FunctionDecl &FD) { } VarDecl *Sema::buildCoroutinePromise(SourceLocation Loc) { - assert(isa<FunctionDecl>(CurContext) && "not in a function scope"); - auto *FD = cast<FunctionDecl>(CurContext); + auto *FD = CurContext->castEnclosingFunction(); bool IsThisDependentType = [&] { if (const auto *MD = dyn_cast_if_present<CXXMethodDecl>(FD)) return MD->isImplicitObjectMemberFunction() && @@ -583,7 +582,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->getEnclosingFunction() && "not in a function scope"); auto *ScopeInfo = S.getCurFunction(); assert(ScopeInfo && "missing function scope for function"); @@ -630,7 +629,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->castEnclosingFunction()->getLocation(), diag::err_coroutine_promise_final_suspend_requires_nothrow); } ThrowingDecls.insert(D); @@ -701,7 +700,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->getEnclosingFunction()); if (!checkCoroutineContext(*this, KWLoc, Keyword)) return false; @@ -726,7 +725,7 @@ bool Sema::ActOnCoroutineBodyStart(Scope *SC, SourceLocation KWLoc, ScopeInfo->setNeedsCoroutineSuspends(false); - auto *Fn = cast<FunctionDecl>(CurContext); + auto *Fn = CurContext->castEnclosingFunction(); SourceLocation Loc = Fn->getLocation(); // Build the initial suspend point auto buildSuspends = [&](StringRef Name) mutable -> StmtResult { @@ -2008,8 +2007,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->castEnclosingFunction(); auto *ScopeInfo = getCurFunction(); if (!ScopeInfo->CoroutineParameterMoves.empty()) diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 5de5821fe263e..742bb01a4ba81 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -6430,6 +6430,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(); @@ -7558,7 +7559,7 @@ static bool hasParsedAttr(Scope *S, const Declarator &PD, } bool Sema::adjustContextForLocalExternDecl(DeclContext *&DC) { - if (!DC->getEnclosingNonExpansionStatementContext()->isFunctionOrMethod()) + if (!DC->isFunctionOrMethod()) return false; // If this is a local extern function or variable declared within a function diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index ea628f29d8a00..7aa1368619608 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 @@ -2074,9 +2076,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 @@ -2263,15 +2262,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 7186aa86fae1e..fba46ef56625b 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -19473,7 +19473,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/test/Parser/cxx2c-expansion-statements.cpp b/clang/test/Parser/cxx2c-expansion-statements.cpp index 736f9fead383c..3a28b3268fd7a 100644 --- a/clang/test/Parser/cxx2c-expansion-statements.cpp +++ b/clang/test/Parser/cxx2c-expansion-statements.cpp @@ -23,7 +23,7 @@ void bad() { template for (__private_extern__ auto y : {1, 2}); // expected-error {{expansion variable 'y' may not be declared 'extern'}} template for (extern static auto y : {1, 2}); // expected-error {{cannot combine with previous 'extern' declaration specifier}} expected-error {{expansion variable 'y' may not be declared 'extern'}} template for (static auto y : {1, 2}); // expected-error {{expansion variable 'y' may not be declared 'static'}} - template for (thread_local auto y : {1, 2}); // expected-error {{'thread_local' variables must have global storage}} + template for (thread_local auto y : {1, 2}); // expected-error {{expansion variable 'y' may not be declared 'thread_local'}} template for (static thread_local auto y : {1, 2}); // expected-error {{expansion variable 'y' may not be declared 'thread_local'}} template for (__thread auto y : {1, 2}); // expected-error {{'__thread' variables must have global storage}} template for (static __thread auto y : {1, 2}); // expected-error {{expansion variable 'y' may not be declared 'static'}} @@ -32,7 +32,7 @@ void bad() { template for (int x; extern auto y : {1, 2}); // expected-error {{expansion variable 'y' may not be declared 'extern'}} template for (int x; extern static auto y : {1, 2}); // expected-error {{cannot combine with previous 'extern' declaration specifier}} expected-error {{expansion variable 'y' may not be declared 'extern'}} template for (int x; static auto y : {1, 2}); // expected-error {{expansion variable 'y' may not be declared 'static'}} - template for (int x; thread_local auto y : {1, 2}); // expected-error {{'thread_local' variables must have global storage}} + template for (int x; thread_local auto y : {1, 2}); // expected-error {{expansion variable 'y' may not be declared 'thread_local'}} template for (int x; static thread_local auto y : {1, 2}); // expected-error {{expansion variable 'y' may not be declared 'thread_local'}} template for (int x; __thread auto y : {1, 2}); // expected-error {{'__thread' variables must have global storage}} template for (int x; static __thread auto y : {1, 2}); // expected-error {{expansion variable 'y' may not be declared 'static'}} diff --git a/clang/test/SemaCXX/cxx2c-expansion-stmts-control-flow.cpp b/clang/test/SemaCXX/cxx2c-expansion-stmts-control-flow.cpp index 1001abae5f6ef..3a8bcb3075106 100644 --- a/clang/test/SemaCXX/cxx2c-expansion-stmts-control-flow.cpp +++ b/clang/test/SemaCXX/cxx2c-expansion-stmts-control-flow.cpp @@ -120,7 +120,7 @@ void GH210575(int i) { switch (i) { template for (auto x : {1, 2}) { switch (i) { - bar baz(); // expected-error {{unknown type name 'bar'}} + bar baz(); // expected-error {{unknown type name 'bar'}} expected-warning {{empty parentheses interpreted as a function declaration}} expected-note {{replace parentheses with an initializer to declare a variable}} } } } 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 class}} + void member(T) {} + }; + } +} + +void thread_local_var() { + template for (int x : {0}) { + thread_local int v1; + __thread int v2; // expected-error {{'__thread' variables must have global storage}} + static __thread int v3; + } +} + +task coro() { + template for (int x : {0}) { + co_await std::suspend_never{}; + co_yield 1; + co_return; + } + co_return; +} + +task coro_discarded() { + template for (int x : {}) { + co_await std::suspend_never{}; // expected-note {{function is a coroutine due to use of 'co_await' here}} + } + + // This is a coroutine even though the co_await above is discarded. + return task(); // expected-error {{return statement not allowed in coroutine; did you mean 'co_return'?}} +} + +void inline_static() { + template for (int x : {0}) { + inline int y = x; // expected-error {{inline declaration of 'y' not allowed in block scope}} + inline void f1(); // expected-error {{inline declaration of 'f1' not allowed in block scope}} + static void f2(); // expected-error {{function declared in block scope cannot have 'static' storage class}} + } +} + +struct VexingParse {}; +void vexing_parse() { + template for (int x : {}) { + VexingParse v(); // expected-warning {{empty parentheses interpreted as a function declaration}} expected-note {{remove parentheses to declare a variable}} + } +} + +void builtin_va_start(int x, ...) { + __builtin_va_list ap; + template for (int y : {1}) { + __builtin_va_start(... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/223005 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
