0

I'm working in an old code where a pointer of an object of type A is passed to a function:

void fun(A* a)
{
    if (dynamic_cast<const B*>(a) != NULL)
    {
    // Use B object
    }
    else
    {
    // Use C object
    }

}

The classes B and C inherit from A and they kind of used dynamic_cast to test the input type (actually to test if "castable"). Which seems wrong.

I tried using std::is_same but I may be doing something wrong:

    if(std::is_same<decltype(*a), A>::value ) {
        std::cout << "Equal" << std::endl;
    }
    else
    {
        std::cout << "Not equal" << std::endl;
        std::cout << typeid(*a).name() << std::endl; // This
        std::cout << typeid(A).name() << std::endl;  // And this prints the same
    }

I always get into the "Not equal" case event if the following line print the same.

  1. Am I doing something wrong with std::is_same ?
  2. Is there another (better) way?
Ivan
  • 1,352
  • 2
  • 13
  • 31
  • 9
    `std::is_same` and `decltype(*a)` are evaluated at compile time, while `dynamic_cast` happens at runtime - so the behavior will definitely differ – UnholySheep Feb 15 '23 at 15:05
  • 3
    `a` is a `A*`, that means that *static type*, the type known at compile time, of `*a` is `A`, even if it happens to point to a `B` or `C`. Since it looks like your type is already polymorphic, the "better way" is to add a virtual function to the class hierarchy that "does the thing", and then you can just do `my_pointer_to_polymorphic_type->do_the_thing()` – NathanOliver Feb 15 '23 at 15:06
  • 6
    what's wrong with the original code? Except for the underlying design issue (which is not using virtual methods), and the use of NULL io nullptr? – Pepijn Kramer Feb 15 '23 at 15:07
  • @UnholySheep that explains a lot. I'm guessing there's a way to properly compare them in runtime? – Ivan Feb 15 '23 at 15:07
  • 3
    Checking if the dynamic cast is-or-is-not null is the way to do runtime type checking like this. It sucks, and is usually an antipattern, but if that's what your code base currently does you're probably going to create more bugs by trying to change this than you'd fix. The entire architecture around this will need a rewrite (to not depend on dynamic casting) and that might not be cost effective. – JohnFilleau Feb 15 '23 at 15:07
  • 1
    @JohnFilleau That describes better what I meant with "design issue" – Pepijn Kramer Feb 15 '23 at 15:08
  • 1
    @Ivan *"I'm guessing there's a way to properly compare them in runtime?"* the most proper way to check at runtime is how it's currently written. – JohnFilleau Feb 15 '23 at 15:08
  • 6
    With legacy code : if it isn't broken (and passes unit tests), and you don't need new features, leave it alone. In most cases a redesign to "modern" insights is needed to really make a step forward. – Pepijn Kramer Feb 15 '23 at 15:09
  • Most important takeaway, which I'm copy and pasting from Pepijn because of how important it is: **With legacy code : if it isn't broken (and passes unit test), and you don't need new features, leave it alone.** – JohnFilleau Feb 15 '23 at 15:10
  • 1
    @PepijnKramer true, but i'm dealing with this because unit tests stopped passing :D – Ivan Feb 15 '23 at 15:11
  • @DrewDormann valgrind gave me a `Conditional jump or move depends on uninitialised value(s)` here.I'm not sure that `dynamic_cast` is the problem , but i thought it was a clue. – Ivan Feb 15 '23 at 15:13
  • *"Conditional jump or move depends on uninitialised value(s)"* that sounds more like the function is being called with an uninitialized pointer. – JohnFilleau Feb 15 '23 at 15:15
  • @Ivan Ok good, you have unit tests ;) But what happened is there now a new class D in the inheritance tree? This code probably has been in for years. so is this the root cause? It might well be a change in the calling path. – Pepijn Kramer Feb 15 '23 at 15:16
  • @JohnFilleau yes, there are values initialized in `B`'s constructor, I thought maybe `dynamic_cast` didn't call it's constructor. I'm trying to find that out – Ivan Feb 15 '23 at 15:17
  • 2
    `dynamic_cast` will never call any constructor, because that's not what it is supposed to do – UnholySheep Feb 15 '23 at 15:19
  • @PepijnKramer we had a big update: we are now using more resent version of our compiler and more resent versions of the public libraries that we include. And yes the original code was written >10 years ago (before C++11) – Ivan Feb 15 '23 at 15:21
  • Untangling this _can_ be hard, but it is sometimes easy. In this case it looks like `fun` should actually be a `virtual` member function in a class derived from `A` and implemented in `B` and `C`. Use the current free function `fun` as a proxy for speedy implementation: [example](https://godbolt.org/z/cehbbPvcd) – Ted Lyngmo Feb 15 '23 at 15:28
  • `dynamic_cast(a)` will only return non-null if `a` points to a `B`. In the control block following this check, you are guaranteed that `a` points to a `B`. In the control block, are you accessing any non-virtual member functions of `A` that might be different in `B`? In that case, `a->some_function()` could be different than `dynamic_cast(a)->some_function()`. – JohnFilleau Feb 15 '23 at 15:29
  • @Ivan, I sometimes still work with code over 30 years old and yes a change of compiler could indeed show problems that where already there (but accidntally went right all the time). Have you checked if 'a' actually points to a valid object? – Pepijn Kramer Feb 15 '23 at 15:32
  • 1
    Compiler upgrades in our past where sometimes difficult like this. But over the years it seems to have gotten easier. A good idea, when you have the resources, is to setup more builds with new versions of tooling, parallel to your current development. That way you can "run into" these kind of issues before switching to your new toolchains. But that is more of a process thing. Usually fixes needed for a new compiler can be applied to the current codebase/compiler too so your current build benefits too. – Pepijn Kramer Feb 15 '23 at 15:37
  • @JohnFilleau I think I'm getting close, it's not `some_function` it's a member, in the "stuff" code. there's a quite dirty call to `float f = (float)((const C*)a)->m_float_member` and the problem is actuallly there... `m_float_member` is initialized in `C`'s contructor which is not called – Ivan Feb 15 '23 at 16:02

0 Answers0