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

--- Comment #3 from Cassio Neri <cassio.neri at gmail dot com> ---
Because of the typeid check the unsafe static_cast never happens and I think
the compiler should not be warning about a problem that doesn't exist. Besides,
there's no array involved in this code. I appreciate the attempt to emit a good
warning that might improve my code but the message is completely misleading and
make me scratch my head. Here the code is minimal and obvious to figure out
that there's no array. In a large code base I could spend longtime looking for
an array that doesn't exist or I could find an array that has no issue but the
compiler makes me think it has.

Re using a dynamic_cast: I could surely use a dynamic_cast in real code but
this is a compiler test case. IMHO, it should be minimal, straight to the point
at the expense of neglecting other aspects of the language (e.g. better
practices) that could otherwise divert the attention. As I said the virtual
destructor in A and the typeid check were there to avoid obvious UB that would
happen had I unconditionally performed the static_cast. In that UB case
(provided the message were clearer and not misleading talking about arrays) I'd
be very grateful for getting the warning. 

Notice also that I provided a couple of changes that don't make the code any
better w.r.t. an unsafe static_cast (which, again, is never performed). These
changes make the spurious warning to go away (which is good) and this shows
that there's certainly something wrong with the logic that decides to emit the
warning for the code as originally posted.

What about this example which involves no virtual method and where dynamic_cast
cannot help?

struct A {
  int type;
};

struct C1 {
  int i;
  int j;
};

struct C2 {
  int i;
};

template <typename T, int i>
struct B : A {
  B() : A{i} {}
  T x;
};

using BC1 = B<C1, 1>;
using BC2 = B<C2, 2>;

void do_something(int);
BC2 get_BC();

void h(A& a) {
  if (a.type == 1) {
    BC1& b = static_cast<BC1&>(a);
    int i = b.x.i;
    do_something(i);
  }
  else if (a.type == 2) {
    BC2& b = static_cast<BC2&>(a);
    int i = b.x.i;
    do_something(i);
  }
}

void foo() {
  auto x = get_BC();
  h(x);
}

Here again, there are changes that make the code no better w.r.t. a potential
unsafe cast but do make the warning to go away:

1) Change the return type of get_BC to BC1.
2) Remove C1::j.
3) Remove the extra level of indirection given by template class B. (See [2].)

[1] Example above: https://godbolt.org/z/Tha3M6xq3
[2] Example with no template: https://godbolt.org/z/nWsPvTrYr

Reply via email to