On 21 April 2012 18:49, Jakub Jelinek <ja...@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.
