[Bug c++/52321] New: poor diagnostic of invalid cast

[Bug c++/52321] New: poor diagnostic of invalid cast

[igodard at pacbell dot net]

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

             Bug #: 52321
           Summary: poor diagnostic of invalid cast
    Classification: Unclassified
           Product: gcc
           Version: 4.6.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: igodard@pacbell.net


This code:
   class foo;
   class bar {};
   int main() {
       foo* f;
       bar* b = static_cast<bar*>(f);
       return 0;
       }

gets you this:

   s3:~/ootbc/sim$ g++ foo.cc
   foo.cc: In function âint main()â:
   foo.cc:5:30: error: invalid static_cast from type âfoo*â to type âbar*â

The issued diagnostic is correct but not very helpful. The real bug is that
class foo is defined (somewhere else) as class foo : public bar {...}, but the
definitions wasn't imported. A diagnostic like:
   foo.cc: In function âint main()â:
   foo.cc:6:3: error: invalid use of incomplete type âstruct fooâ
   foo.cc:1:7: error: forward declaration of âstruct fooâ
(which is what you put out for -> on an undefined) would be *much* better.

[Bug c++/52321] poor diagnostic of invalid cast

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |diagnostic
           Severity|normal                      |enhancement

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-02-21 11:08:07 UTC ---
Comeau online gives:
"ComeauTest.c", line 5: error: invalid type conversion

Clang gives:
l.cc:5:17: error: static_cast from 'foo *' to 'bar *' is not allowed

So G++ is no worse at least.

It wouldn't be as simple as just checking if the operand is a pointer/reference
to incomplete type when a static_cast fails, e.g. this fails because of casting
away const, not because the type is incomplete:

 class foo;
 int main() {
   const foo* f;
   static_cast<void*>(f);
 }

There are lots of reasons a static_cast could be invalid (inaccessible bases,
virtual bases, casting away const etc. and that's just for casting Class1* to
Class2*) so if the diagnostic is improved it should cover more than just
casting from a pointer/reference to (possibly cv-qualified) incomplete type.

[Bug c++/52321] poor diagnostic of invalid cast

[igodard at pacbell dot net]

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

--- Comment #2 from Ivan Godard <igodard at pacbell dot net> 2012-02-21 15:30:42 UTC ---
Somewhere there's an attept to coerce a to b that sees the source is a class
and the target is a class and tries to see if the source is derived from
target. That check fails because source is undefined, and the failure
propagates out as other possible casts are tried. If you save the reason for
failure your "can't cast" message can look at the reason. The message could
expand on other possible reasons too.

Just a suggestion - that's how we did it in the Mary compilers, and gave a list
of the plausible reasons for failure.

[Bug c++/52321] poor diagnostic of invalid cast

[redi at gcc dot gnu.org]

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

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-02-21 15:53:10 UTC ---
Yep, it's build_static_cast_1 in typeck.c

But currently that has no way to store or pass back a message (just a boolean
indicating success or failure and the result of the cast) and would need a lot
of restructuring.

if (target is pointer or reference
    && source is class type
    && target is class type
    && (target is rvalue || source is lvalue)
    && target is derived from source
    && can convert
    && at least as qualified)

Your example fails the "target is derived from source" test, but that test
doesn't say it failed because the type is incomplete, it just returns false. 
And the "can convert" step might fail for a number of reasons, but again it
just returns a boolean.

Modifying that to report different reasons (rather than just evaluating to
'false') is not simple.

[Bug c++/52321] poor diagnostic of invalid cast

[igodard at pacbell dot net]

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

--- Comment #4 from Ivan Godard <igodard at pacbell dot net> 2012-02-21 17:38:30 UTC ---
Define an enum of reasons with "success" first, flop the sense of the test so
that false means coercion was OK (grep to find all calls and put a "!" in front
of each), and return the reason enum instead of bool. The code that is
reason-aware saves the enum and builds a good message; the legacy code that is
not reason-aware treats the enum as a bool and works as before except for the
inverted sense of the test. Maybe half an hour of work.

Plausible?

[Bug c++/52321] poor diagnostic of invalid cast

[manu at gcc dot gnu.org]

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

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> 2012-02-22 15:58:26 UTC ---
(In reply to comment #4)
> Define an enum of reasons with "success" first, flop the sense of the test so
> that false means coercion was OK (grep to find all calls and put a "!" in front
> of each), and return the reason enum instead of bool. The code that is
> reason-aware saves the enum and builds a good message; the legacy code that is
> not reason-aware treats the enum as a bool and works as before except for the
> inverted sense of the test. Maybe half an hour of work.
> 
> Plausible?

Plausible in theory, sadly unrealistic in practice. But I would like to be
proven wrong, so give it a try.

[Bug c++/52321] poor diagnostic of invalid cast

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |chengniansun at gmail dot com

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
*** Bug 65293 has been marked as a duplicate of this bug. ***