This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [C/C++] Do not pretty-print expressions with caret diagnostics for wrong function call


On 21 April 2012 18:49, Jakub Jelinek <jakub@redhat.com> wrote:
>
> Before we start to add many similar tests, I wonder if we shouldn't
> test not just that caret diagnostics is on, but that it will be actually
> printed for the specific locus. ?The source could come up from
> stdin, or the file no longer available, etc.  So, shouldn't
> this be guarded instead on some predicate that takes locus_t as an
> argument (input_location in this case), say can_emit_caret_diagnostics_p,
> which would return false right away for !flag_diagnostics_show_caret,
> otherwise would try to grab the source line (and cache it) and return true
> only if it succeeded?  Then error_at after it if returned true would just
> use the cached line, so it wouldn't read things twice.

Well, my preference would be to have the same message with and without
caret diagnostics, instead of still pretty-printing expressions when
the caret is disabled. Because none of my patches do actually fix the
pretty-printing of expressions: when the caret is disabled the
diagnostics are still broken.

Clang has perfect source locations and an AST closer the original
source code and they don't ever try to pretty-print expressions. On
the other hand, GCC does not even try to track the source location of
every token and its AST does not attempt to match the original source
code, and yet GCC still pretends to rebuild the original source code
when pretty-printing. And, hence, GCC often fails in embarrassing
ways.

Thus, pretty-printing expressions when the caret is disabled is the
wrong thing to do in my opinion. However, since Gabriel disagrees and
it is easy to just test for flag_diagnostics_show_caret and keep the
old code if disabled, I am doing just that in the patches that remove
pretty-printing expressions. And there is still a huge work ahead to
remove all the pretty-printing when the caret is enabled. So I
personally don't want to spend a significant effort trying to guess
when the caret was not printed in order to fall-back to something that
is broken.

People that think that pretty-printing expressions is a good idea
should work on it, if they care enough. But perhaps they should start
first by fixing the bugs in the pretty-printer, starting with the
examples in http://gcc.gnu.org/PR49152

> There have been requests to (at least optionally) limit caret diagnostics
> to certain number of carets and then stop emitting them if there are too
> many, because with caret the output is most often 3 times as long, such
> predicate could take that into account too if it is added.

That sounds very difficult to get right and a hack to avoid addressing
the real problem, which is that GCC prints a lot of useless
diagnostics, with either irrelevant or duplicated locations. Until
now, it was easy to ignore them because somewhere in the output there
was the correct diagnostic or the correct location, but with the caret
it is now too obvious that the information is either redundant or just
wrong. So the solution is to fix the diagnostics, not to hide the
caret so it is not so obvious that the diagnostics are broken. And
this is what I am doing. See
http://gcc.gnu.org/ml/gcc-patches/2012-04/msg01283.html, any help on
getting that patch working is welcome.

Cheers,

Manuel.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]