[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
