Bug 52321 - poor diagnostic of invalid cast
Summary: poor diagnostic of invalid cast
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 4.6.1
: P3 enhancement
Target Milestone: 9.0
Assignee: Jonathan Wakely
URL:
Keywords: diagnostic, patch
: 65293 88503 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-02-21 01:14 UTC by Ivan Godard
Modified: 2018-12-17 21:50 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2018-12-17 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ivan Godard 2012-02-21 01:14:09 UTC
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.
Comment 1 Jonathan Wakely 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.
Comment 2 Ivan Godard 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.
Comment 3 Jonathan Wakely 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.
Comment 4 Ivan Godard 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?
Comment 5 Manuel López-Ibáñez 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.
Comment 6 Jonathan Wakely 2015-03-03 10:19:21 UTC
*** Bug 65293 has been marked as a duplicate of this bug. ***
Comment 7 Jonathan Wakely 2018-12-17 13:44:46 UTC
*** Bug 88503 has been marked as a duplicate of this bug. ***
Comment 8 Jonathan Wakely 2018-12-17 13:47:08 UTC
Clang outputs an extra line saying the type is incomplete (which should probably be a "note:" but nevermind):

53231.cc:5:17: error: static_cast from 'foo *' to 'bar *', which are not related by inheritance, is not allowed
       bar* b = static_cast<bar*>(f);
                ^~~~~~~~~~~~~~~~~~~~
53231.cc:1:7: note: 'foo' is incomplete
class foo;
      ^
1 error generated.


When both types are incomplete it says that too:

88503.cc:6:16: error: static_cast from 'Parent *' to 'Derived *', which are not related by inheritance, is not allowed
        return static_cast<Derived*>(p);
               ^~~~~~~~~~~~~~~~~~~~~~~~
88503.cc:2:7: note: 'Derived' is incomplete
class Derived;
      ^
88503.cc:1:7: note: 'Parent' is incomplete
class Parent;
      ^
1 error generated.
Comment 9 Jonathan Wakely 2018-12-17 13:50:05 UTC
(In reply to Jonathan Wakely from comment #8)
> Clang outputs an extra line saying the type is incomplete (which should
> probably be a "note:" but nevermind):

Ha, it is a note, but on my terminal the "note:" part is shown as black text on a black background. It only showed up when I pasted it in here!
Comment 10 Jonathan Wakely 2018-12-17 14:17:45 UTC
Untested patch:

--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -7348,8 +7348,19 @@ build_static_cast (tree type, tree oexpr, tsubst_flags_t complain)
     }
 
   if (complain & tf_error)
-    error ("invalid static_cast from type %qT to type %qT",
-           TREE_TYPE (expr), type);
+    {
+      error ("invalid static_cast from type %qT to type %qT",
+            TREE_TYPE (expr), type);
+      if ((TYPE_PTR_P (type) || TYPE_REF_P (type))
+         || !COMPLETE_TYPE_P (TREE_TYPE (type)))
+       inform (DECL_SOURCE_LOCATION (TYPE_MAIN_DECL (TREE_TYPE (type))),
+               "class type %qT is incomplete", TREE_TYPE (type));
+      tree expr_type = TREE_TYPE (expr);
+      if ((TYPE_PTR_P (expr_type) || TYPE_REF_P (expr_type))
+         || !COMPLETE_TYPE_P (TREE_TYPE (expr_type)))
+       inform (DECL_SOURCE_LOCATION (TYPE_MAIN_DECL (TREE_TYPE (expr_type))),
+               "class type %qT is incomplete", TREE_TYPE (expr_type));
+    }
   return error_mark_node;
 }
 

For the original example this produces:

53231.cc: In function 'int main()':
53231.cc:5:31: error: invalid static_cast from type 'foo*' to type 'bar*'
    5 |   bar* b = static_cast<bar*>(f);
      |                               ^
53231.cc:1:7: note: class type 'foo' is incomplete
    1 | class foo;
      |       ^~~


And for the example from Bug 88503:

88503.cc: In function 'Derived* foo(Parent*)':
88503.cc:6:39: error: invalid static_cast from type 'Parent*' to type 'Derived*'
    6 |         return static_cast<Derived*>(p);
      |                                       ^
88503.cc:2:7: note: class type 'Derived' is incomplete
    2 | class Derived;
      |       ^~~~~~~
88503.cc:1:7: note: class type 'Parent' is incomplete
    1 | class Parent;
      |       ^~~~~~
Comment 11 Jonathan Wakely 2018-12-17 15:38:43 UTC
Patch tested and posted to https://gcc.gnu.org/ml/gcc-patches/2018-12/msg01228.html
Comment 12 Jonathan Wakely 2018-12-17 15:40:51 UTC
(In reply to Ivan Godard from 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.

A much simpler "good enough" solution is to just leave build_static_cast_1 alone. In the caller, if the cast fails and one or both types is a pointer/reference to incomplete class, issue a note. It doesn't matter if the reason it failed is the incomplete type, because I don't try to say that, I just say it's incomplete.
Comment 13 Jonathan Wakely 2018-12-17 21:50:14 UTC
Fixed for gcc 9.
Comment 14 Jonathan Wakely 2018-12-17 21:50:30 UTC
Author: redi
Date: Mon Dec 17 21:49:58 2018
New Revision: 267219

URL: https://gcc.gnu.org/viewcvs?rev=267219&root=gcc&view=rev
Log:
PR c++/52321 print note for static_cast to/from incomplete type

	PR c++/52321
	* typeck.c (build_static_cast): Print a note when the destination
	type or the operand is a pointer/reference to incomplete class type.

Added:
    trunk/gcc/testsuite/g++.dg/expr/static_cast8.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/typeck.c