[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
AaronBallman wrote: > We need code that compiles in both GCC and Clang without modifying existing > source—not some separate, incompatible alternative from Clang. To be clear, this is not some separate, incompatible thing. The C standards committee produced a technical specification describing a feature our users were quite happy for us to support. GCC also has an in-progress implementation of the same functionality. This is new feature work just like any other, the main difference being that we're not obligated to implement it for conformance reasons. Further, `_Defer` and nested functions are different features serving different needs, so it's not clear why you're bringing them up in this context. Nested functions are something the Clang community has considered in the past and we do not have plans to support at this time. However, WG14 is considering proposals for functionality in this space, including nested functions from GCC, so it's possible the committee will standardize something in that space at some point. But right now, nested functions are actually the separate, incompatible thing -- they're not standard C and so they're not supported in most C compilers. https://github.com/llvm/llvm-project/pull/162848 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
top-rannuo wrote: _Defer is too ugly https://github.com/llvm/llvm-project/pull/162848 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
Sirraide wrote: > except `#include ` fails even with `-std=c2y -fdefer-ts` 🤔 No I forgot to add that to our list of headers; should be fixed by #172512 https://github.com/llvm/llvm-project/pull/162848 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
fdwr wrote: Cool, `_Defer` works nicely in Godbolt clang trunk (https://godbolt.org/z/zfTjPK66o), except `#include ` fails even with `-std=c2y -fdefer-ts` 🤔. I'll try it again next week in case Godbolt's version is not new enough yet. Thank you for implementing this Sirraide 🙌. https://github.com/llvm/llvm-project/pull/162848 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
thradams wrote: I think there is also a problem in flow analysis. I opened #172338 https://github.com/llvm/llvm-project/pull/162848 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
Sirraide wrote: > My suggestion is to add a note identifying the defer execution that triggered > the warning.(The return line) > In complicated code with multiple defer executions, this may be hard to find. Hmm, I’m no familiar enough w/ how `-Wuninitialized` works to be able to tell how hard it would be to implement this—and I’m not sure how useful that would be in practice. Also, I’d suggest opening a new issue for this since this pr is already closed now. https://github.com/llvm/llvm-project/pull/162848 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
thradams wrote:
Suggestion:
Consider this code:
```c
void f2(int i) {}
void f(int k) {
int i;
_Defer f2(i);
if (k > 1) return; //defer executed
if (k > 2) return; //defer executed
i = 1;
} //defer executed
```
The warning using -Wall is: (https://godbolt.org/z/3PKxMKMKM)
```
:4:15: warning: variable 'i' is uninitialized when used here
[-Wuninitialized]
4 | _Defer f2(i);
| ^
:3:10: note: initialize the variable 'i' to silence this warning
3 | int i;
| ^
| = 0
1 warning generated.
ASM generation compiler returned: 0
:4:15: warning: variable 'i' is uninitialized when used here
[-Wuninitialized]
4 | _Defer f2(i);
| ^
:3:10: note: initialize the variable 'i' to silence this warning
3 | int i;
| ^
| = 0
1 warning generated.
Execution build compiler returned: 0
Program returned: 0
```
My suggestion is to add a note identifying the defer execution that triggered
the warning.(The return line)
In complicated code with multiple defer executions, this may be hard to find.
https://github.com/llvm/llvm-project/pull/162848
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
llvm-ci wrote: LLVM Buildbot has detected a new failure on builder `sanitizer-aarch64-linux-bootstrap-ubsan` running on `sanitizer-buildbot10` while building `clang-tools-extra,clang` at step 2 "annotate". Full details are available at: https://lab.llvm.org/buildbot/#/builders/85/builds/16580 Here is the relevant piece of the build log for the reference ``` Step 2 (annotate) failure: 'python ../sanitizer_buildbot/sanitizers/zorg/buildbot/builders/sanitizers/buildbot_selector.py' (failure) ... llvm-lit: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:564: note: using lld-link: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build2_ubsan/bin/lld-link llvm-lit: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:564: note: using ld64.lld: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build2_ubsan/bin/ld64.lld llvm-lit: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:564: note: using wasm-ld: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build2_ubsan/bin/wasm-ld llvm-lit: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:564: note: using ld.lld: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build2_ubsan/bin/ld.lld llvm-lit: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:564: note: using lld-link: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build2_ubsan/bin/lld-link llvm-lit: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:564: note: using ld64.lld: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build2_ubsan/bin/ld64.lld llvm-lit: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:564: note: using wasm-ld: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build2_ubsan/bin/wasm-ld llvm-lit: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/utils/lit/lit/main.py:74: note: The test suite configuration requested an individual test timeout of 0 seconds but a timeout of 900 seconds was requested on the command line. Forcing timeout to be 900 seconds. -- Testing: 89641 tests, 72 workers -- Testing: 0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. FAIL: lld :: ELF/ppc64-toc-call-to-pcrel-long-jump.s (88707 of 89641) TEST 'lld :: ELF/ppc64-toc-call-to-pcrel-long-jump.s' FAILED Exit Code: -11 Command Output (stdout): -- # RUN: at line 2 split-file /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/lld/test/ELF/ppc64-toc-call-to-pcrel-long-jump.s /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build2_ubsan/tools/lld/test/ELF/Output/ppc64-toc-call-to-pcrel-long-jump.s.tmp # executed command: split-file /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/lld/test/ELF/ppc64-toc-call-to-pcrel-long-jump.s /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build2_ubsan/tools/lld/test/ELF/Output/ppc64-toc-call-to-pcrel-long-jump.s.tmp # note: command had no output on stdout or stderr # RUN: at line 4 /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build2_ubsan/bin/llvm-mc -filetype=obj -triple=powerpc64le /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build2_ubsan/tools/lld/test/ELF/Output/ppc64-toc-call-to-pcrel-long-jump.s.tmp/asm -o /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build2_ubsan/tools/lld/test/ELF/Output/ppc64-toc-call-to-pcrel-long-jump.s.tmp.o # executed command: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build2_ubsan/bin/llvm-mc -filetype=obj -triple=powerpc64le /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build2_ubsan/tools/lld/test/ELF/Output/ppc64-toc-call-to-pcrel-long-jump.s.tmp/asm -o /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build2_ubsan/tools/lld/test/ELF/Output/ppc64-toc-call-to-pcrel-long-jump.s.tmp.o # note: command had no output on stdout or stderr # RUN: at line 5 /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build2_ubsan/bin/ld.lld -T /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build2_ubsan/tools/lld/test/ELF/Output/ppc64-toc-call-to-pcrel-long-jump.s.tmp/lts /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build2_ubsan/tools/lld/test/ELF/Output/ppc64-toc-call-to-pcrel-long-jump.s.tmp.o -o /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build2_ubsan/tools/lld/test/ELF/Output/ppc64-toc-call-to-pcrel-long-jump.s.tmp_le # executed command: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build2_ubsan/bin/ld.lld -T /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build2_ubsan/tools/lld/test/ELF/Output/ppc64-toc-call-to-pcrel-long-jump.s.t
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
https://github.com/Sirraide closed https://github.com/llvm/llvm-project/pull/162848 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
https://github.com/cor3ntin approved this pull request. Thanks! https://github.com/llvm/llvm-project/pull/162848 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
https://github.com/AaronBallman approved this pull request. This LGTM, but I don't feel comfortable giving final sign-off because of the CodeGen changes. @rjmccall @efriedma-quic could you give the codegen changes a look? Thanks! https://github.com/llvm/llvm-project/pull/162848 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
@@ -0,0 +1,5 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s +// RUN: %clang_cc1 -fsyntax-only -verify -fdefer-ts %s + +// expected-no-diagnostics +int defer; Sirraide wrote: I thought I found all of them but I missed this one; thanks https://github.com/llvm/llvm-project/pull/162848 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
https://github.com/AaronBallman commented: Code generally looks good to me, but this still needs some dedicated review for CodeGen. I did find a testing bug though. https://github.com/llvm/llvm-project/pull/162848 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
@@ -0,0 +1,5 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s +// RUN: %clang_cc1 -fsyntax-only -verify -fdefer-ts %s + +// expected-no-diagnostics +int defer; AaronBallman wrote: ```suggestion int _Defer; ``` otherwise we're not testing what we think we're testing. :-) I still think it makes sense to warn about enabling the flag in C++ mode just so users don't get baffling diagnostics. https://github.com/llvm/llvm-project/pull/162848 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/162848 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
@@ -498,6 +498,11 @@ static void InitializeStandardPredefinedMacros(const
TargetInfo &TI,
Builder.defineMacro("__STDC_EMBED_EMPTY__",
llvm::itostr(static_cast(EmbedResult::Empty)));
+ // We define this to '1' here to indicate that we only support '_Defer'
+ // as a keyword; it will be set to '2' by .
cor3ntin wrote:
There is a mismatch between these comment and your implementation of stddefer.h
https://github.com/llvm/llvm-project/pull/162848
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
@@ -2000,6 +2004,80 @@ void CodeGenFunction::EmitDefaultStmt(const DefaultStmt
&S,
EmitStmt(S.getSubStmt());
}
+namespace {
+struct EmitDeferredStatement final : EHScopeStack::Cleanup {
+ const DeferStmt &Stmt;
+ EmitDeferredStatement(const DeferStmt *Stmt) : Stmt(*Stmt) {}
+
+ void Emit(CodeGenFunction &CGF, Flags) override {
+// Take care that any cleanups pushed by the body of the 'defer' don't
+// clobber the current cleanup slot value.
+//
+// This situation warrants some explanation: Assume we have a scope that
cor3ntin wrote:
```suggestion
// Assume we have a scope that
```
https://github.com/llvm/llvm-project/pull/162848
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
@@ -2370,6 +2373,27 @@ StmtResult Parser::ParseReturnStatement() {
return Actions.ActOnReturnStmt(ReturnLoc, R.get(), getCurScope());
}
+StmtResult Parser::ParseDeferStatement(SourceLocation *TrailingElseLoc) {
+ assert(Tok.is(tok::kw__Defer));
+ SourceLocation DeferLoc = ConsumeToken();
+ Actions.ActOnStartOfDeferStmt(DeferLoc, getCurScope());
+ auto OnError = llvm::make_scope_exit(
cor3ntin wrote:
```suggestion
SourceLocation DeferLoc = ConsumeToken();
Actions.ActOnStartOfDeferStmt(DeferLoc, getCurScope());
auto OnError = llvm::make_scope_exit(
```
https://github.com/llvm/llvm-project/pull/162848
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
@@ -3267,12 +3267,22 @@ Sema::ActOnIndirectGotoStmt(SourceLocation GotoLoc,
SourceLocation StarLoc,
return new (Context) IndirectGotoStmt(GotoLoc, StarLoc, E);
}
-static void CheckJumpOutOfSEHFinally(Sema &S, SourceLocation Loc,
- const Scope &DestScope) {
+static void CheckJumpOutOfSEHFinallyOrDefer(Sema &S, SourceLocation Loc,
+const Scope &DestScope,
+unsigned DeferJumpKind) {
if (!S.CurrentSEHFinally.empty() &&
DestScope.Contains(*S.CurrentSEHFinally.back())) {
S.Diag(Loc, diag::warn_jump_out_of_seh_finally);
}
+
+ if (!S.CurrentDefer.empty()) {
+Scope *Parent = S.CurrentDefer.back().first;
+
cor3ntin wrote:
maybe an assert that Parent is not null would be useful here
https://github.com/llvm/llvm-project/pull/162848
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
@@ -2000,6 +2004,80 @@ void CodeGenFunction::EmitDefaultStmt(const DefaultStmt
&S,
EmitStmt(S.getSubStmt());
}
+namespace {
+struct EmitDeferredStatement final : EHScopeStack::Cleanup {
+ const DeferStmt &Stmt;
+ EmitDeferredStatement(const DeferStmt *Stmt) : Stmt(*Stmt) {}
+
+ void Emit(CodeGenFunction &CGF, Flags) override {
+// Take care that any cleanups pushed by the body of the 'defer' don't
+// clobber the current cleanup slot value.
+//
+// This situation warrants some explanation: Assume we have a scope that
+// pushes a cleanup; when that scope is exited, we need to run that
cleanup;
+// this is accomplished by emitting the cleanup into a separate block and
+// then branching to that block at scope exit.
+//
+// Where this gets complicated is if we exit the scope in multiple
different
+// ways; e.g. in a 'for' loop, we may exit the scope of its body by falling
+// off the end (in which case we need to run the cleanup and then branch to
+// the increment), or by 'break'ing out of the loop (in which case we need
+// to run the cleanup and then branch to the loop exit block); in both
cases
+// we first branch to the cleanup block to run the cleanup, but the block
we
+// need to jump to *after* running the cleanup is different.
+//
+// This is accomplished using a local integer variable called the 'cleanup
+// slot': before branching to the cleanup block, we store a value into that
+// slot. Then, in the cleanup block, after running the cleanup, we load the
+// value of that variable and 'switch' on it to branch to the appropriate
+// continuation block.
+//
+// The problem that arises once 'defer' statements are involved is that the
+// body of a 'defer' is an arbitrary statement which itself can create more
+// cleanups. This means we may end up overwriting the cleanup slot before
we
+// ever have a chance to 'switch' on it, which means that once we *do* get
+// to the 'switch', we end up in whatever block the cleanup code happened
to
+// pick as the default 'switch' exit label!
+//
+// That is, what is normally supposed to happen is something like:
+//
+// 1. Store 'X' to cleanup slot.
+// 2. Branch to cleanup block.
+// 3. Execute cleanup.
+// 4. Read value from cleanup slot.
+// 5. Branch to the block associated with 'X'.
+//
+// But if we encounter a 'defer' statement that contains a cleanup, then
+// what might instead happen is:
+//
+// 1. Store 'X' to cleanup slot.
+// 2. Branch to cleanup block.
+// 3. Execute cleanup; this ends up pushing another cleanup, so:
+// 3a. Store 'Y' to cleanup slot.
+// 3b. Run steps 2–5 recursively.
+// 4. Read value from cleanup slot, which is now 'Y' instead of 'X'.
+// 5. Branch to the block associated with 'Y'... which doesn't even
+// exist because the value 'Y' is only meaningful for the inner
+// cleanup. The result is we just branch 'somewhere random'.
+//
+// The rest of the cleanup code simply isn't prepared to handle this case
+// because there are no other cleanups that can push more cleanups, and
+// thus, emitting any other cleanup cannot clobber the cleanup slot.
+//
+// To prevent this from happening, simply force the allocation of a new
+// cleanup slot for the body of the defer statement by temporarily clearing
+// out any existing slot.
+SaveAndRestore PreserveCleanupSlot{CGF.NormalCleanupDest,
+ RawAddress::invalid()};
+CGF.EmitStmt(Stmt.getBody());
cor3ntin wrote:
@erichkeane can you have a look at that bit?
https://github.com/llvm/llvm-project/pull/162848
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
@@ -3232,6 +3243,46 @@ class ReturnStmt final
}
};
+/// DeferStmt - This represents a deferred statement.
+class DeferStmt : public Stmt {
+ friend class ASTStmtReader;
+
+ /// The deferred statement.
+ Stmt *Body;
+
+public:
+ DeferStmt(SourceLocation DeferLoc, Stmt *Body) : Stmt(DeferStmtClass) {
+setDeferLoc(DeferLoc);
+setBody(Body);
+ }
+
+ explicit DeferStmt(EmptyShell Empty) : Stmt(DeferStmtClass, Empty) {}
cor3ntin wrote:
I would prefer it too - that also let us have a distinct CreateEmpty
https://github.com/llvm/llvm-project/pull/162848
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
https://github.com/cor3ntin commented: Random unhinged thought: Can i put a defer statement inside a statement expression? in the initializer of a compound literal? https://github.com/llvm/llvm-project/pull/162848 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
@@ -6981,6 +6981,9 @@ void Clang::ConstructJob(Compilation &C, const JobAction
&JA,
types::isCXX(InputType))
CmdArgs.push_back("-fcoro-aligned-allocation");
+ if (Args.hasFlag(options::OPT_fdefer_ts, options::OPT_fno_defer_ts, false))
+CmdArgs.push_back("-fdefer-ts");
cor3ntin wrote:
```suggestion
if (Args.hasFlag(options::OPT_fdefer_ts, options::OPT_fno_defer_ts,
/*???=*/false))
CmdArgs.push_back("-fdefer-ts");
```
https://github.com/llvm/llvm-project/pull/162848
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
https://github.com/cor3ntin edited https://github.com/llvm/llvm-project/pull/162848 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
@@ -2000,6 +2004,80 @@ void CodeGenFunction::EmitDefaultStmt(const DefaultStmt
&S,
EmitStmt(S.getSubStmt());
}
+namespace {
+struct EmitDeferredStatement final : EHScopeStack::Cleanup {
+ const DeferStmt &Stmt;
+ EmitDeferredStatement(const DeferStmt *Stmt) : Stmt(*Stmt) {}
+
+ void Emit(CodeGenFunction &CGF, Flags) override {
+// Take care that any cleanups pushed by the body of the 'defer' don't
+// clobber the current cleanup slot value.
cor3ntin wrote:
```suggestion
// Take care that any cleanups pushed by the body of a 'defer' statement
don't
// clobber the current cleanup slot value.
```
https://github.com/llvm/llvm-project/pull/162848
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
Sirraide wrote: I’ve been busy w/ expansion statements lately, but I finally managed to track down that codegen bug: essentially, we were clobbering the cleanup slot while emitting cleanups inside the `defer`. I also added a huge comment that explains the situation because it was very much not obvious to me. https://github.com/llvm/llvm-project/pull/162848 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
@@ -0,0 +1,22 @@ +/*=== stddefer.h - Standard header for 'defer' -=== + * + * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. + * See https://llvm.org/LICENSE.txt for license information. + * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + * + *===---=== + */ + +#ifndef __CLANG_STDDEFER_H +#define __CLANG_STDDEFER_H + +/* Provide 'defer' if '_Defer' is supported and update the predefined + macro accordingly. */ +#if defined __STDC_DEFER_TS25755__ && __STDC_DEFER_TS25755__ == 1 +#undef __STDC_DEFER_TS25755__ Sirraide wrote: I suppose it should be enough to just check if it’s defined yes. > We don't want to redefine `__STDC_DEFER_TS25755__` to 2 because we're not > providing `defer` as a keyword, Ah, I thought it should be defined to `2` if `defer` has the same meaning as `_Defer`, irrespective of whether it’s a proper keyword or a macro, but I think you’re right, it should just be `1` in our case. https://github.com/llvm/llvm-project/pull/162848 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
@@ -0,0 +1,22 @@ +/*=== stddefer.h - Standard header for 'defer' -=== + * + * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. + * See https://llvm.org/LICENSE.txt for license information. + * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + * + *===---=== + */ + +#ifndef __CLANG_STDDEFER_H +#define __CLANG_STDDEFER_H + +/* Provide 'defer' if '_Defer' is supported and update the predefined + macro accordingly. */ +#if defined __STDC_DEFER_TS25755__ && __STDC_DEFER_TS25755__ == 1 +#undef __STDC_DEFER_TS25755__ AaronBallman wrote: Do we need to do this dance? I would have assumed this header would be (with a header guard): ``` #ifdef __STDC_DEFER_TS25755__ #define __STDC_VERSION_STDDEFER_H__ 202602L #define defer _Defer #endif ``` We don't want to redefine `__STDC_DEFER_TS25755__` to 2 because we're not providing `defer` as a keyword, only `_Defer`. And we don't need to worry about working with other implementations because this header is tied to Clang itself, so we don't need to worry about the value being `2` instead. https://github.com/llvm/llvm-project/pull/162848 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
Sirraide wrote: > I recommend we only support `_Defer` as a keyword and let users include > `` to get the prettier spelling. Sgtm > I think we should vend that header file ourselves instead of leaving it to a > C library Yeah, definitely. https://github.com/llvm/llvm-project/pull/162848 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
AaronBallman wrote: > We talked about this PR a bit in my office hours yesterday and one thing > which came up for feedback to WG14 is that this should be spelled `_Defer` > and not `defer` if the committee is serious about the feature (which I > believe they are). It's going into a TS, and so users have to opt-in for > `defer` to become a keyword which may break their code. That's fine and we > can live with it. But when `defer` is rolled into the IS, that makes for a > harder migration path. But more importantly, we like this feature -- we want > to expose it as a conforming extension in other language modes; that's not > possible with the keyword spelled `defer` because that'd be a non-conforming > extension. And from looking at available open source code, the identifier > `defer` is used quite frequently. Following up on this, the author of the TS has modified their draft to require `_Defer` as the keyword, provide an option for an implementation to additionally support `defer` as the keyword (the feature test macro now resolves to a value which tells you whether `defer` is supported or only `_Defer`), adds a `` header with the expected macros, and allows attributes on the defer statement. I recommend we only support `_Defer` as a keyword and let users include `` to get the prettier spelling. I think we should vend that header file ourselves instead of leaving it to a C library (that makes deployment of the feature easier; we're not having to wait for standard libraries to also choose to support the TS). https://github.com/llvm/llvm-project/pull/162848 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
llvmbot wrote:
@llvm/pr-subscribers-clang-codegen
Author: None (Sirraide)
Changes
I was talking to @AaronBallman about this, and we decided it would make
sense to open a PR for this at this point, even if we ultimately decide to
defer merging it to a later point in time.
---
Patch is 57.12 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/162848.diff
40 Files Affected:
- (modified) clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp (+6)
- (modified) clang/include/clang/AST/RecursiveASTVisitor.h (+1)
- (modified) clang/include/clang/AST/Stmt.h (+51)
- (modified) clang/include/clang/Basic/DiagnosticParseKinds.td (+3)
- (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+15)
- (modified) clang/include/clang/Basic/LangOptions.def (+1)
- (modified) clang/include/clang/Basic/StmtNodes.td (+1)
- (modified) clang/include/clang/Basic/TokenKinds.def (+4)
- (modified) clang/include/clang/Driver/Options.td (+8)
- (modified) clang/include/clang/Parse/Parser.h (+10)
- (modified) clang/include/clang/Sema/Sema.h (+8)
- (modified) clang/include/clang/Serialization/ASTBitCodes.h (+1)
- (modified) clang/lib/AST/StmtPrinter.cpp (+5)
- (modified) clang/lib/AST/StmtProfile.cpp (+2)
- (modified) clang/lib/Basic/IdentifierTable.cpp (+4-1)
- (modified) clang/lib/CodeGen/CGStmt.cpp (+19)
- (modified) clang/lib/CodeGen/CodeGenFunction.h (+1)
- (modified) clang/lib/Driver/ToolChains/Clang.cpp (+3)
- (modified) clang/lib/Frontend/InitPreprocessor.cpp (+3)
- (modified) clang/lib/Parse/ParseStmt.cpp (+32)
- (modified) clang/lib/Sema/JumpDiagnostics.cpp (+26-1)
- (modified) clang/lib/Sema/SemaExceptionSpec.cpp (+1)
- (modified) clang/lib/Sema/SemaExpr.cpp (+29)
- (modified) clang/lib/Sema/SemaStmt.cpp (+38-6)
- (modified) clang/lib/Sema/TreeTransform.h (+8)
- (modified) clang/lib/Serialization/ASTReaderStmt.cpp (+10)
- (modified) clang/lib/Serialization/ASTWriterStmt.cpp (+7)
- (modified) clang/lib/StaticAnalyzer/Core/ExprEngine.cpp (+1)
- (added) clang/test/AST/ast-dump-defer-ts.c (+27)
- (added) clang/test/AST/ast-print-defer-ts.c (+33)
- (added) clang/test/CodeGen/defer-ts-seh.c (+44)
- (added) clang/test/CodeGen/defer-ts.c (+427)
- (added) clang/test/Lexer/defer-keyword.cpp (+5)
- (added) clang/test/Parser/defer-ts.c (+41)
- (added) clang/test/Parser/defer-ts.cpp (+9)
- (added) clang/test/Preprocessor/defer-ts.c (+4)
- (added) clang/test/Sema/defer-ts-seh.c (+17)
- (added) clang/test/Sema/defer-ts-sjlj.c (+52)
- (added) clang/test/Sema/defer-ts.c (+157)
- (modified) clang/tools/libclang/CXCursor.cpp (+5)
``diff
diff --git a/clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
index 07bb08166a006..f1b4682c397ab 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
@@ -241,6 +241,12 @@ static bool isIdenticalStmt(const ASTContext &Ctx, const
Stmt *Stmt1,
return false;
return true;
}
+ case Stmt::DeferStmtClass: {
+const auto *DefStmt1 = cast(Stmt1);
+const auto *DefStmt2 = cast(Stmt2);
+return isIdenticalStmt(Ctx, DefStmt1->getBody(), DefStmt2->getBody(),
+ IgnoreSideEffects);
+ }
case Stmt::CompoundStmtClass: {
const auto *CompStmt1 = cast(Stmt1);
const auto *CompStmt2 = cast(Stmt2);
diff --git a/clang/include/clang/AST/RecursiveASTVisitor.h
b/clang/include/clang/AST/RecursiveASTVisitor.h
index 1d1b7f183f75a..a7a89e8338af5 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2560,6 +2560,7 @@ DEF_TRAVERSE_STMT(DefaultStmt, {})
DEF_TRAVERSE_STMT(DoStmt, {})
DEF_TRAVERSE_STMT(ForStmt, {})
DEF_TRAVERSE_STMT(GotoStmt, {})
+DEF_TRAVERSE_STMT(DeferStmt, {})
DEF_TRAVERSE_STMT(IfStmt, {})
DEF_TRAVERSE_STMT(IndirectGotoStmt, {})
DEF_TRAVERSE_STMT(LabelStmt, {})
diff --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h
index 76942f1a84f9a..219a99bee8432 100644
--- a/clang/include/clang/AST/Stmt.h
+++ b/clang/include/clang/AST/Stmt.h
@@ -317,6 +317,16 @@ class alignas(void *) Stmt {
SourceLocation KeywordLoc;
};
+ class DeferStmtBitfields {
+friend class DeferStmt;
+
+LLVM_PREFERRED_TYPE(StmtBitfields)
+unsigned : NumStmtBits;
+
+/// The location of the "defer".
+SourceLocation DeferLoc;
+ };
+
//===--- Expression bitfields classes ---===//
class ExprBitfields {
@@ -1318,6 +1328,7 @@ class alignas(void *) Stmt {
LoopControlStmtBitfields LoopControlStmtBits;
ReturnStmtBitfields ReturnStmtBits;
SwitchCaseBitfields SwitchCaseBits;
+DeferStmtBitfields DeferStmtBits;
// Expressions
ExprBitfields ExprBits;
@@ -3232,6 +3243,46 @@ class ReturnStmt final
}
};
+/// DeferStmt - This represents a deferred statement.
+class DeferStmt : public Stmt {
+ fri
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
@@ -2376,6 +2381,33 @@ StmtResult Parser::ParseReturnStatement() {
return Actions.ActOnReturnStmt(ReturnLoc, R.get(), getCurScope());
}
+StmtResult Parser::ParseDeferStatement(SourceLocation *TrailingElseLoc) {
+ assert(Tok.is(tok::kw_defer));
+ SourceLocation DeferLoc = ConsumeToken();
+ Actions.ActOnStartOfDeferStmt(DeferLoc, getCurScope());
+ auto OnError = llvm::make_scope_exit(
+ [&] { Actions.ActOnDeferStmtError(getCurScope()); });
+
+ StmtResult Res = ParseStatement(TrailingElseLoc);
+ if (!Res.isUsable())
+return StmtError();
+
+ // Diagnose this *after* parsing the body for better synchronisation.
+ if (getLangOpts().CPlusPlus) {
+Diag(DeferLoc, diag::err_defer_unsupported);
+return StmtError();
+ }
cor3ntin wrote:
Maybe the alternative is to not parse the driver option at all in c++ mode
(`ShouldParseIf`)
And in the front end we can just assert on it
https://github.com/llvm/llvm-project/pull/162848
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
https://github.com/Sirraide created
https://github.com/llvm/llvm-project/pull/162848
I was talking to @AaronBallman about this, and we decided it would make sense
to open a PR for this at this point, even if we ultimately decide to defer
merging it to a later point in time.
>From a2f523c46338d53bc2c9efb522ddfc8309d44212 Mon Sep 17 00:00:00 2001
From: Sirraide
Date: Thu, 7 Aug 2025 01:48:41 +0200
Subject: [PATCH 01/20] [Clang] Add support for the C 'defer' TS
---
.../clang-tidy/bugprone/BranchCloneCheck.cpp | 6 +++
clang/include/clang/AST/RecursiveASTVisitor.h | 1 +
clang/include/clang/AST/Stmt.h| 51 +++
.../clang/Basic/DiagnosticParseKinds.td | 3 ++
.../clang/Basic/DiagnosticSemaKinds.td| 4 ++
clang/include/clang/Basic/LangOptions.def | 1 +
clang/include/clang/Basic/StmtNodes.td| 1 +
clang/include/clang/Basic/TokenKinds.def | 4 ++
clang/include/clang/Driver/Options.td | 8 +++
clang/include/clang/Parse/Parser.h| 10
clang/include/clang/Sema/Sema.h | 2 +
.../include/clang/Serialization/ASTBitCodes.h | 1 +
clang/lib/AST/StmtPrinter.cpp | 7 +++
clang/lib/AST/StmtProfile.cpp | 2 +
clang/lib/Basic/IdentifierTable.cpp | 5 +-
clang/lib/CodeGen/CGStmt.cpp | 1 +
clang/lib/Driver/ToolChains/Clang.cpp | 3 ++
clang/lib/Parse/ParseStmt.cpp | 27 ++
clang/lib/Sema/SemaExceptionSpec.cpp | 1 +
clang/lib/Sema/SemaStmt.cpp | 6 +++
clang/lib/Sema/TreeTransform.h| 8 +++
clang/lib/Serialization/ASTReaderStmt.cpp | 10
clang/lib/Serialization/ASTWriterStmt.cpp | 7 +++
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 1 +
clang/test/Lexer/defer-keyword.cpp| 5 ++
clang/test/Parser/defer-ts.c | 41 +++
clang/test/Parser/defer-ts.cpp| 9
clang/tools/libclang/CXCursor.cpp | 5 ++
28 files changed, 229 insertions(+), 1 deletion(-)
create mode 100644 clang/test/Lexer/defer-keyword.cpp
create mode 100644 clang/test/Parser/defer-ts.c
create mode 100644 clang/test/Parser/defer-ts.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
index a6cd68edda55e..80b1006a70b0f 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
@@ -241,6 +241,12 @@ static bool isIdenticalStmt(const ASTContext &Ctx, const
Stmt *Stmt1,
return false;
return true;
}
+ case Stmt::DeferStmtClass: {
+const auto *DefStmt1 = cast(Stmt1);
+const auto *DefStmt2 = cast(Stmt2);
+return isIdenticalStmt(Ctx, DefStmt1->getBody(), DefStmt2->getBody(),
+ IgnoreSideEffects);
+ }
case Stmt::CompoundStmtClass: {
const auto *CompStmt1 = cast(Stmt1);
const auto *CompStmt2 = cast(Stmt2);
diff --git a/clang/include/clang/AST/RecursiveASTVisitor.h
b/clang/include/clang/AST/RecursiveASTVisitor.h
index 62991d986e675..0527e5cbd7875 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2476,6 +2476,7 @@ DEF_TRAVERSE_STMT(DefaultStmt, {})
DEF_TRAVERSE_STMT(DoStmt, {})
DEF_TRAVERSE_STMT(ForStmt, {})
DEF_TRAVERSE_STMT(GotoStmt, {})
+DEF_TRAVERSE_STMT(DeferStmt, {})
DEF_TRAVERSE_STMT(IfStmt, {})
DEF_TRAVERSE_STMT(IndirectGotoStmt, {})
DEF_TRAVERSE_STMT(LabelStmt, {})
diff --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h
index a5b0d5053003f..22daceb77d490 100644
--- a/clang/include/clang/AST/Stmt.h
+++ b/clang/include/clang/AST/Stmt.h
@@ -327,6 +327,16 @@ class alignas(void *) Stmt {
SourceLocation KeywordLoc;
};
+ class DeferStmtBitfields {
+friend class DeferStmt;
+
+LLVM_PREFERRED_TYPE(StmtBitfields)
+unsigned : NumStmtBits;
+
+/// The location of the "defer".
+SourceLocation DeferLoc;
+ };
+
//===--- Expression bitfields classes ---===//
class ExprBitfields {
@@ -1329,6 +1339,7 @@ class alignas(void *) Stmt {
BreakStmtBitfields BreakStmtBits;
ReturnStmtBitfields ReturnStmtBits;
SwitchCaseBitfields SwitchCaseBits;
+DeferStmtBitfields DeferStmtBits;
// Expressions
ExprBitfields ExprBits;
@@ -3201,6 +3212,46 @@ class ReturnStmt final
}
};
+/// DeferStmt - This represents a deferred statement.
+class DeferStmt : public Stmt {
+ friend class ASTStmtReader;
+
+ /// The deferred statement.
+ Stmt *Body;
+
+public:
+ DeferStmt(SourceLocation DeferLoc, Stmt *Body) : Stmt(DeferStmtClass) {
+setDeferLoc(DeferLoc);
+setBody(Body);
+ }
+
+ explicit DeferStmt(EmptyShell Empty) : Stmt(DeferStmtClass, Empty) {}
+
+ SourceLocation getDeferLoc() const { return DeferStmtBits.DeferLoc; }
+ void setDeferLoc(SourceLocation DeferL
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
AaronBallman wrote: We talked about this PR a bit in my office hours yesterday and one thing which came up for feedback to WG14 is that this should be spelled `_Defer` and not `defer` if the committee is serious about the feature (which I believe they are). It's going into a TS, and so users have to opt-in for `defer` to become a keyword which may break their code. That's fine and we can live with it. But when `defer` is rolled into the IS, that makes for a harder migration path. But more importantly, we like this feature -- we want to expose it as a conforming extension in other language modes; that's not possible with the keyword spelled `defer` because that'd be a non-conforming extension. And from looking at available open source code, the identifier `defer` is used quite frequently. https://github.com/llvm/llvm-project/pull/162848 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [Clang] Add support for the C `defer` TS (PR #162848)
@@ -2376,6 +2381,33 @@ StmtResult Parser::ParseReturnStatement() {
return Actions.ActOnReturnStmt(ReturnLoc, R.get(), getCurScope());
}
+StmtResult Parser::ParseDeferStatement(SourceLocation *TrailingElseLoc) {
+ assert(Tok.is(tok::kw_defer));
+ SourceLocation DeferLoc = ConsumeToken();
+ Actions.ActOnStartOfDeferStmt(DeferLoc, getCurScope());
+ auto OnError = llvm::make_scope_exit(
+ [&] { Actions.ActOnDeferStmtError(getCurScope()); });
+
+ StmtResult Res = ParseStatement(TrailingElseLoc);
+ if (!Res.isUsable())
+return StmtError();
+
+ // Diagnose this *after* parsing the body for better synchronisation.
+ if (getLangOpts().CPlusPlus) {
+Diag(DeferLoc, diag::err_defer_unsupported);
+return StmtError();
+ }
Sirraide wrote:
> Maybe the alternative is to not parse the driver option at all in c++ mode
> (`ShouldParseIf`)
I didn’t know that was a thing, but that looks like the proper way to do this.
https://github.com/llvm/llvm-project/pull/162848
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
