public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug c++/107571] New: Missing fallthrough attribute diagnostics
@ 2022-11-08 14:49 jakub at gcc dot gnu.org
2022-11-08 22:27 ` [Bug c++/107571] " jakub at gcc dot gnu.org
` (5 more replies)
0 siblings, 6 replies; 7+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-11-08 14:49 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107571
Bug ID: 107571
Summary: Missing fallthrough attribute diagnostics
Product: gcc
Version: 13.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c++
Assignee: unassigned at gcc dot gnu.org
Reporter: jakub at gcc dot gnu.org
Target Milestone: ---
On:
void
foo (int n)
{
void g(), h(), i();
switch (n)
{
case 1:
case 2:
g();
[[fallthrough]];
case 3: // warning on fallthrough discouraged
do {
[[fallthrough]]; // error: next statement is not part of the same
substatement execution
} while (false);
case 6:
do {
[[fallthrough]]; // error: next statement is not part of the same
substatement execution
} while (n--);
case 7:
while (false) {
[[fallthrough]]; // error: next statement is not part of the same
substatement execution
}
case 5:
h();
case 4: // implementation may warn on fallthrough
i();
[[fallthrough]]; // error
}
}
mentioned in https://isocpp.org/files/papers/D2552R1.pdf we don't diagnose
misplaced [[fallthrough]] in 2 spots.
The original dump shows:
switch (n)
{
case 1:;
case 2:;
<<cleanup_point <<< Unknown tree: expr_stmt
g () >>>>>;
<<cleanup_point <<< Unknown tree: expr_stmt
.FALLTHROUGH () >>>>>;
case 3:;
<<cleanup_point <<< Unknown tree: expr_stmt
.FALLTHROUGH () >>>>>;
case 6:;
<D.2778>:;
<<cleanup_point <<< Unknown tree: expr_stmt
.FALLTHROUGH () >>>>>;
if (<<cleanup_point n-- != 0>>) goto <D.2778>; else goto <D.2776>;
<D.2776>:;
case 7:;
goto <D.2779>;
<<cleanup_point <<< Unknown tree: expr_stmt
.FALLTHROUGH () >>>>>;
<D.2779>:;
case 5:;
<<cleanup_point <<< Unknown tree: expr_stmt
h () >>>>>;
case 4:;
<<cleanup_point <<< Unknown tree: expr_stmt
i () >>>>>;
<<cleanup_point <<< Unknown tree: expr_stmt
.FALLTHROUGH () >>>>>;
}
so the reason we don't warn in the do { ... } while (false); case is that it
disappears probably during
genericize_c_loop and the while (false) case because the genericization in that
case makes the loop body followed by artificial label.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug c++/107571] Missing fallthrough attribute diagnostics
2022-11-08 14:49 [Bug c++/107571] New: Missing fallthrough attribute diagnostics jakub at gcc dot gnu.org
@ 2022-11-08 22:27 ` jakub at gcc dot gnu.org
2022-11-08 22:28 ` mpolacek at gcc dot gnu.org
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-11-08 22:27 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107571
--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Apparently this is
https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#2406
which we probably never implemented.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug c++/107571] Missing fallthrough attribute diagnostics
2022-11-08 14:49 [Bug c++/107571] New: Missing fallthrough attribute diagnostics jakub at gcc dot gnu.org
2022-11-08 22:27 ` [Bug c++/107571] " jakub at gcc dot gnu.org
@ 2022-11-08 22:28 ` mpolacek at gcc dot gnu.org
2022-11-08 22:40 ` jakub at gcc dot gnu.org
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: mpolacek at gcc dot gnu.org @ 2022-11-08 22:28 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107571
Marek Polacek <mpolacek at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Ever confirmed|0 |1
Last reconfirmed| |2022-11-08
Status|UNCONFIRMED |NEW
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug c++/107571] Missing fallthrough attribute diagnostics
2022-11-08 14:49 [Bug c++/107571] New: Missing fallthrough attribute diagnostics jakub at gcc dot gnu.org
2022-11-08 22:27 ` [Bug c++/107571] " jakub at gcc dot gnu.org
2022-11-08 22:28 ` mpolacek at gcc dot gnu.org
@ 2022-11-08 22:40 ` jakub at gcc dot gnu.org
2022-11-09 0:36 ` joseph at codesourcery dot com
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-11-08 22:40 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107571
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |jsm28 at gcc dot gnu.org
--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
And looking at the C wording in n2596.pdf, seems it is different again:
"The next block item(6.8.2) that would be encountered after a fallthrough
declaration shall be a case label or default label associated with the smallest
enclosing switch statement."
So, if my understanding is well,
int j = 0;
switch (n)
{
case 1:
for (int i = 0; i < 1; ++i)
{
[[fallthrough]]; // Invalid in both C and C++
}
case 2:
while (++j < 2)
{
[[fallthrough]]; // Invalid in both C and C++
}
case 3:
do
{
[[fallthrough]]; // Invalid in both C and C++
}
while (0);
case 4:
if (1)
{
[[fallthrough]]; // Invalid in C, valid in C++?
}
case 5:
for (int i = 0; i < 1; ++i)
[[fallthrough]]; // Invalid in C++, dunno about C
case 6:
while (++j < 2)
[[fallthrough]]; // Invalid in C++, dunno about C
case 7:
do
[[fallthrough]]; // Invalid in C++, dunno about C
while (0);
case 8:
if (1)
[[fallthrough]]; // Dunno about either C or C++
case 9:
{
[[fallthrough]]; // Invalid in C, valid in C++?
}
default:
break;
}
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug c++/107571] Missing fallthrough attribute diagnostics
2022-11-08 14:49 [Bug c++/107571] New: Missing fallthrough attribute diagnostics jakub at gcc dot gnu.org
` (2 preceding siblings ...)
2022-11-08 22:40 ` jakub at gcc dot gnu.org
@ 2022-11-09 0:36 ` joseph at codesourcery dot com
2023-11-17 14:51 ` cvs-commit at gcc dot gnu.org
2023-11-17 14:55 ` jakub at gcc dot gnu.org
5 siblings, 0 replies; 7+ messages in thread
From: joseph at codesourcery dot com @ 2022-11-09 0:36 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107571
--- Comment #3 from joseph at codesourcery dot com <joseph at codesourcery dot com> ---
On Tue, 8 Nov 2022, jakub at gcc dot gnu.org via Gcc-bugs wrote:
> And looking at the C wording in n2596.pdf, seems it is different again:
That's a very old version. N3054 is the most recent public draft (SC22
N5777 is more recent than that and is the actual CD ballot text).
> "The next block item(6.8.2) that would be encountered after a fallthrough
> declaration shall be a case label or default label associated with the smallest
> enclosing switch statement."
It's not exactly clear what "next block item" is for any of the examples
you give - next lexically (OK once the current one is exited) or in
execution (no good for a Constraint)? And thus not clear that any of
these are invalid. I've noted that the inconsistency with C++ should be
raised in an NB comment.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug c++/107571] Missing fallthrough attribute diagnostics
2022-11-08 14:49 [Bug c++/107571] New: Missing fallthrough attribute diagnostics jakub at gcc dot gnu.org
` (3 preceding siblings ...)
2022-11-09 0:36 ` joseph at codesourcery dot com
@ 2023-11-17 14:51 ` cvs-commit at gcc dot gnu.org
2023-11-17 14:55 ` jakub at gcc dot gnu.org
5 siblings, 0 replies; 7+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-11-17 14:51 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107571
--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:
https://gcc.gnu.org/g:52eedfa00960f2d255ec542626e3531a65aa8bb8
commit r14-5561-g52eedfa00960f2d255ec542626e3531a65aa8bb8
Author: Jakub Jelinek <jakub@redhat.com>
Date: Fri Nov 17 15:43:31 2023 +0100
c++: Implement C++ DR 2406 - [[fallthrough]] attribute and iteration
statements
The following patch implements
CWG 2406 - [[fallthrough]] attribute and iteration statements
The genericization of some loops leaves nothing at all or just a label
after a body of a loop, so if the loop is later followed by
case or default label in a switch, the fallthrough statement isn't
diagnosed.
The following patch implements it by marking the IFN_FALLTHROUGH call
in such a case, such that during gimplification it can be pedantically
diagnosed even if it is followed by case or default label or some normal
labels followed by case/default labels.
While looking into this, I've discovered other problems.
expand_FALLTHROUGH_r is removing the IFN_FALLTHROUGH calls from the IL,
but wasn't telling that to walk_gimple_stmt/walk_gimple_seq_mod, so
the callers would then skip the next statement after it, and it would
return non-NULL if the removed stmt was last in the sequence. This could
lead to wi->callback_result being set even if it didn't appear at the very
end of switch sequence.
The patch makes use of wi->removed_stmt such that the callers properly
know what happened, and use different way to handle the end of switch
sequence case.
That change discovered a bug in the gimple-walk handling of
wi->removed_stmt. If that flag is set, the callback is telling the callers
that the current statement has been removed and so the innermost
walk_gimple_seq_mod shouldn't gsi_next. The problem is that
wi->removed_stmt is only reset at the start of a walk_gimple_stmt, but that
can be too late for some cases. If we have two nested gimple sequences,
say GIMPLE_BIND as the last stmt of some gimple seq, we remove the last
statement inside of that GIMPLE_BIND, set wi->removed_stmt there, don't
do gsi_next correctly because already gsi_remove moved us to the next stmt,
there is no next stmt, so we return back to the caller, but
wi->removed_stmt
is still set and so we don't do gsi_next even in the outer sequence,
despite
the GIMPLE_BIND (etc.) not being removed. That means we walk the
GIMPLE_BIND with its whole sequence again.
The patch fixes that by resetting wi->removed_stmt after we've used that
flag in walk_gimple_seq_mod. Nothing really uses that flag after the
outermost walk_gimple_seq_mod, it is just a private notification that
the stmt callback has removed a stmt.
2023-11-17 Jakub Jelinek <jakub@redhat.com>
PR c++/107571
gcc/
* gimplify.cc (expand_FALLTHROUGH_r): Use wi->removed_stmt after
gsi_remove, change the way of passing fallthrough stmt at the end
of sequence to expand_FALLTHROUGH. Diagnose IFN_FALLTHROUGH
with GF_CALL_NOTHROW flag.
(expand_FALLTHROUGH): Change loc into array of 2 location_t elts,
don't test wi.callback_result, instead check whether first
elt is not UNKNOWN_LOCATION and in that case pedwarn with the
second location.
* gimple-walk.cc (walk_gimple_seq_mod): Clear wi->removed_stmt
after the flag has been used.
* internal-fn.def (FALLTHROUGH): Mention in comment the special
meaning of the TREE_NOTHROW/GF_CALL_NOTHROW flag on the calls.
gcc/c-family/
* c-gimplify.cc (genericize_c_loop): For C++ mark IFN_FALLTHROUGH
call at the end of loop body as TREE_NOTHROW.
gcc/testsuite/
* g++.dg/DRs/dr2406.C: New test.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug c++/107571] Missing fallthrough attribute diagnostics
2022-11-08 14:49 [Bug c++/107571] New: Missing fallthrough attribute diagnostics jakub at gcc dot gnu.org
` (4 preceding siblings ...)
2023-11-17 14:51 ` cvs-commit at gcc dot gnu.org
@ 2023-11-17 14:55 ` jakub at gcc dot gnu.org
5 siblings, 0 replies; 7+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-11-17 14:55 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107571
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |FIXED
Status|NEW |RESOLVED
--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Should be implemented now.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-11-17 14:55 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-08 14:49 [Bug c++/107571] New: Missing fallthrough attribute diagnostics jakub at gcc dot gnu.org
2022-11-08 22:27 ` [Bug c++/107571] " jakub at gcc dot gnu.org
2022-11-08 22:28 ` mpolacek at gcc dot gnu.org
2022-11-08 22:40 ` jakub at gcc dot gnu.org
2022-11-09 0:36 ` joseph at codesourcery dot com
2023-11-17 14:51 ` cvs-commit at gcc dot gnu.org
2023-11-17 14:55 ` jakub at gcc dot gnu.org
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).