Bug 49152 - pretty printer cannot handle iterators and other complex expressions
Summary: pretty printer cannot handle iterators and other complex expressions
Status: REOPENED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 4.7.0
: P3 enhancement
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
: 25362 (view as bug list)
Depends on: caret
Blocks:
  Show dependency treegraph
 
Reported: 2011-05-24 21:14 UTC by Jonathan Wakely
Modified: 2016-01-26 11:33 UTC (History)
6 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2012-03-21 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Wakely 2011-05-24 21:14:52 UTC
This code, based on what happens in std::find():

#include <vector>

struct X { };
struct Y { } val;

std::vector<X>::iterator first;

bool b = *first == val;

produces this output:

fail.cc:8:20: error: no match for 'operator==' in 'first.__gnu_cxx::__normal_iterator<_Iterator, _Container>::operator* [with _Iterator = X*, _Container = std::vector<X>, __gnu_cxx::__normal_iterator<_Iterator, _Container>::reference = X&]() == val'

or with -fno-pretty-templates

fail.cc:8:20: error: no match for 'operator==' in 'first.__gnu_cxx::__normal_iterator<X*, std::vector<X, std::allocator<X> > >::operator*() == val'


I have a few problems with the diagnostic, the main ones are

1) Why is the "()" after the "[with ...]" template argument list in the -fpretty-templates version?

2) "first.__gnu_cxx::__normal_iterator<_Iterator, _Container>::operator*" is an abomination, "*first" would be far more helpful

3) after all that verbosity about *first, there's no clue what type val is.


To be helpful it should either show something resembling the original source:

   *first == val;

or it should show the types involved:

   no match for operator== with operands X& and Y&

or it should show both, like clang does:

error: invalid operands to binary expression ('X' and 'struct Y')
bool b = *first == val;
         ~~~~~~ ^  ~~~

but what we actually have doesn't manage to resemble the original source or show the types involved
Comment 1 Christopher Yeleighton 2011-09-21 13:17:44 UTC
To figure out what the types involved are, you can say 

(int &) first; (int &) val;

which would work most of the time.

However, this requires modifying the source and recompiling, which is such a waste of time.
Comment 2 Christopher Yeleighton 2011-09-21 20:06:42 UTC
== Code ==

struct X { int x; }; 
void trigger (X x []) { x [01] = 0; }

== Result (v4.6) ==

doit.cpp:2:34: error: no match for ‘operator=’ in ‘*(x + 4u) = 0’

But who referenced (x + 4u)?
Comment 3 Jonathan Wakely 2011-09-21 20:16:24 UTC
Wow, that one is worthy of its own bug report, it's not just an unclear diagnostic, it's completely bogus.

x[01] is *(x+1) or *((char*)x + 4) but what G++ prints is just wrong
Comment 4 Jonathan Wakely 2011-09-21 21:03:37 UTC
The common thing in my original example and comment 2 is that when printing "no match" for a binary operator, the diagnostic machinery tries to "reconstruct" the left operand, but produces a poor result if that left operand is itself the result of an operator expression

e.g. in my case *first gets "reconstructed" to first.foo::operator*
(while technically equivalent, noone would write the latter)

in comment 2 x[1] gets "reconstructed" to *(x + 4)
(not even equivalent, the reconstruction has forgotten to divide the offset by sizeof(*x))
Comment 5 Manuel López-Ibáñez 2011-09-21 21:50:21 UTC
This is a very old issue, and there are several PRs open already. It is widely acknowledged that GCC needs to print the input expression instead of reconstructing it. But this is (very) hard to fix with current GCC design. See point A) in

http://gcc.gnu.org/wiki/Better_Diagnostics

and the several PRs mentioned there.

Dodji is working on A.0, and it will probably land on 4.7, but nobody else is working in any other point there. I started working on A.3 but I got totally discouraged and gave up.

The only short term solution is to stop printing expressions and let the user check the source by the location. In fact, this is also part of the long-term solution, because printing whole expressions in diagnostics is just clutter if those expressions take several lines. The only fix is to implement caret diagnostics and avoid expressions in diagnostic messages, like clang does. But who is going to work on that?
Comment 6 Jonathan Wakely 2011-09-21 23:30:34 UTC
Thanks, Manu.  Most of the PRs mentioned on that page are FIXED, and I don't see specific mention of outputting the types involved in the operation, just better way of outputting the original source instead of reconstructing it.  In many cases the types would be more useful than printing the original source range.

i.e. I'd rather see:

   no match for operator== with operands X& and Y&

than:

   no match for operator== in 'foo == bar'

because whether a suitable operator exists is a property of the variables' types, not their names.

That might also be easier to implement, since the compiler knows the types. It doesn't (without a lot of work) know the character range from the original source file that corresponds to the expression.

Another alternative would be to note when operator notation is used, so that when *foo is transformed to a call to the member function foo.operator*() internally, the diagnostic can know to reconstruct it to *foo not foo.operator*() in diagnostics.
This might only be needed for operators, since I haven't encountered such serious problems with reconstructing normal function calls such as bar(g), only for operator notation.
Comment 7 Manuel López-Ibáñez 2011-09-21 23:56:25 UTC
(In reply to comment #6)
> Thanks, Manu.  Most of the PRs mentioned on that page are FIXED, and I don't

PR 35441 is still broken. In any case, most of the fixes just gave up on printing a reconstructed expression, and they print something else, for example '({...})'.

> i.e. I'd rather see:
> 
>    no match for operator== with operands X& and Y&
> 
> than:
> 
>    no match for operator== in 'foo == bar'
> 
> because whether a suitable operator exists is a property of the variables'
> types, not their names.
> 
> That might also be easier to implement, since the compiler knows the types.

I agree that this would be an improvement, even without clang's selective typedef unwrapping. But I have some doubts such a patch will be accepted. I was told in a couple of occasions that I cannot just remove the expression from the diagnostic and rely on the user to look up the source code location.
Comment 8 Christopher Yeleighton 2011-09-27 22:23:34 UTC
> I agree that this would be an improvement, even without clang's selective
> typedef unwrapping. But I have some doubts such a patch will be accepted. I was
> told in a couple of occasions that I cannot just remove the expression from the
> diagnostic and rely on the user to look up the source code location.

What follows is an expression regurgitated by g++ for your convenient perusal.  You might want to show it to the person who expresses such strong opinions.  Ask him gently to tell you what it means.

doit.cpp:290:129: error: no match for ‘operator<<’ in ‘std::cout << boost::range::equal_range [with ForwardRange = boost::range_detail::transform_range<boost::value_factory<ls::file_rec<char>::as_char0>, std::vector<ls::file_rec<char> > >, Value = int, typename boost::range_iterator<const ForwardRange>::type = boost::transform_iterator<boost::value_factory<ls::file_rec<char>::as_char0>, __gnu_cxx::__normal_iterator<ls::file_rec<char>*, std::vector<ls::file_rec<char> > >, boost::use_default, boost::use_default>]((*(const boost::range_detail::transform_range<boost::value_factory<ls::file_rec<char>::as_char0>, std::vector<ls::file_rec<char> > >*)(& boost::range_detail::operator| [with InputRng = std::vector<ls::file_rec<char> >, UnaryFunction = boost::value_factory<ls::file_rec<char>::as_char0>]((* & lv_fr), (*(const boost::range_detail::transform_holder<boost::value_factory<ls::file_rec<char>::as_char0> >*)(& boost::adaptors::{anonymous}::transformed.boost::range_detail::forwarder<Holder>::operator() [with T = boost::value_factory<ls::file_rec<char>::as_char0>, Holder = boost::range_detail::transform_holder]((boost::value_factory<ls::file_rec<char>::as_char0>(), boost::value_factory<ls::file_rec<char>::as_char0>()))))))), (* &((int)a_n.std::basic_string<_CharT, _Traits, _Alloc>::operator[] [with _CharT = char, _Traits = std::char_traits<char>, _Alloc = std::allocator<char>, std::basic_string<_CharT, _Traits, _Alloc>::reference = char&, std::basic_string<_CharT, _Traits, _Alloc>::size_type = long unsigned int](0ul))))’
Comment 9 Manuel López-Ibáñez 2012-03-21 13:21:12 UTC
I think this is confirmed. I agree with the suggestion in comment #6.
Comment 10 Manuel López-Ibáñez 2012-03-21 13:21:53 UTC
*** Bug 25362 has been marked as a duplicate of this bug. ***
Comment 11 Paolo Carlini 2012-03-22 00:00:20 UTC
On it.
Comment 12 Paolo Carlini 2012-03-22 10:29:33 UTC
Not actively working on it. See:

  http://gcc.gnu.org/ml/gcc-patches/2012-03/msg01471.html

about why we don't want to just print types (missing the caret).
Comment 13 Jonathan Wakely 2012-03-22 11:02:29 UTC
Please don't close this as a dup of the caret diagnostics PR.

I agree there's no consensus on what should be done, but printing of reconstructed expressions is utterly awful.  I think I qualify as a C++ expert by almost any measure and am very familiar with G++ and its diagnostics, but I frequently have to totally ignore G++ diagnostics because a single expression is printed as 10 or more lines of unhelpful nested-name-specifiers and template arguments. I just ignore the diagnostic and go back to the code to figure out what's wrong without the compiler's help.

Re Gaby's point about the error appearing in a sub-expression of a larger full expression, that's true, but the current behaviour doesn't help. The message in comment 0 is unreadable and if it appears in a larger expression it would be very difficult to determine which sub-expression the error came from, because there is no resemblance to the original source.  The source says *first, the message says first.__gnu_cxx::__normal_iterator<_Iterator, _Container>::operator* which is not the same thing to users.  I'm using a std::vector::iterator, WTF is __normal_iterator? How does printing that help?  Oh, I see, right at the end of the line it refers to "val" so I can try to find the relevant sub-expression, but by that point I've already clawed out my eyes trying to look at the rest of the diagnostic.

Going back to point (3) in comment 0:  why is the left operand printed as irrelevant crap about an iterator (when the relevant type is X&) but the right operand printed as it appears in the source, not as a type?


Let's change the example slightly:

#include <vector>

struct Y { } val;

std::vector<Y>::iterator iter;

bool b = *iter == val;

4.7 gives:

t.cc:7:19: error: no match for 'operator==' in 'iter.__gnu_cxx::__normal_iterator<_Iterator, _Container>::operator*<Y*, std::vector<Y> >() == val'

The problem is that Y is not equality comparable, it has nothing to do with iterators, but more than 50% of the output refers to an iterator type which is an implementation detail.  Also, the message is just bogus, WTF is "operator*<Y*, std::vector<Y> >()" ?  That operator isn't a template function, why does it have a template argument list?!

-fno-pretty-templates makes it less totally wrong:

t.cc:7:19: error: no match for 'operator==' in 'iter.__gnu_cxx::__normal_iterator<Y*, std::vector<Y, std::allocator<Y> > >::operator*() == val'

Now at least we see the type printed correctly and no bogus template argument list on a non-template member function.
Comment 14 Jonathan Wakely 2012-03-22 11:17:36 UTC
Also, comment 2 shows another clear bug in the current diagnostics. It doesn't matter whether you prefer carets or types or reconstructed expressions, printing x[1] as *(x + 4u) is plain wrong.

So let's keep it open.
Comment 15 Manuel López-Ibáñez 2012-03-22 11:37:17 UTC
(In reply to comment #13)
> Please don't close this as a dup of the caret diagnostics PR.
> 
> I agree there's no consensus on what should be done, but printing of

I only know of one dissenting opinion. What does Jason and Benjamin think about
it? A dissenting opinion has been overridden before and the sky didn't fall.

> Re Gaby's point about the error appearing in a sub-expression of a larger full
> expression, that's true, but the current behaviour doesn't help. The message 

I think it is a red herring. Any IDE, including emacs, will show the location
of the error in the source. So as long as the location is right, it is more
clear to not print anything than to print a *different* expression. That is,
even without caret, it is better to not print the expression if the expression
is not *exactly* the one appearing in the code. 

The only people that are helped by the current behavior are those that
understand what 'first.__gnu_cxx::__normal_iterator<_Iterator,
_Container>::operator*' means. See comment #6 and cry.
Comment 16 Jonathan Wakely 2012-03-22 12:01:58 UTC
Right, we have the column numbers to indicate the position in the source
(not perfect, but enough to show which sub-expression the error refers to)
Comment 17 Manuel López-Ibáñez 2012-03-22 12:59:30 UTC
(In reply to comment #16)
> Right, we have the column numbers to indicate the position in the source
> (not perfect, but enough to show which sub-expression the error refers to)

It is in fact much easier to fix the location of a sub-expression than to fix the pretty-printers. If the location cannot be fixed because of folding or other transformation, my guess is that it will be impossible to pretty-print something useful, and even the caret will not work.
Comment 18 Ralph Loader 2012-03-22 14:21:33 UTC
Flaws from the pretty-printing not listed in this bug (from 25362):

It takes the address of integer constants.

The pretty printing confuses '.' and '->' [this has changed slightly since 25362 was logged.  An attempt has been made to fix it by inserting a unary '&'.  But that is not correct: '&' might be overloaded for the type in question...]

It prints something utterly bizarre when a std::map is a base class --- it looks like it is inventing a member '<Anonymous>' to contain the base type.  [I assume that is how gcc represents inheritance internally.  But it shouldn't be in the error message.]


My thoughts on bikeshed colours:

This bug is currently listed as 'P3 - enhancement'.  But it is a real bug.  The current output is incorrect, not just something that could be improved.

This bug is a regression.  Once upon a time [I just checked with gcc-3.2.3], gcc did actually produce usable diagnostics that were vaguely correct (by giving the relevant type information instead).  That also had the advantage of being fairly concise, readable, and useful to the user.
Comment 19 Jonathan Wakely 2012-03-22 14:27:11 UTC
(In reply to comment #18)
>  But that is not correct: '&' might be overloaded for the type in question...]

If the diagnostics were improved for all types except for those with overloaded operator& then I think we could happily mark the PR as FIXED ;)
Comment 20 Ralph Loader 2012-03-23 07:54:51 UTC
Re comment 12 - as someone who regularly needs to understand gcc diagnostics, I disagree completely.

Understanding a failure to look something up, the single most important thing to know is the key being looked up, and in this case the key is the function/operator name and the argument types.

gcc used to output that information, and removing that was a regression, completely independently of whether the current diagnostics make any sense.

Given that no significant progress has been made with fixing the current diagnostics in nearly a decade, it is puzzling to me why the obvious & simple fix of reverting to the previous, useful, diagnostics, has not been made.
Comment 21 Manuel López-Ibáñez 2012-03-24 16:32:07 UTC
Jason, do you have an opinion on this?
Comment 22 Manuel López-Ibáñez 2012-03-31 00:25:50 UTC
Is there a final verdict on this? Jonathan, Paolo, did you change your mind? 

Or do you still think this should be fixed but you don't believe there is any hope to get your patch approved?
Comment 23 Manuel López-Ibáñez 2012-03-31 00:34:58 UTC
BTW, I think this example was mentioned some where already, but I cannot find it now. From http://clang.llvm.org/diagnostics.html

manuel@gcc12:~$ cat t.cc
struct a {
  virtual int bar();
};
  
struct foo : public virtual a {
};
  
int test(foo *P) {
  return P->bar() + *P;
}

manuel@gcc12:~$ trunk/180166/install/bin/g++ -fsyntax-only ~/t.cc
/home/manuel/t.cc: In function ‘void test(foo*)’:
/home/manuel/t.cc:9:22: error: no match for ‘operator+’ in ‘(((a*)P) + ((sizetype)(*(long int*)(P->foo::<anonymous>.a::_vptr.a + 0xffffffffffffffffffffffffffffffe0u))))->a::bar() + * P’

manuel@gcc12:~$ ~/bin/clang -fsyntax-only ~/t.cc 
/home/manuel/t.cc:9:19: error: invalid operands to binary expression ('int' and 'foo')
  return P->bar() + *P;
         ~~~~~~~~ ^ ~~
1 error generated.
Comment 24 Paolo Carlini 2012-03-31 02:03:02 UTC
Personally, I don't believe Gaby is open to other solutions outside the full-fledged "caret diagnostics" context, thus for the time being at least I'm personally going to stand to his judgment (as diagnostics maintainer).
Comment 25 Paolo Carlini 2012-03-31 02:15:19 UTC
And, hey, I'm of course speaking only for myself, you are welcome to pursue a compromise solution. For example, I don't know, if we could identify a *restricted* class of expressions which we have troubles reconstructing and propose skipping printing the expressions only in those specific cases.
Comment 26 Manuel López-Ibáñez 2012-04-01 11:24:14 UTC
(In reply to comment #24)
> Personally, I don't believe Gaby is open to other solutions outside the
> full-fledged "caret diagnostics" context, thus for the time being at least I'm
> personally going to stand to his judgment (as diagnostics maintainer).

The caret is not a solution to this problem, because what Gabriel wants is to not reconstruct expressions ONLY when the caret is shown, but he has said in the past that the caret should default to OFF to not change the current output for IDEs and other software parsing the output of gcc like emacs, so we are back to printing the monsters mentioned above by default.

This is another reason why I am not motivated to keep working on the caret. What is the point if it is going to default to OFF anyway? That and that moving line-map out of libcpp to create a source-location library has been rejected in the past.

And there is no compromise solution. The only argument in favor of pretty-printing expressions is to help identify the culprit in a complex expression, so printing only expressions that have non-expression operands will surely be rejected by Gabriel.

Even if the caret was the solution, nobody has worked on it in the last 25 years, and now that we have good column information, I don't expect that there will be anyone working on it in the next 25. Specially now that there is a better alternative, because if you want nice diagnostics, why don't just use Clang?
Comment 27 Jonathan Wakely 2012-04-01 12:55:34 UTC
(In reply to comment #26)
> because if you want nice diagnostics, why don't just use
> Clang?

Because G++ gives much better diagnostics for template argument deduction failures, so "just use clang" isn't a solution even if all you care about is better diagnostics.
Comment 28 Manuel López-Ibáñez 2012-04-01 13:23:01 UTC
(In reply to comment #27)
> (In reply to comment #26)
> > because if you want nice diagnostics, why don't just use
> > Clang?
> 
> Because G++ gives much better diagnostics for template argument deduction
> failures, so "just use clang" isn't a solution even if all you care about is
> better diagnostics.

I am talking about the perspective of a new contributor that starts from zero knowledge in both Clang and GCC. Then, it looks easier and it is in fact much less effort to fix Clang to give the same diagnostics as g++ for template argument deduction than to add caret diagnostics to GCC.

But my main point is that it is not a matter of effort: Even if g++ indeed gives better diagnostics in some cases, this is either recognized as a bug in Clang that can be fixed or it is a matter of opinion (more information versus conciseness, assume users know the language standard terminology vs. use less precise but more widely understandable language).

On the other hand, the current status is that the output of G++ is broken "by design" and no patches will be accepted to change the design. Quoting http://clang.llvm.org/diagnostics.html:

No Pretty Printing of Expressions in Diagnostics

Since Clang has range highlighting, it never needs to pretty print your code back out to you. GCC can produce inscrutible error messages in some cases when it tries to do this. In this example P and Q have type "int*":

  $ gcc-4.2 -fsyntax-only t.c
  #'exact_div_expr' not supported by pp_c_expression#'t.c:12: error: called object  is not a function
  $ clang -fsyntax-only t.c
  t.c:12:8: error: called object type 'int' is not a function or function pointer
    (P-Q)();
    ~~~~~^

This can be particularly bad in G++ which often emits errors containing lowered vtable references:

  $ gcc-4.2 t.cc
  t.cc: In function 'void test(foo*)':
  t.cc:9: error: no match for 'operator+' in '(((a*)P) + (*(long int*)(P->foo::<anonymous>.a::_vptr$a + -0x00000000000000020)))->a::bar() + * P'
  t.cc:9: error: return-statement with a value, in function returning 'void'
  $ clang t.cc
  t.cc:9:18: error: invalid operands to binary expression ('int' and 'foo')
    return P->bar() + *P;
           ~~~~~~~~ ^ ~~
Comment 29 Marc Glisse 2012-04-01 20:28:14 UTC
(In reply to comment #24)
> Personally, I don't believe Gaby is open to other solutions outside the
> full-fledged "caret diagnostics" context,

He didn't seem opposed to _adding_ the type information (without removing the current information).
Comment 30 Jason Merrill 2012-04-02 05:16:05 UTC
(In reply to comment #26)
> The caret is not a solution to this problem, because what Gabriel wants is to
> not reconstruct expressions ONLY when the caret is shown, but he has said in
> the past that the caret should default to OFF to not change the current output
> for IDEs and other software parsing the output of gcc like emacs, so we are
> back to printing the monsters mentioned above by default.

I think I've said before that caret should default to on when the output is a terminal.

> That and that moving
> line-map out of libcpp to create a source-location library has been rejected in
> the past.

I don't remember the discussion about this.  What was the goal?

> Even if the caret was the solution, nobody has worked on it in the last 25
> years, and now that we have good column information, I don't expect that there
> will be anyone working on it in the next 25.

Well, we didn't have the competitive pressure from clang in the past 25 years; now we do.

Good column information is equivalent to caret diagnostics; the column number tells you where the caret goes, so an IDE can put the cursor there.  As I've suggested before, one quick way to get caret diagnostics given the column information would be to write a quick perl postprocessor that just prints the designated line and puts a caret under the designated column.

Range highlighting like clang has is something further; we would need two source locations per expression, rather than the one we currently have.

And G++ still needs some work to track expression locations better, it hasn't gotten as much work on that as the C front end.
Comment 31 Manuel López-Ibáñez 2012-04-02 08:16:52 UTC
(In reply to comment #30)
> (In reply to comment #26)
> > The caret is not a solution to this problem, because what Gabriel wants is to
> > not reconstruct expressions ONLY when the caret is shown, but he has said in
> > the past that the caret should default to OFF to not change the current output
> > for IDEs and other software parsing the output of gcc like emacs, so we are
> > back to printing the monsters mentioned above by default.
> 
> I think I've said before that caret should default to on when the output is a
> terminal.
> 

Well, that is reassuring. Then, will we still pretty-print expressions in diagnostics once we have the caret?

Is there a GCC way to detect that the output is a terminal?
Comment 32 Jason Merrill 2012-04-02 15:57:16 UTC
(In reply to comment #31)
> Well, that is reassuring. Then, will we still pretty-print expressions in
> diagnostics once we have the caret?

No, there should be no need to.
 
> Is there a GCC way to detect that the output is a terminal?

I don't know if there's a usual GCC way, but isatty seems like a good way.
Comment 33 Manuel López-Ibáñez 2012-04-02 17:15:47 UTC
(In reply to comment #32)
> (In reply to comment #31)
> > Well, that is reassuring. Then, will we still pretty-print expressions in
> > diagnostics once we have the caret?
> 
> No, there should be no need to.

As a first minimal step, would a first patch be enough that simply:

* Adds an option to disable/enable the caret? By default on if(isatty), otherwise off.
* Implements the caret by reopening the file, looks for the appropriate line, and prints this line, then prints a '^' below it using the column info.

Of course this may fail in some cases, like non-ANSI input, and not preprocessing, but it will work in 99% of the cases. In any case, it is not clear to me that we want to print the preprocessed line instead of the original text.
Comment 34 Manuel López-Ibáñez 2012-04-02 17:18:34 UTC
(In reply to comment #32)
> 
> Of course this may fail in some cases, like non-ANSI input, and not
> preprocessing, but it will work in 99% of the cases. In any case, it is not
> clear to me that we want to print the preprocessed line instead of the original
> text.

BTW, non-ANSI input may not even handled correctly by libcpp according to PR49152, so I think we don't need to worry about it just now.
Comment 35 Manuel López-Ibáñez 2012-04-02 17:19:15 UTC
(In reply to comment #34)
> (In reply to comment #32)
> > 
> > Of course this may fail in some cases, like non-ANSI input, and not
> > preprocessing, but it will work in 99% of the cases. In any case, it is not
> > clear to me that we want to print the preprocessed line instead of the original
> > text.
> 
> BTW, non-ANSI input may not even handled correctly by libcpp according to
> PR49152, so I think we don't need to worry about it just now.

Sorry, I meant PR49973 .
Comment 36 pinskia@gmail.com 2012-04-02 17:35:59 UTC
I know some of us use tee and that disables termainal detection code usually. Or output to a file and then use tail -f. So please don't do that. It would confuse lots of users.



Sent from my Palm Pre on AT&amp;T
On Apr 2, 2012 4:17, manu at gcc dot gnu.org &lt;gcc-bugzilla@gcc.gnu.org&gt; wrote: 

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49152



--- Comment #31 from Manuel López-Ibáñez &lt;manu at gcc dot gnu.org&gt; 2012-04-02 08:16:52 UTC ---

(In reply to comment #30)

&gt; (In reply to comment #26)

&gt; &gt; The caret is not a solution to this problem, because what Gabriel wants is to

&gt; &gt; not reconstruct expressions ONLY when the caret is shown, but he has said in

&gt; &gt; the past that the caret should default to OFF to not change the current output

&gt; &gt; for IDEs and other software parsing the output of gcc like emacs, so we are

&gt; &gt; back to printing the monsters mentioned above by default.

&gt; 

&gt; I think I've said before that caret should default to on when the output is a

&gt; terminal.

&gt; 



Well, that is reassuring. Then, will we still pretty-print expressions in

diagnostics once we have the caret?



Is there a GCC way to detect that the output is a terminal?
Comment 37 Jason Merrill 2012-04-02 22:05:13 UTC
(In reply to comment #36)
> I know some of us use tee and that disables termainal detection code usually.

Right, so then you don't get the caret by default.  You can still enable it with a flag if you want.

Actually, it's not clear to me that the caret line would be likely to cause trouble for IDEs in any case; they already have to deal with output that isn't a specific error.  Maybe we should just turn it on by default in all cases, and possibly backtrack to only on a tty if it causes problems.
Comment 38 Andrew Pinski 2012-04-03 03:41:00 UTC
(In reply to comment #37)
> Actually, it's not clear to me that the caret line would be likely to cause
> trouble for IDEs in any case; they already have to deal with output that isn't
> a specific error.  Maybe we should just turn it on by default in all cases, and
> possibly backtrack to only on a tty if it causes problems.

What I was trying to say, I think it would confuse an user who did a make and saw an error and then did another make but this time piped to tee or to an output file.  And looked at the output file and it was different than what was on the tty before.

This is why I think we should not worry about the IDEs and just have them handle those cases just as we handle the case of emitting more debugging info and such.
Comment 39 Manuel López-Ibáñez 2012-04-04 15:27:35 UTC
Patch available for caret diagnostics in PR24985. It works!
Comment 40 paolo@gcc.gnu.org 2012-04-16 15:32:28 UTC
Author: paolo
Date: Mon Apr 16 15:32:22 2012
New Revision: 186499

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=186499
Log:
/cp
2012-04-16  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/49152
	* call.c (op_error): Print types; when flag_diagnostics_show_caret
	is false print expressions too.
	(op_error_string): Add.

/testsuite
2012-04-16  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/49152
	* g++.dg/diagnostic/operator1.C: New.
	* g++.dg/ext/label5.C: Adjust.
	* g++.dg/ext/va-arg1.C: Likewise.
	* g++.dg/other/error20.C: Likewise.
	* g++.dg/other/error20.C: Likewise.
	* g++.dg/other/error16.C: Likewise.
	* g++.dg/other/error10.C: Likewise.
	* g++.dg/parse/error30.C: Likewise.
	* g++.dg/cpp0x/lambda/lambda-err1.C: Likewise.

Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/call.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-err1.C
    trunk/gcc/testsuite/g++.dg/ext/label5.C
    trunk/gcc/testsuite/g++.dg/ext/va-arg1.C
    trunk/gcc/testsuite/g++.dg/other/error10.C
    trunk/gcc/testsuite/g++.dg/other/error16.C
    trunk/gcc/testsuite/g++.dg/other/error20.C
    trunk/gcc/testsuite/g++.dg/parse/error30.C
Comment 41 Paolo Carlini 2012-04-16 15:36:07 UTC
Done.
Comment 42 Jonathan Wakely 2012-04-16 15:47:37 UTC
Awesome, thank you very very much, Paolo and Manu.

The example in comment 23 can now be added to http://gcc.gnu.org/wiki/ClangDiagnosticsComparison ;)
Comment 43 Manuel López-Ibáñez 2012-04-16 16:26:09 UTC
Thanks Paolo. It is nice that by default, we do not print these monsters anymore. But the pretty-printer is still broken, and the diagnostic is still broken for -fno-diagnostics-show-caret. So let's keep it around in case someone decides to fix that or Gabriel changes his mind.

(In any case, I don't expect anyone to fix this any year soon now that it is not the default anymore, so it should have the lowest priority possible.)
Comment 44 joseph@codesourcery.com 2012-04-23 19:54:09 UTC
On Sun, 1 Apr 2012, manu at gcc dot gnu.org wrote:

> moving
> line-map out of libcpp to create a source-location library has been rejected in
> the past.

I've reviewed my contributions to that discussion 
<http://gcc.gnu.org/ml/gcc-patches/2008-11/msg00658.html> and I at least 
did not reject that move; I wanted the new library not to contain any 
translatable messages (so it doesn't need another message domain), and I 
wanted motivation for the patch in terms of modularity and better 
interfaces being the goal and physical separation into a library simply 
being a means to that goal, but I did not object to a move as such.
Comment 45 Manuel López-Ibáñez 2012-04-23 20:27:39 UTC
(In reply to comment #44)
> On Sun, 1 Apr 2012, manu at gcc dot gnu.org wrote:
> 
> > moving
> > line-map out of libcpp to create a source-location library has been rejected in
> > the past.
> 
> I've reviewed my contributions to that discussion 
> <http://gcc.gnu.org/ml/gcc-patches/2008-11/msg00658.html> and I at least 
> did not reject that move; I wanted the new library not to contain any 
> translatable messages (so it doesn't need another message domain), and I 
> wanted motivation for the patch in terms of modularity and better 
> interfaces being the goal and physical separation into a library simply 
> being a means to that goal, but I did not object to a move as such.

Re-reading that very old discussion, and with four years of more experience, I basically agree with everything you and others say there, including the fact that a new library is probably not needed. However, I still think that physical separation, be a directory along-side libcpp or under gcc, helps to clarify module boundaries. 

Nevertheless, since it turns out I did not need a source-location library to implement the caret, I am not planning to work on that patch. Getting that patch approved is likely going to be a bikesheding exercise of epic proportions. It would be nice if GCC became more modular, but now I don't see that patch making a huge difference.
Comment 46 Jakub Jelinek 2013-03-22 14:44:46 UTC
GCC 4.8.0 is being released, adjusting target milestone.
Comment 47 Jakub Jelinek 2013-05-31 10:58:43 UTC
GCC 4.8.1 has been released.
Comment 48 Jakub Jelinek 2013-10-16 09:50:26 UTC
GCC 4.8.2 has been released.
Comment 49 Manuel López-Ibáñez 2016-01-26 04:17:18 UTC
(In reply to Jonathan Wakely from comment #13)
> Re Gaby's point about the error appearing in a sub-expression of a larger
> full expression, that's true

Now that the caret has been the default for several years and GCC 6 has range info thanks to dmalcom, this argument in favour of pretty-printing is not true anymore: %qE must die! :)