On Mon, 2017-07-03 at 19:57 +0100, Richard Sandiford wrote:
> [Thanks for all your diagnostic work btw.]
>
> David Malcolm <dmalc...@redhat.com> writes:
> > clang can also print notes about matching opening symbols
> > e.g. the note here:
> >
> > missing-symbol-2.c:25:22: error: expected ']'
> > const char test [42;
> > ^
> > missing-symbol-2.c:25:19: note: to match this '['
> > const char test [42;
> > ^
> > which, although somewhat redundant for this example, seems much
> > more
> > useful if there's non-trivial nesting of constructs, or more than a
> > few
> > lines separating the open/close symbols (e.g. showing a stray
> > "namespace {"
> > that the user forgot to close).
> >
> > I'd like to implement both of these ideas as followups, but in
> > the meantime, is the fix-it hint patch OK for trunk?
> > (successfully bootstrapped & regrtested on x86_64-pc-linux-gnu)
>
> Just wondering: how easy would it be to restrict the note to the
> kinds
> of cases you mention? TBH I think clang goes in for extra notes too
> much, and it's not always that case that an "expected 'foo'" message
> really is caused by a missing 'foo'. It'd be great if there was some
> way of making the notes a bit more discerning. :-)
My plan was to only do it for open/close punctuation, i.e.:
* '(' and ')'
* '{' and '}'
* '[' and ']'
* maybe '<' and '>' in C++
> Or maybe do something like restrict the extra note to cases in which
> the
> opening character is on a different line and use an underlined range
> when the opening character is on the same line?
Good idea: if it's on the same line, use a secondary range; if it's on
a different line, use a note.
The above example would look something like this (with the '[' as a
secondary range):
missing-symbol-2.c:25:22: error: expected ']'
const char test [42;
~ ^
]
which is more compact than the "separate note" approach, whilst (IMHO)
being just as readable.
FWIW diagnostic-show-locus.c can handle widely-separated secondary
ranges within one rich_location, provided they're in the same source
file (see calculate_line_spans, and the start_span callback within
diagnostic_context).
Consider the unclosed namespace here:
$ cat -n test.cc
1 namespace ns {
2
3 void test ()
4 {
5 }
for which we currently emit the rather unhelpful:
$ gcc test.cc
test.cc:5:1: error: expected ‘}’ at end of input
}
^
Printing it via a secondary range using a single rich_location with
just an "error_at_rich_loc" call would print something like:
test.cc:5:1: error: expected ‘}’ at end of input
test.cc:1:14:
namespace ns {
^
test.cc:5:1:
}
^
}
which works, but I'm not a fan of.
In constrast, with the "if it's on a different line, use a note" approach, we
would print:
test.cc:5:1: error: expected ‘}’ at end of input
}
^
}
test.cc:1:14: note: to match this '{'
namespace ns {
^
which I think is better (and supports the cases where they're in different
files (e.g. you have a stray unclosed namespace in a header file,
somewhere...), or macros are involved, etc)
So I'll have a go at implementing the "is it on a different line" logic you
suggest.
For reference, clang prints the following for the above case:
test.cc:5:2: error: expected '}'
}
^
test.cc:1:14: note: to match this '{'
namespace ns {
^
Thinking aloud, maybe it would be better for the fix-it hint to suggest putting
the '}' on a whole new line. Might even be good to suggest adding
} // namespace ns
or similar (for this specific case), giving this output:
test.cc:5:1: error: expected ‘}’ at end of input
}
+} // namespace ns
test.cc:1:14: note: to match this '{'
namespace ns {
^
(only works if the proposed insertion point is on the end of a line, given the
current restrictions on what our fix-it machinery is capable of - we don't
currently support splitting a pre-existing line via a fix-it hint)
Thanks.
Dave
