[Bug c++/86908] static_cast()->virtualMehod() calls base version of virtualMethod()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86908 --- Comment #9 from Jonathan Wakely --- (In reply to Kostya Frumkin from comment #8) > At the same time this design pattern (strategy) with placement-new is more > efficient than with common new. That's fine, but you can't use to access the AStrategy object you created at that position. You have several choices: - use an untyped buffer and create objects in there: alignas(AStrategy) unsigned char buf[sizeof(AStrategy)]; - use std::launder - use the pointer returned by the placement new-expression, which has the right type There are ways to do this safely in C++ today, but your original code is not one of those ways. You should fix your code. > So it has no rationale if the base methods > are used when using placement-new. At the same time other compilers > generates code where the derived method is callsed after placement-new. It's undefined behaviour, so anything can happen.
[Bug c++/86908] static_cast()->virtualMehod() calls base version of virtualMethod()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86908 --- Comment #8 from Kostya Frumkin --- > Either way you need to placement new the original type back again, otherwise > the wrong destructor gets called on scope exit, which adds more undefined > behaviour. There should not be undefined behavior because the size of both classes is same and destructors executes same code (in the case it is noop). So the memory to be freed up is same. The behavior must be defined by ISO committee because it is counterintuitive when the base methods are being called. (where to ask?) At the same time this design pattern (strategy) with placement-new is more efficient than with common new. So it has no rationale if the base methods are used when using placement-new. At the same time other compilers generates code where the derived method is callsed after placement-new. So I see two ways: - Ask the community to define behavior. or - Define behavior in gcc itself.
[Bug c++/86908] static_cast()->virtualMehod() calls base version of virtualMethod()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86908 --- Comment #7 from Jonathan Wakely --- (In reply to Richard Biener from comment #6) > (In reply to Jonathan Wakely from comment #1) > > No, your program has undefined behaviour. To make it valid you either need > > to use std::launder, or use the pointer that is returned by the placement > > new. > > > > The compiler is allowed to assume that the dynamic type of does > > not change (that's why you need to use std::launder). > > You need to use std::launder at every use of the object, right? Or just launder the pointer once and then use that: auto laundered = std::launder(); laundered->doIt(); // > I wonder > why static_cast() isn't a good enough "hint" here that > the type of strategy changed (from QOI perspective, std::launder is new). Casting from type X* to Y* where X and Y happen to be the same type can happen in generic code that was already written years ago, and doesn't play these sort of dirty tricks. It would be a shame to prevent useful optimisations because of a no-op cast. We don't need to make it easier to play nasty games with the type system. There are ways to do this sanely, without undefined behaviour, even without std::launder (just using the pointer returned by the placement new-expression should be good enough, instead of accessing through every time). > I guess a better testcase would be to declare strategy as AStrategy > and placement-new BaseStrategy over it or does the standard guaranteee > that AStrategy fits in the storage of BaseStrategy? For these types, with the Itanium ABI, it fits, and you could add a static_assert to verify it. Either way you need to placement new the original type back again, otherwise the wrong destructor gets called on scope exit, which adds more undefined behaviour.
[Bug c++/86908] static_cast()->virtualMehod() calls base version of virtualMethod()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86908 Richard Biener changed: What|Removed |Added CC||rguenth at gcc dot gnu.org --- Comment #6 from Richard Biener --- (In reply to Jonathan Wakely from comment #1) > No, your program has undefined behaviour. To make it valid you either need > to use std::launder, or use the pointer that is returned by the placement > new. > > The compiler is allowed to assume that the dynamic type of does > not change (that's why you need to use std::launder). You need to use std::launder at every use of the object, right? I wonder why static_cast() isn't a good enough "hint" here that the type of strategy changed (from QOI perspective, std::launder is new). I guess a better testcase would be to declare strategy as AStrategy and placement-new BaseStrategy over it or does the standard guaranteee that AStrategy fits in the storage of BaseStrategy?
[Bug c++/86908] static_cast()->virtualMehod() calls base version of virtualMethod()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86908 --- Comment #5 from Kostya Frumkin --- (In reply to Jonathan Wakely from comment #4) > (In reply to Kostya Frumkin from comment #3) > > Hi, for example msvc2013 calls base class's virtual method when msvc2015 > > calls derived class's virtual method. > > It's undefined behaviour. Anything can happen. > > > This is developer's mistake which can be predicted by compiler. Few > > developers know about this behavior. > > Using placement new to replace a polymorphic type is not a common idiom, I > don't think it's worth adding a warning about it. Just don't do it. > > > It'd be awesome to see the correct behavior or at least warning that base > > method is being used after placement new. > > GCC's behaviour is already correct. It is best way to get away with heap allocation in many cases. Let's make a warning about undefined behavior.
[Bug c++/86908] static_cast()->virtualMehod() calls base version of virtualMethod()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86908 --- Comment #4 from Jonathan Wakely --- (In reply to Kostya Frumkin from comment #3) > Hi, for example msvc2013 calls base class's virtual method when msvc2015 > calls derived class's virtual method. It's undefined behaviour. Anything can happen. > This is developer's mistake which can be predicted by compiler. Few > developers know about this behavior. Using placement new to replace a polymorphic type is not a common idiom, I don't think it's worth adding a warning about it. Just don't do it. > It'd be awesome to see the correct behavior or at least warning that base > method is being used after placement new. GCC's behaviour is already correct.
[Bug c++/86908] static_cast()->virtualMehod() calls base version of virtualMethod()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86908 Kostya Frumkin changed: What|Removed |Added Status|RESOLVED|UNCONFIRMED Version|unknown |9.0 Resolution|INVALID |--- --- Comment #3 from Kostya Frumkin --- (In reply to Jonathan Wakely from comment #1) > No, your program has undefined behaviour. To make it valid you either need > to use std::launder, or use the pointer that is returned by the placement > new. > > The compiler is allowed to assume that the dynamic type of does > not change (that's why you need to use std::launder). Hi, for example msvc2013 calls base class's virtual method when msvc2015 calls derived class's virtual method. This is developer's mistake which can be predicted by compiler. Few developers know about this behavior. It'd be awesome to see the correct behavior or at least warning that base method is being used after placement new.
[Bug c++/86908] static_cast()->virtualMehod() calls base version of virtualMethod()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86908 --- Comment #2 from Jonathan Wakely --- This makes the program correct: strategyPtr = new() AStrategy; static_cast(std::launder())->doIt(); strategyPtr->doIt();
[Bug c++/86908] static_cast()->virtualMehod() calls base version of virtualMethod()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86908 Jonathan Wakely changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution|--- |INVALID --- Comment #1 from Jonathan Wakely --- No, your program has undefined behaviour. To make it valid you either need to use std::launder, or use the pointer that is returned by the placement new. The compiler is allowed to assume that the dynamic type of does not change (that's why you need to use std::launder).
