Bug 22238

Summary: Awful error messages with virtual functions
Product: gcc Reporter: Volker Reichelt <reichelt>
Component: c++Assignee: Not yet assigned to anyone <unassigned>
Status: NEW ---    
Severity: normal CC: gcc-bugs, jason, manu, pinskia, redi, webrown.cpp
Priority: P2 Keywords: diagnostic, monitored
Version: 4.1.0   
Target Milestone: ---   
Host: Target:
Build: Known to work:
Known to fail: Last reconfirmed: 2006-09-03 21:39:27
Attachments: Patch to fix the case in comment #12 (for next stage 1)

Description Volker Reichelt 2005-06-29 23:21:30 UTC
Since GCC 4.0.0 the compiler issues a hosed error message for the
following invalid code snippet:

======================================
struct A
{
  virtual void foo();
  void bar() { if (foo()) ; }
};
======================================

The error message reads:
  bug.cc: In member function 'void A::bar()':
  bug.cc:4: error: could not convert '#'obj_type_ref' not supported by
dump_expr#<expression error>(this)' to 'bool'

With GCC 3.4.4 I get
  bug.cc: In member function `void A::bar()':
  bug.cc:4: error: could not convert `(**((A*)this)->A::_vptr.A)(((A*)this))' to
`bool'
which is better, but not optimal.
Comment 1 Andrew Pinski 2005-06-30 02:00:07 UTC
Confirmed, caused by the patch which introduced obj_type_ref.  This also means we can produce better 
diagnostic.
Comment 2 Gabriel Dos Reis 2005-06-30 03:19:36 UTC
Subject: Re:  New: [4.0/4.1 regression] '#'obj_type_ref' not supported by dump_expr

"reichelt at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org> writes:

| Since GCC 4.0.0 the compiler issues a hosed error message for the
| following invalid code snippet:
| 
| ======================================
| struct A
| {
|   virtual void foo();
|   void bar() { if (foo()) ; }
| };
| ======================================
| 
| The error message reads:
|   bug.cc: In member function 'void A::bar()':
|   bug.cc:4: error: could not convert '#'obj_type_ref' not supported by
| dump_expr#<expression error>(this)' to 'bool'

I think I crossed this silly behaviour from the error.c several types,
but it was always in a bigger context.  Thanks for reducing it to this
small.  I'll fix it. 

| With GCC 3.4.4 I get
|   bug.cc: In member function `void A::bar()':
|   bug.cc:4: error: could not convert `(**((A*)this)->A::_vptr.A)(((A*)this))' to
| `bool'
| which is better, but not optimal.

Agreed.

-- Gaby
Comment 3 Gabriel Dos Reis 2005-06-30 03:19:58 UTC
Subject: Re:  [4.0/4.1 regression] '#'obj_type_ref' not supported by dump_expr

"pinskia at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org> writes:

| ------- Additional Comments From pinskia at gcc dot gnu dot org  2005-06-30 02:00 -------
| Confirmed, caused by the patch which introduced obj_type_ref.  This also means we can produce better 
| diagnostic.

indeed.  Assign it to me.

-- Gaby
Comment 4 Gabriel Dos Reis 2005-07-03 14:10:55 UTC
(In reply to comment #3)
> Subject: Re:  [4.0/4.1 regression] '#'obj_type_ref' not supported by dump_expr
> 
> "pinskia at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org> writes:
> 
> | ------- Additional Comments From pinskia at gcc dot gnu dot org  2005-06-30
02:00 -------
> | Confirmed, caused by the patch which introduced obj_type_ref.  This also
means we can produce better 
> | diagnostic.
> 
> indeed.  Assign it to me.
> 
> -- Gaby
> 


It's simple to move back to the awfull printing -- just add a case
for OBJ_TYPE_REF.
However, it is hard to fix this completely without, either
  (a) having the front-end stop generating low-level three nodes;
  (b) and/or moving to diagnostic with caret.

Adding the following to dump_expr():<case CALL_EXPR>

        if (TREE_CODE (fn) == OBJ_TYPE_REF)
          {
            tree obj_type = TREE_TYPE (OBJ_TYPE_REF_OBJECT (fn));
            int idx = tree_low_cst (OBJ_TYPE_REF_TOKEN (fn), 1);
            fn = BINFO_VIRTUALS (TYPE_BINFO (TREE_TYPE (obj_type)));
            while (idx--)
              fn = TREE_CHAIN (fn);
            fn = BV_FN (fn);
          }

yields

22238.C: In member function 'void A::bar()':
22238.C:4: error: could not convert 'A::foo()' to 'bool'

(while an improvement, it is still suboptimal).  Now, if you slightly
change the testcase to

struct A
{
   virtual void foo();
};

struct B : virtual A {
   void bar() { if (foo()) ; }
};


then you would get the awful

3.C: In member function 'void B::bar()':
3.C:7: error: could not convert '(((A*)this) +
(*(int*)(((B*)this)->B::<anonymous>.A::_vptr.A + -0x000000010)))->A::foo()' to
'bool'
Comment 5 Andrew Pinski 2005-10-13 19:55:50 UTC
Any news on getting this partial patch committed, as this will at least bring us back to pre 4.0 times.
Comment 6 Mark Mitchell 2005-10-31 03:55:56 UTC
Gaby, please apply the simple OBJ_TYPE_REF patch so that we can remove the regression markers from this PR.

(I agree that a complete solution is difficult; my opinion continues to be that we should use carets, rather than trying to print out expressions.)
Comment 7 Gabriel Dos Reis 2005-11-16 19:05:31 UTC
Subject: Re:  [4.0/4.1 regression] '#'obj_type_ref' not supported by dump_expr

"mmitchel at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org> writes:

| Gaby, please apply the simple OBJ_TYPE_REF patch so that we can remove the
| regression markers from this PR.

OK.

-- Gaby
Comment 8 Gabriel Dos Reis 2005-11-22 17:04:17 UTC
Subject: Bug 22238

Author: gdr
Date: Tue Nov 22 17:04:12 2005
New Revision: 107366

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=107366
Log:
        PR c++/22238
        * error.c (resolve_virtual_fun_from_obj_type_ref): New.
        (dump_expr): Use it in <case CALL_EXPR>.

Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/error.c

Comment 9 Gabriel Dos Reis 2005-11-22 18:08:24 UTC
Subject: Bug 22238

Author: gdr
Date: Tue Nov 22 18:08:17 2005
New Revision: 107367

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=107367
Log:
        PR c++/22238
        * error.c (resolve_virtual_fun_from_obj_type_ref): New.
        (dump_expr): Use it in <case CALL_EXPR>.

Modified:
    branches/gcc-4_1-branch/gcc/cp/ChangeLog
    branches/gcc-4_1-branch/gcc/cp/error.c

Comment 10 Gabriel Dos Reis 2005-11-22 22:27:58 UTC
Subject: Bug 22238

Author: gdr
Date: Tue Nov 22 22:27:52 2005
New Revision: 107376

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=107376
Log:
        PR c++/22238
        * error.c (resolve_virtual_fun_from_obj_type_ref): New.
        (dump_expr): Use it in <case CALL_EXPR>.

Modified:
    branches/gcc-4_0-branch/gcc/cp/ChangeLog
    branches/gcc-4_0-branch/gcc/cp/error.c

Comment 11 Gabriel Dos Reis 2005-11-22 23:20:04 UTC
No longer a regression.
Set bug back at "normal" state -- no milestone.
Comment 12 Volker Reichelt 2007-06-17 23:40:32 UTC
Andrew, Gaby's testcase from comment #4 gets an even worse diagnostic
since the merge of the ptr_plus stuff:

=========================================
struct A
{
    void foo();
};

struct B : virtual A
{
    void bar() { if (foo()) ; }
};
=========================================

bug.cc: In member function 'void B::bar()':
bug.cc:8: error: could not convert '#'pointer_plus_expr' not supported by dump_expr#<expression error>->A::foo()' to 'bool'

Would you mind having a look?
Comment 13 Andrew Pinski 2007-06-17 23:42:58 UTC
>Would you mind having a look?

I was going to fix the pretty printing of pointer_plus_expr for C++ after I returned from Japan.  I already have a testcase which is better than the one here.
Comment 14 Volker Reichelt 2007-08-21 19:56:18 UTC
The pointer_plus_exprt stuff has been fixed.

We are now back to error messages like

bug.cc: In member function 'void B::bar()':
bug.cc:4: error: could not convert '(((A*)this) + ((unsigned int)(*(int*)(
((B*)this)->B::_vptr.B + 0xfffffffffffffff4u))))->A::foo()' to 'bool'
Comment 15 gdr@cs.tamu.edu 2007-08-21 20:27:26 UTC
Subject: Re:  Awful error messages with virtual functions

"reichelt at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org> writes:

| The pointer_plus_exprt stuff has been fixed.
| 
| We are now back to error messages like
| 
| bug.cc: In member function 'void B::bar()':
| bug.cc:4: error: could not convert '(((A*)this) + ((unsigned int)(*(int*)(
| ((B*)this)->B::_vptr.B + 0xfffffffffffffff4u))))->A::foo()' to 'bool'

Until we have a separate higher level representation in the C++
front-end, this is going to be hard to fix correctly.  The alternative
is that we don't print expressions at all -- but that cure seems to me
to be worse than the disease.

-- Gaby
Comment 16 Manuel López-Ibáñez 2011-12-01 17:27:01 UTC
I think this is a duplicate of another PR I cannot find, but everything boils down to "please kill %qE and stop pretty-printing expressions". Behold the beauty of clang:

pr22238.cc:8:20: error: value of type 'void' is not contextually convertible to 'bool'
  void bar() { if (foo()) ; }
                   ^~~~~
1 error generated. 

(it looks even prettier with the default colors!)
Comment 17 Jonathan Wakely 2011-12-01 17:39:24 UTC
(In reply to comment #16)
> I think this is a duplicate of another PR I cannot find

PR 50817 ?
Comment 18 Manuel López-Ibáñez 2011-12-01 17:50:25 UTC
*** Bug 50817 has been marked as a duplicate of this bug. ***
Comment 19 David Malcolm 2016-01-27 18:34:07 UTC
gcc trunk (r232888 fwiw) currently emits this for comment #0:

/tmp/test.cc: In member function ‘void A::bar()’:
/tmp/test.cc:4:23: error: could not convert ‘A::foo()’ from ‘void’ to ‘bool’
   void bar() { if (foo()) ; }
                    ~~~^~

and this for the dup reported in PR 50817:

/tmp/test2.cc: In function ‘void test(foo*)’:
/tmp/test2.cc:9:21: error: no match for ‘operator+’ (operand types are ‘int’ and ‘foo’)
     return P->bar() + *P;
            ~~~~~~~~~^~~~
/tmp/test2.cc:9:24: error: return-statement with a value, in function returning 'void' [-fpermissive]
     return P->bar() + *P;
                        ^

Is it time to close this one out as fixed?
Comment 20 Manuel López-Ibáñez 2016-01-27 19:02:23 UTC
(In reply to David Malcolm from comment #19)
> Is it time to close this one out as fixed?

with gcc HEAD 6.0.0 20160127 and the testcase in comment #12, I get:

prog.cc: In member function 'void B::bar()':
prog.cc:8:25: error: could not convert '(((A*)this) + ((sizetype)(*(long int*)(((B*)this)->B::_vptr.B + 18446744073709551592u))))->A::foo()' from 'void' to 'bool'
     void bar() { if (foo()) ; }
                      ~~~^~

Clang:

prog.cc:8:22: error: value of type 'void' is not contextually convertible to 'bool'
    void bar() { if (foo()) ; }
                     ^~~~~

I maintain my opinion that any user-facing diagnostic using %qE is potentially broken.
Comment 21 Manuel López-Ibáñez 2016-01-27 19:14:17 UTC
(In reply to David Malcolm from comment #19)
> /tmp/test2.cc:9:24: error: return-statement with a value, in function
> returning 'void' [-fpermissive]
>      return P->bar() + *P;
>                         ^

And that location is terrible, but that is a different issue.
Comment 22 David Malcolm 2016-01-27 21:58:37 UTC
(In reply to Manuel López-Ibáñez from comment #20)
[...]
> I maintain my opinion that any user-facing diagnostic using %qE is
> potentially broken.

Thanks; I'm inclined to agree.

Notes to self: implementation of %qE is in cp/error.c:cp_printer, which calls expr_to_string.

$ grep "%qE" gcc/cp/*.c | wc -l
195
Comment 23 David Malcolm 2016-01-27 22:00:15 UTC
Created attachment 37496 [details]
Patch to fix the case in comment #12 (for next stage 1)
Comment 24 AK 2018-02-19 06:56:26 UTC
The recent error messages look much better. Maybe we can close this.

prog.cpp: In member function ‘void A::bar()’:
prog.cpp:6:23: error: could not convert ‘A::foo()’ from ‘void’ to ‘bool’
   void bar() { if (foo()) ; }
Comment 25 Jonathan Wakely 2018-02-19 15:39:27 UTC
(In reply to AK from comment #24)
> The recent error messages look much better. Maybe we can close this.
> 
> prog.cpp: In member function ‘void A::bar()’:
> prog.cpp:6:23: error: could not convert ‘A::foo()’ from ‘void’ to ‘bool’
>    void bar() { if (foo()) ; }

There are other issues being tracked here, like the one in comment 12.