On Thu, 2016-08-18 at 15:50 +0200, Marek Polacek wrote:
> Now that all various switch fallthrough bugfixes and adjustments were
> committed, and this patch has shrunk considerably, I'm presenting the
> latest
> version. The changes from the last version are not huge; we don't
> warn for a
> fall through to a user-defined label anymore, and I made some tiny
> changes
> regarding parsing attributes in the C FE, as requested by Joseph.
>
> This patch is accompanied by another patch that merely adds various
> gcc_fallthroughs, in places where a FALLTHRU comment wouldn't work.
>
> It's been tested on powerpc64le-unknown-linux-gnu, aarch64-linux-gnu,
> and x86_64-redhat-linux.
>
> 2016-08-18 Marek Polacek <pola...@redhat.com>
> Jakub Jelinek <ja...@redhat.com>
>
> PR c/7652
[...]
> diff --git gcc/gcc/gimplify.c gcc/gcc/gimplify.c
> index 1e43dbb..1925263 100644
> --- gcc/gcc/gimplify.c
> +++ gcc/gcc/gimplify.c
[...]
> + if (warned_p)
> + {
> + rich_location richloc (line_table, gimple_location
> (next));
> + richloc.add_fixit_insert (gimple_location (next),
> + "insert '__attribute__ "
> + "((fallthrough));' to
> silence "
> + "this warning");
> + richloc.add_fixit_insert (gimple_location (next),
> + "insert 'break;' to avoid
> "
> + "fall-through");
> + inform_at_rich_loc (&richloc, "here");
> + }
This isn't quite the way that fix-its are meant to be used, in my mind,
at least: the insertion text is supposed to be something that could be
literally inserted into the code (e.g. by an IDE) i.e. it should be a
code fragment, rather than a message to the user.
Here's an idea of how the above could look:
if (warned_p)
{
/* Suggestion one: add "__attribute__ ((fallthrough));". */
rich_location richloc_attr (line_table, gimple_location (next));
richloc_attr.add_fixit_insert (gimple_location (next),
"__attribute__ ((fallthrough));");
inform_at_rich_loc (&richloc_attr, "insert %qs to silence this warning",
"__attribute__ ((fallthrough));")
/* Suggestion two: add "break;". */
rich_location richloc_break (line_table, gimple_location (next));
richloc_break.add_fixit_insert (gimple_location (next),
"break;");
inform_at_rich_loc (&richloc_break, "insert %qs to avoid fall-through",
"break;");
}
(and using %qs for quoting the language elements in the messages
themselves).
There doesn't seem to be any test coverage in the patch for the fix-it
hints.
The easiest way to do this is to create a test case with:
/* { dg-options "-fdiagnostics-show-caret" } */
and then add:
/* { dg-begin-multiline-output "" }
QUOTE OF EXPECTED CARET+FIXIT OUTPUT, omitting any trailing deja-gnu
directives
{ dg-end-multiline-output "" } */
so that we can directly verify that the results look sane.
BTW, is there some way to break up warn_implicit_fallthrough_r, maybe
moving the GIMPLE_LABEL handling to a subroutine? (and perhaps the
suggestion-handling above could live in its own subroutine, etc).
[...]
Hope this is constructive
Dave
