On 07/11/2017 09:24 AM, David Malcolm wrote:
[This patch kit is effectively just one patch; I've split it up into
3 parts, in the hope of making it easier to review:
the c-family parts, the C parts, and the C++ parts]
This patch adds a hint to the user to various errors generated
in the C frontend by:
c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>")
c_parser_skip_until_found (parser, CPP_CLOSE_BRACE, "expected %<}%>")
etc (where there's a non-NULL msgid), and in the C++ frontend by:
cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN)
cp_parser_require (parser, CPP_CLOSE_BRACE, RT_CLOSE_BRACE)
The hint shows the user where the pertinent open paren or open brace
is, which ought to be very helpful for complicated nested collections
of parentheses, and somewhat helpful even for simple cases;
I've played with the patch a bit. First, let me say that I like
how it associates the curlies. I agree that it will be helpful.
There are other cases where highlighting mismatched or missing
tokens might be useful, such as for pairs of < and > in complex
template declarations.
But I mainly experimented with it to see if I could get it to
manifest some of the same symptoms I described in bug 81269.
I'm not sure it does reproduce the exact same thing or if it's
a feature, so let me use this as an opportunity to ask. Given
something like
namespace {
enum { e
<EOF>
I see this output:
a.C:3:8: error: expected ‘}’ at end of input
enum { e
~ ^
a.C:3:8: error: expected unqualified-id at end of input
a.C:3:8: error: expected ‘}’ at end of input
a.C:1:11: note: to match this ‘{’
namespace {
^
with the first open curly/caret in green, the 'e' in red, and
the last open curly/caret in cyan.
Is the green color intended? And if yes, what is the intent of
distinguishing it from the red 'e'? I note that the caret is
red (and there are no other colors in the output) in this case:
namespace { enum {
<EOF>
but becomes green again when I add an enumerator:
namespace { enum { e
<EOF>
I ask because in the test case in 81269, highlighting the different
tokens in the three colors seems especially confusing and I'd like
to better understand if it's intentional (and what it means).
Incidentally, I tried to make use of this feature in the middle
end (in gimple-ssa-sprintf.c), to achieve the same effect as
-Wrestric does, but it led to even stranger-looking results so
I went back to using plain old warning. See the attachment to
bug 81269:
https://gcc.gnu.org/bugzilla/attachment.cgi?id=41660
Martin
consider e.g.:
...lots of lines of code...
extern "C" {
...lots of lines of code...
int test ();
EOF
where the user currently has to hunt through the source file to find
the unclosed '{':
test.cc:262:12: error: expected '}' at end of input
int test ();
^
With this patch we tell them:
test.cc:262:12: error: expected '}' at end of input
int test ();
^
test.cc:98:12: note: to match this '{'
extern "C" {
^
The patch avoids using a note if the tokens are on the same line,
highlighting the unclosed open token with an underline:
test.c:3:32: error: expected ')' before ';' token
return ((b * b) - (4 * a * c);
~ ^
The bulk of the changes in the patch are to the parsers, done using
new classes "matching_braces" and "matching_parens", which stash the
location of the opening token during parsing, so that e.g.:
if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>"))
return;
...do stuff...
c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>");
becomes:
matching_parens parens;
if (!parens.require_open (parser))
return;
...do stuff...
parens.require_close (parser);
The exact implementation of these classes varies somewhat between the
C and C++ frontends, to deal with implementation differences between
them (I tried to keep them as similar as possible).
Successfully bootstrapped®rtested on x86_64-pc-linux-gnu;
adds 23 PASS results to gcc.sum; adds 99 PASS results to g++.sum.
OK for trunk?
gcc/c-family/c-common.c | 17 +-
gcc/c-family/c-common.h | 3 +-
gcc/c/c-parser.c | 647 ++++++++++------
gcc/c/c-parser.h | 8 +-
gcc/cp/parser.c | 821 ++++++++++++++-------
gcc/testsuite/c-c++-common/missing-close-symbol.c | 33 +
gcc/testsuite/c-c++-common/missing-symbol.c | 50 ++
.../g++.dg/diagnostic/unclosed-extern-c.C | 3 +
.../g++.dg/diagnostic/unclosed-function.C | 3 +
.../g++.dg/diagnostic/unclosed-namespace.C | 2 +
gcc/testsuite/g++.dg/diagnostic/unclosed-struct.C | 3 +
gcc/testsuite/g++.dg/parse/pragma2.C | 4 +-
gcc/testsuite/gcc.dg/unclosed-init.c | 3 +
13 files changed, 1084 insertions(+), 513 deletions(-)
create mode 100644 gcc/testsuite/c-c++-common/missing-close-symbol.c
create mode 100644 gcc/testsuite/c-c++-common/missing-symbol.c
create mode 100644 gcc/testsuite/g++.dg/diagnostic/unclosed-extern-c.C
create mode 100644 gcc/testsuite/g++.dg/diagnostic/unclosed-function.C
create mode 100644 gcc/testsuite/g++.dg/diagnostic/unclosed-namespace.C
create mode 100644 gcc/testsuite/g++.dg/diagnostic/unclosed-struct.C
create mode 100644 gcc/testsuite/gcc.dg/unclosed-init.c
