[Bug c++/49152] New: Unhelpful diagnostic for iterator dereference

[Bug c++/49152] New: Unhelpful diagnostic for iterator dereference

[redi at gcc dot gnu.org]

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

           Summary: Unhelpful diagnostic for iterator dereference
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Keywords: diagnostic
          Severity: enhancement
          Priority: P3
         Component: c++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: redi@gcc.gnu.org


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

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[giecrilj at stegny dot 2a.pl]

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

Christopher Yeleighton <giecrilj at stegny dot 2a.pl> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |giecrilj at stegny dot
                   |                            |2a.pl

--- Comment #1 from Christopher Yeleighton <giecrilj at stegny dot 2a.pl> 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.

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[giecrilj at stegny dot 2a.pl]

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

--- Comment #2 from Christopher Yeleighton <giecrilj at stegny dot 2a.pl> 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)?

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[redi at gcc dot gnu.org]

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

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> 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

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[redi at gcc dot gnu.org]

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

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> 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))

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[manu at gcc dot gnu.org]

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

Manuel López-Ibáñez <manu at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |manu at gcc dot gnu.org

--- Comment #5 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 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?

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[redi at gcc dot gnu.org]

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

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> 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.

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[manu at gcc dot gnu.org]

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

--- Comment #7 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 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.

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[giecrilj at stegny dot 2a.pl]

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

--- Comment #8 from Christopher Yeleighton <giecrilj at stegny dot 2a.pl> 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))))’

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[manu at gcc dot gnu.org]

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

Manuel López-Ibáñez <manu at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-03-21
     Ever Confirmed|0                           |1

--- Comment #9 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-03-21 13:21:12 UTC ---
I think this is confirmed. I agree with the suggestion in comment #6.

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[manu at gcc dot gnu.org]

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

Manuel López-Ibáñez <manu at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |suckfish at ihug dot co.nz

--- Comment #10 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-03-21 13:21:53 UTC ---
*** Bug 25362 has been marked as a duplicate of this bug. ***

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[paolo.carlini at oracle dot com]

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

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|unassigned at gcc dot       |paolo.carlini at oracle dot
                   |gnu.org                     |com
   Target Milestone|---                         |4.8.0

--- Comment #11 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-03-22 00:00:20 UTC ---
On it.

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[paolo.carlini at oracle dot com]

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

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |NEW

--- Comment #12 from Paolo Carlini <paolo.carlini at oracle dot com> 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).

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[redi at gcc dot gnu.org]

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

--- Comment #13 from Jonathan Wakely <redi at gcc dot gnu.org> 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.

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[redi at gcc dot gnu.org]

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

--- Comment #14 from Jonathan Wakely <redi at gcc dot gnu.org> 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.

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[manu at gcc dot gnu.org]

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

--- Comment #15 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 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.

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[redi at gcc dot gnu.org]

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

--- Comment #16 from Jonathan Wakely <redi at gcc dot gnu.org> 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)

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[manu at gcc dot gnu.org]

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

--- Comment #17 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 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.

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[suckfish at ihug dot co.nz]

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

--- Comment #18 from Ralph Loader <suckfish at ihug dot co.nz> 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.

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[redi at gcc dot gnu.org]

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

--- Comment #19 from Jonathan Wakely <redi at gcc dot gnu.org> 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 ;)

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[suckfish at ihug dot co.nz]

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

--- Comment #20 from Ralph Loader <suckfish at ihug dot co.nz> 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.

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[manu at gcc dot gnu.org]

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

Manuel López-Ibáñez <manu at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jason at gcc dot gnu.org

--- Comment #21 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-03-24 16:32:07 UTC ---
Jason, do you have an opinion on this?

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[manu at gcc dot gnu.org]

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

--- Comment #22 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 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?

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[manu at gcc dot gnu.org]

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

--- Comment #23 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 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.

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[paolo.carlini at oracle dot com]

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

--- Comment #24 from Paolo Carlini <paolo.carlini at oracle dot com> 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).

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[paolo.carlini at oracle dot com]

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

--- Comment #25 from Paolo Carlini <paolo.carlini at oracle dot com> 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.

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[manu at gcc dot gnu.org]

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

--- Comment #26 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 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?

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[redi at gcc dot gnu.org]

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

--- Comment #27 from Jonathan Wakely <redi at gcc dot gnu.org> 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.

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[manu at gcc dot gnu.org]

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

--- Comment #28 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 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;
           ~~~~~~~~ ^ ~~

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[marc.glisse at normalesup dot org]

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

Marc Glisse <marc.glisse at normalesup dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |marc.glisse at normalesup
                   |                            |dot org

--- Comment #29 from Marc Glisse <marc.glisse at normalesup dot org> 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).

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[jason at gcc dot gnu.org]

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

--- Comment #30 from Jason Merrill <jason at gcc dot gnu.org> 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.

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[manu at gcc dot gnu.org]

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

--- Comment #31 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 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?

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[jason at gcc dot gnu.org]

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

--- Comment #32 from Jason Merrill <jason at gcc dot gnu.org> 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.

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[manu at gcc dot gnu.org]

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

--- Comment #33 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 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.

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[manu at gcc dot gnu.org]

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

--- Comment #34 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 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.

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[manu at gcc dot gnu.org]

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

--- Comment #35 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 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 .

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[pinskia at gmail dot com]

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

--- Comment #36 from pinskia at gmail dot com <pinskia at gmail dot 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?

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[jason at gcc dot gnu.org]

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

--- Comment #37 from Jason Merrill <jason at gcc dot gnu.org> 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.

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[pinskia at gcc dot gnu.org]

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

--- Comment #38 from Andrew Pinski <pinskia at gcc dot gnu.org> 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.

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[manu at gcc dot gnu.org]

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

Manuel López-Ibáñez <manu at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Depends on|                            |24985

--- Comment #39 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-04-04 15:27:35 UTC ---
Patch available for caret diagnostics in PR24985. It works!

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[paolo at gcc dot gnu.org]

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

--- Comment #40 from paolo at gcc dot gnu.org <paolo at gcc dot 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

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[paolo.carlini at oracle dot com]

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

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED
         AssignedTo|unassigned at gcc dot       |paolo.carlini at oracle dot
                   |gnu.org                     |com

--- Comment #41 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-04-16 15:36:07 UTC ---
Done.

[Bug c++/49152] Unhelpful diagnostic for iterator dereference

[redi at gcc dot gnu.org]

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

--- Comment #42 from Jonathan Wakely <redi at gcc dot gnu.org> 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 ;)

[Bug c++/49152] pretty printer cannot handle iterators and other complex expressions

[manu at gcc dot gnu.org]

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

Manuel López-Ibáñez <manu at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |
         AssignedTo|paolo.carlini at oracle dot |unassigned at gcc dot
                   |com                         |gnu.org
            Summary|Unhelpful diagnostic for    |pretty printer cannot
                   |iterator dereference        |handle iterators and other
                   |                            |complex expressions

--- Comment #43 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 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.)

[Bug c++/49152] pretty printer cannot handle iterators and other complex expressions

[joseph at codesourcery dot com]

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

--- Comment #44 from joseph at codesourcery dot com <joseph at codesourcery dot 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.

[Bug c++/49152] pretty printer cannot handle iterators and other complex expressions

[manu at gcc dot gnu.org]

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

--- Comment #45 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 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.

[Bug c++/49152] pretty printer cannot handle iterators and other complex expressions

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.8.0                       |4.8.1

--- Comment #46 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-03-22 14:44:46 UTC ---
GCC 4.8.0 is being released, adjusting target milestone.

[Bug c++/49152] pretty printer cannot handle iterators and other complex expressions

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.8.1                       |4.8.2

--- Comment #47 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 4.8.1 has been released.

[Bug c++/49152] pretty printer cannot handle iterators and other complex expressions

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.8.2                       |4.8.3

--- Comment #48 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 4.8.2 has been released.

[Bug c++/49152] pretty printer cannot handle iterators and other complex expressions

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.8.3                       |---