core.stdcpp.new_.cpp_delete unnecessarily requires destruction to be @nogc #3284
Conversation
|
Thanks for your pull request, @n8sh! Bugzilla references
Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "stable + druntime#3284" |
| @@ -14,8 +14,6 @@ module core.stdcpp.new_; | |||
| import core.stdcpp.xutility : __cpp_sized_deallocation, __cpp_aligned_new; | |||
| import core.stdcpp.exception : exception; | |||
|
|
|||
| @nogc: | |||
There was a problem hiding this comment.
Every function between here and where @nogc: is re-added is either a template or explicitly annotated @nogc.
|
Is the parallel for construction also addressed here? |
|
I'd like to see a unit test that demonstrates use with GC and also |
Thanks but this wasn't a "catch" so much as something stumbled across naturally while trying to mix C++ and D! |
|
Same thing. It's great that you're using this. |
| { | ||
| __gshared ulong numDeleted; | ||
| int x = 3; | ||
| ~this() @nogc { ++numDeleted; } |
There was a problem hiding this comment.
I feel like this class should test @nogc constructors too.
test/stdcpp/src/new_test.d
Outdated
| { | ||
| __gshared ulong numDeleted; | ||
| int x = 5; | ||
| ~this() { if (++numDeleted < 0) throw new Exception("overflow"); } |
There was a problem hiding this comment.
No with-GC constructor test?
There was a problem hiding this comment.
It should probably new something in the constructor...
| MyClassNoGC c1 = cpp_new!MyClassNoGC(4); | ||
| assert(c1.x == 4); | ||
| assert(MyClassNoGC.numDeleted == 0); | ||
| cpp_delete(c1); |
There was a problem hiding this comment.
Seems this doesn't infer @nogc. I guess destroy is not inferring correctly?
n8sh
force-pushed
the
issue-21417
branch
2 times, most recently
from
c7cdab6 to
440b1fd
Compare
test/stdcpp/src/new_test.d
Outdated
| @nogc unittest | ||
| { | ||
| // TEST NOT COMPILING: compiler can't tell dtor of MyClassNoGC is @nogc. | ||
| if (0) | ||
| MyClassNoGC.init.__xdtor(); | ||
| } |
There was a problem hiding this comment.
The problem wasn't in destroy it's here:
test\stdcpp\src\new_test.d(48): Error: @nogc function new_test.__unittest_L44_C7 cannot call non-@nogc destructor new_test.MyClassNoGC.~thisThere was a problem hiding this comment.
__xdtor is not the destructor; that's actually a function synthesised by the compiler.
Maybe there's a compiler bug where that function should be inferring some attributes but it's not?
It will also execute all the field destructors, and the super destructor, so if any of those are not @nogc, then that may be breaking the inference...?
…destruction to be `@nogc`
|
Updated tests to test |
No description provided.