Recently I was looking in the code of an open source project, and I saw a bunch of statements of the form T & object = *dynamic_cast<T*>(ptr).
(Actually this was occuring in macro used to declare many functions following a similar pattern.)
To me this looked like a code-smell. My reasoning was, if you know the cast will succeed, then why not use a static_cast? If you aren't sure, then shouldn't you use an assert to test? Since the compiler can assume that any pointer that you * is not null.
I asked one of the devs on irc about it, and he said that, he considers static_cast downcast to be unsafe. They could add an assert, but even if they don't, he says you will still get a null pointer dereference and crash when obj is actually used. (Because, on failure, the dynamic_cast will convert the pointer to null, then when you access any member, you will be reading from some address of value very close to zero, which the OS won't allow.) If you use a static_cast, and it goes bad, you might just get some memory corruption. So by using the *dynamic_cast option, you are trading off speed for slightly better debuggability. You aren't paying for the assert, instead you are basically relying on the OS to catch the nullptr dereference, at least that's what I understood.
I accepted that explanation at the time, but it bothered me and I thought about it some more.
Here's my reasoning.
If I understand the standard right, a static_cast pointer cast basically means to do some fixed pointer arithmetic. That is, if I have A * a, and I static cast it to a related type B *, what the compiler is actually going to do with that is add some offset to the pointer, the offset depending only on the layout of the types A, B, (and which C++ implementation potentially). This theory can be tested by static casting pointers to void * and outputting them, before and after the static cast. I expect that if you look at the generated assembly, the static_cast will turn into "add some fixed constant to the register corresponding to the pointer."
A dynamic_cast pointer cast means, first check the RTTI and only do the static cast if it is valid based on the dynamic type. If it is not, then return nullptr. So, I'd expect that the compiler will at some point expand an expresion dynamic_cast<B*>(ptr) where ptr is of type A* into an expression like
(__validate_dynamic_cast_A_to_B(ptr) ? static_cast<B*>(ptr) : nullptr)
However, if we then * the result of the dynamic_cast, * of nullptr is UB, so we are implicitly promising that the nullptr branch never happens. And conforming compilers are permitted to "reason backwards" from that and eliminate null checks, a point driven home in Chris Lattner's famous blog post.
If the test function __validate_dynamic_cast_A_to_B(ptr) is opaque to the optimizer, i.e. it might have side effects, then the optimizer can't get rid of it, even if it "knows" the nullptr branch doesn't happen. However, probably this function is not opaque to the optimizer -- probably it has a very good understanding of its possible side effects.
So, my expectation is that the optimizer will essentially convert *dynamic_cast<T*>(ptr) into *static_cast<T*>(ptr), and that interchanging these should give the same generated assembly.
If true, that would justify my original argument that *dynamic_cast<T*> is a code smell, even if you don't really care about UB in your code and only care about what "actually" happens. Because, if a conforming compiler would be permitted to change it to a static_cast silently, then you aren't getting any safety that you think you are, so you should either explicitly static_cast or explicitly assert. At least, that would be my vote in a code review. I'm trying to figure out if that argument is actually right.
Aucun commentaire:
Enregistrer un commentaire