* [PATCH] C: detect more missing semicolons (PR c/7356)
@ 2017-10-11 19:39 David Malcolm
2017-10-25 16:05 ` Jeff Law
0 siblings, 1 reply; 3+ messages in thread
From: David Malcolm @ 2017-10-11 19:39 UTC (permalink / raw)
To: gcc-patches; +Cc: David Malcolm
[This patch assumes the two patches here:
https://gcc.gnu.org/ml/gcc-patches/2017-10/msg00274.html ]
c_parser_declaration_or_fndef has logic for parsing what might be
either a declaration or a function definition.
This patch adds a test to detect cases where a semicolon would have
terminated the decls as a declaration, where the token that follows
would start a new declaration specifier, and updates the error message
accordingly, with a fix-it hint.
This addresses PR c/7356, fixing the case of a stray token before a
#include which previously gave inscrutable output, and improving e.g.:
int i
int j;
from:
t.c:2:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'int'
int j;
^~~
to:
t.c:1:6: error: expected ';' before 'int'
int i
^
;
int j;
~~~
The patch also adds a test for PR c/44515 as a baseline.
gcc.dg/noncompile/920923-1.c needs a slight update, as the output for
the first line changes from:
920923-1.c:2:14: error: expected '=', ',', ';', 'asm' or
'__attribute__' before 'unsigned'
typedef BYTE unsigned char; /* { dg-error "expected" } */
^~~~~~~~
to:
920923-1.c:2:13: error: expected ';' before 'unsigned'
typedef BYTE unsigned char; /* { dg-error "expected" } */
^~~~~~~~~
;
920923-1.c:2:1: warning: useless type name in empty declaration
typedef BYTE unsigned char; /* { dg-error "expected" } */
^~~~~~~
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
OK for trunk?
Thanks
Dave
gcc/c/ChangeLog:
PR c/7356
* c-parser.c (c_parser_declaration_or_fndef): Detect missing
semicolons.
gcc/testsuite/ChangeLog:
PR c/7356
PR c/44515
* c-c++-common/pr44515.c: New test case.
* gcc.dg/pr7356-2.c: New test case.
* gcc.dg/pr7356.c: New test case.
* gcc.dg/spellcheck-typenames.c: Update the "singed" char "TODO"
case to reflect changes to output.
* gcc.dg/noncompile/920923-1.c: Add dg-warning to reflect changes
to output.
---
gcc/c/c-parser.c | 36 +++++++++++++++++++++++++----
gcc/testsuite/c-c++-common/pr44515.c | 14 +++++++++++
gcc/testsuite/gcc.dg/noncompile/920923-1.c | 1 +
gcc/testsuite/gcc.dg/pr7356-2.c | 33 ++++++++++++++++++++++++++
gcc/testsuite/gcc.dg/pr7356.c | 17 ++++++++++++++
gcc/testsuite/gcc.dg/spellcheck-typenames.c | 5 ++--
6 files changed, 99 insertions(+), 7 deletions(-)
create mode 100644 gcc/testsuite/c-c++-common/pr44515.c
create mode 100644 gcc/testsuite/gcc.dg/pr7356-2.c
create mode 100644 gcc/testsuite/gcc.dg/pr7356.c
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index a5e3ec4..7c3b834 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -2241,11 +2241,37 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok,
}
if (!start_function (specs, declarator, all_prefix_attrs))
{
- /* This can appear in many cases looking nothing like a
- function definition, so we don't give a more specific
- error suggesting there was one. */
- c_parser_error (parser, "expected %<=%>, %<,%>, %<;%>, %<asm%> "
- "or %<__attribute__%>");
+ /* At this point we've consumed:
+ declaration-specifiers declarator
+ and the next token isn't CPP_EQ, CPP_COMMA, CPP_SEMICOLON,
+ RID_ASM, RID_ATTRIBUTE, or RID_IN,
+ but the
+ declaration-specifiers declarator
+ aren't grokkable as a function definition, so we have
+ an error. */
+ gcc_assert (!c_parser_next_token_is (parser, CPP_SEMICOLON));
+ if (c_parser_next_token_starts_declspecs (parser))
+ {
+ /* If we have
+ declaration-specifiers declarator decl-specs
+ then assume we have a missing semicolon, which would
+ give us:
+ declaration-specifiers declarator decl-specs
+ ^
+ ;
+ <~~~~~~~~~ declaration ~~~~~~~~~~>
+ Use c_parser_require to get an error with a fix-it hint. */
+ c_parser_require (parser, CPP_SEMICOLON, "expected %<;%>");
+ parser->error = false;
+ }
+ else
+ {
+ /* This can appear in many cases looking nothing like a
+ function definition, so we don't give a more specific
+ error suggesting there was one. */
+ c_parser_error (parser, "expected %<=%>, %<,%>, %<;%>, %<asm%> "
+ "or %<__attribute__%>");
+ }
if (nested)
c_pop_function_context ();
break;
diff --git a/gcc/testsuite/c-c++-common/pr44515.c b/gcc/testsuite/c-c++-common/pr44515.c
new file mode 100644
index 0000000..dbb77509
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr44515.c
@@ -0,0 +1,14 @@
+/* { dg-options "-fdiagnostics-show-caret" } */
+
+void bar(void);
+void foo(void)
+{
+ bar() /* { dg-error "expected ';' before '.' token" } */
+}
+/* { dg-begin-multiline-output "" }
+ bar()
+ ^
+ ;
+ }
+ ~
+ { dg-end-multiline-output "" } */
diff --git a/gcc/testsuite/gcc.dg/noncompile/920923-1.c b/gcc/testsuite/gcc.dg/noncompile/920923-1.c
index 1cb140e..006a071 100644
--- a/gcc/testsuite/gcc.dg/noncompile/920923-1.c
+++ b/gcc/testsuite/gcc.dg/noncompile/920923-1.c
@@ -1,5 +1,6 @@
/* { dg-message "undeclared identifier is reported only once" "reminder for mmu_base" { target *-*-* } 0 } */
typedef BYTE unsigned char; /* { dg-error "expected" } */
+/* { dg-warning "useless type name in empty declaration" "" { target *-*-* } .-1 } */
typedef int item_n;
typedef int perm_set;
struct PENT { caddr_t v_addr; };/* { dg-error "unknown type name" } */
diff --git a/gcc/testsuite/gcc.dg/pr7356-2.c b/gcc/testsuite/gcc.dg/pr7356-2.c
new file mode 100644
index 0000000..ad67975
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr7356-2.c
@@ -0,0 +1,33 @@
+/* { dg-options "-fdiagnostics-show-caret" } */
+
+int i /* { dg-error "6: expected ';' before 'int'" } */
+int j;
+/* { dg-begin-multiline-output "" }
+ int i
+ ^
+ ;
+ int j;
+ ~~~
+ { dg-end-multiline-output "" } */
+
+
+void test (void)
+{
+ int i /* { dg-error "8: expected ';' before 'int'" } */
+ int j;
+
+ /* { dg-begin-multiline-output "" }
+ int i
+ ^
+ ;
+ int j;
+ ~~~
+ { dg-end-multiline-output "" } */
+}
+
+int old_style_params (first, second)
+ int first;
+ int second;
+{
+ return first + second;
+}
diff --git a/gcc/testsuite/gcc.dg/pr7356.c b/gcc/testsuite/gcc.dg/pr7356.c
new file mode 100644
index 0000000..84baf07
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr7356.c
@@ -0,0 +1,17 @@
+/* { dg-options "-fdiagnostics-show-caret" } */
+
+a /* { dg-line stray_token } */
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+int main(int argc, char** argv)
+{
+ return 0;
+}
+
+/* { dg-error "expected ';' before '.*'" "" { target *-*-* } stray_token } */
+/* { dg-begin-multiline-output "" }
+ a
+ ^
+ ;
+ { dg-end-multiline-output "" } */
diff --git a/gcc/testsuite/gcc.dg/spellcheck-typenames.c b/gcc/testsuite/gcc.dg/spellcheck-typenames.c
index f3b8102..3717ad8 100644
--- a/gcc/testsuite/gcc.dg/spellcheck-typenames.c
+++ b/gcc/testsuite/gcc.dg/spellcheck-typenames.c
@@ -100,8 +100,9 @@ baz value; /* { dg-error "1: unknown type name .baz.; use .enum. keyword to refe
{ dg-end-multiline-output "" } */
/* TODO: it would be better to detect the "singed" vs "signed" typo here. */
-singed char ch; /* { dg-error "8: before .char." } */
+singed char ch; /* { dg-error "7: before .char." } */
/* { dg-begin-multiline-output "" }
singed char ch;
- ^~~~
+ ^~~~~
+ ;
{ dg-end-multiline-output "" } */
--
1.8.5.3
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] C: detect more missing semicolons (PR c/7356)
2017-10-11 19:39 [PATCH] C: detect more missing semicolons (PR c/7356) David Malcolm
@ 2017-10-25 16:05 ` Jeff Law
2017-10-26 0:26 ` David Malcolm
0 siblings, 1 reply; 3+ messages in thread
From: Jeff Law @ 2017-10-25 16:05 UTC (permalink / raw)
To: David Malcolm, gcc-patches
On 10/11/2017 01:32 PM, David Malcolm wrote:
> [This patch assumes the two patches here:
> https://gcc.gnu.org/ml/gcc-patches/2017-10/msg00274.html ]
I see the patch directly referenced in that message on the trunk. But
I'm not sure what you mean by "two patches". If there's a prereq that
hasn't been approved, let me know.
>
> c_parser_declaration_or_fndef has logic for parsing what might be
> either a declaration or a function definition.
>
> This patch adds a test to detect cases where a semicolon would have
> terminated the decls as a declaration, where the token that follows
> would start a new declaration specifier, and updates the error message
> accordingly, with a fix-it hint.
>
> This addresses PR c/7356, fixing the case of a stray token before a
> #include which previously gave inscrutable output, and improving e.g.:
>
> int i
> int j;
>
> from:
>
> t.c:2:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'int'
> int j;
> ^~~
>
> to:
>
> t.c:1:6: error: expected ';' before 'int'
> int i
> ^
> ;
> int j;
> ~~~
>
> The patch also adds a test for PR c/44515 as a baseline.
Personally I find the current error easier to digest. It quite clearly
tells me to look before the current token and shove in an appropriate
thingie :-) The extra vertical space, to me, makes the error message
harder to parse.
But I can see how others would prefer to see the point where the
punctuation was missing. So I won't let my biases get in the way here.
>
> gcc/c/ChangeLog:
> PR c/7356
> * c-parser.c (c_parser_declaration_or_fndef): Detect missing
> semicolons.
>
> gcc/testsuite/ChangeLog:
> PR c/7356
> PR c/44515
> * c-c++-common/pr44515.c: New test case.
> * gcc.dg/pr7356-2.c: New test case.
> * gcc.dg/pr7356.c: New test case.
> * gcc.dg/spellcheck-typenames.c: Update the "singed" char "TODO"
> case to reflect changes to output.
> * gcc.dg/noncompile/920923-1.c: Add dg-warning to reflect changes
> to output.
OK.
jeff
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] C: detect more missing semicolons (PR c/7356)
2017-10-25 16:05 ` Jeff Law
@ 2017-10-26 0:26 ` David Malcolm
0 siblings, 0 replies; 3+ messages in thread
From: David Malcolm @ 2017-10-26 0:26 UTC (permalink / raw)
To: Jeff Law, gcc-patches
On Wed, 2017-10-25 at 09:59 -0600, Jeff Law wrote:
> On 10/11/2017 01:32 PM, David Malcolm wrote:
> > [This patch assumes the two patches here:
> > https://gcc.gnu.org/ml/gcc-patches/2017-10/msg00274.html ]
>
> I see the patch directly referenced in that message on the
> trunk. But
> I'm not sure what you mean by "two patches". If there's a prereq
> that
> hasn't been approved, let me know.
Sorry about the confusion; the one at the URL above was:
"[PATCH 2/2] C/C++: add fix-it hints for various missing symbols (v3)",
which is an updated version of:
"[PATCH 2/2] C/C++: add fix-it hints for various missing symbols (v2)"
https://gcc.gnu.org/ml/gcc-patches/2017-09/msg01746.html
the other one I was referring to was:
"[PATCH 1/2] C++: avoid partial duplicate implementation of
cp_parser_error"
https://gcc.gnu.org/ml/gcc-patches/2017-09/msg01745.html
Both these prereqs are in trunk (as r253686 and r253690 respectively).
>
> >
> > c_parser_declaration_or_fndef has logic for parsing what might be
> > either a declaration or a function definition.
> >
> > This patch adds a test to detect cases where a semicolon would have
> > terminated the decls as a declaration, where the token that follows
> > would start a new declaration specifier, and updates the error
> > message
> > accordingly, with a fix-it hint.
> >
> > This addresses PR c/7356, fixing the case of a stray token before a
> > #include which previously gave inscrutable output, and improving
> > e.g.:
> >
> > int i
> > int j;
> >
> > from:
> >
> > t.c:2:1: error: expected '=', ',', ';', 'asm' or '__attribute__'
> > before 'int'
> > int j;
> > ^~~
> >
> > to:
> >
> > t.c:1:6: error: expected ';' before 'int'
> > int i
> > ^
> > ;
> > int j;
> > ~~~
> >
> > The patch also adds a test for PR c/44515 as a baseline.
>
> Personally I find the current error easier to digest. It quite
> clearly
> tells me to look before the current token and shove in an appropriate
> thingie :-) The extra vertical space, to me, makes the error
> message
> harder to parse.
You've been using gcc for multiple decades and are used to the existing
behavior, though ;)
> But I can see how others would prefer to see the point where the
> punctuation was missing. So I won't let my biases get in the way
> here.
FWIW the benefit is a *lot* clearer for the first case described, where
the two locations are in different source files due to the missing
semicolon being immediately before a #include (PR c/7356), or at the
end of a header; it replaces dozens of crazy-looking errors with a
single sane one.
> > gcc/c/ChangeLog:
> > PR c/7356
> > * c-parser.c (c_parser_declaration_or_fndef): Detect missing
> > semicolons.
> >
> > gcc/testsuite/ChangeLog:
> > PR c/7356
> > PR c/44515
> > * c-c++-common/pr44515.c: New test case.
> > * gcc.dg/pr7356-2.c: New test case.
> > * gcc.dg/pr7356.c: New test case.
> > * gcc.dg/spellcheck-typenames.c: Update the "singed" char
> > "TODO"
> > case to reflect changes to output.
> > * gcc.dg/noncompile/920923-1.c: Add dg-warning to reflect
> > changes
> > to output.
>
> OK.
Thanks; committed to trunk as r254093 (and closing out PR c/7356 after
15 years!)
Dave
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-10-26 0:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-11 19:39 [PATCH] C: detect more missing semicolons (PR c/7356) David Malcolm
2017-10-25 16:05 ` Jeff Law
2017-10-26 0:26 ` David Malcolm
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).