Disallow dynamic cast to extern(C++) class

[ntrel]

trafficstars

See Issue 21690 - Unable to dynamic cast extern(C++) classes.

Instead, use cast(Derived)cast(void*)base.

[dlang-bot]

Thanks for your pull request and interest in making D better, @ntrel! We are looking forward to reviewing it, and you should be hearing from a maintainer soon. Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

⚠️⚠️⚠️ Warnings ⚠️⚠️⚠️

• In preparation for migrating from Bugzilla to GitHub Issues, the issue reference syntax has changed. Please add the word "Bugzilla" to issue references. For example, Fix Bugzilla Issue 12345 or Fix Bugzilla 12345.(Reminder: the edit needs to be done in the Git commit message, not the GitHub pull request.)

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#15293"

[Bolpat]

What does this PR do exactly? If it makes casting a C++ class impossible, there is no way to fix code. There must be a way to static cast C++ class objects.

[ntrel]

@Bolpat This deprecates casting any class instance to an extern(C++) class type. If the source expression is not a class it still works.

Edit: unless the target type is a base class of the source type.

[ntrel]

I haven't worked on finishing this as it seems quite disruptive given the NG thread about deprecating code that is working properly (even if it catches some code that isn't working). Is there a plan for changes like this - should they be delayed to a special release, or triggered by a preview switch instead?

[dkorpel]

Is there a plan for changes like this

It's still being discussed, but I think this PR is fine, since there's a good reason and a clear migration path.

[thewilsonator]

(please retitle the commit that fixes the issue "Fix bugzilla issue 23957 - Casting to derived extern(C++) class is unsafe")

[thewilsonator]

See also the deprecations:

src/dmd/asttypename.d(58): Deprecation: dynamic cast is not implemented for `extern(C++)` classes
src/dmd/asttypename.d(58):        use `*cast(dmd.expression.Expression*) &object` instead
src/dmd/asttypename.d(60): Deprecation: dynamic cast is not implemented for `extern(C++)` classes
src/dmd/asttypename.d(60):        use `*cast(dmd.dsymbol.Dsymbol*) &object` instead
src/dmd/asttypename.d(62): Deprecation: dynamic cast is not implemented for `extern(C++)` classes
src/dmd/asttypename.d(62):        use `*cast(dmd.mtype.Type*) &object` instead
src/dmd/asttypename.d(64): Deprecation: dynamic cast is not implemented for `extern(C++)` classes
src/dmd/asttypename.d(64):        use `*cast(dmd.mtype.Parameter*) &object` instead
src/dmd/asttypename.d(66): Deprecation: dynamic cast is not implemented for `extern(C++)` classes
src/dmd/asttypename.d(66):        use `*cast(dmd.statement.Statement*) &object` instead
src/dmd/asttypename.d(68): Deprecation: dynamic cast is not implemented for `extern(C++)` classes
src/dmd/asttypename.d(68):        use `*cast(dmd.cond.Condition*) &object` instead
src/dmd/asttypename.d(70): Deprecation: dynamic cast is not implemented for `extern(C++)` classes
src/dmd/asttypename.d(70):        use `*cast(dmd.dtemplate.TemplateParameter*) &object` instead
src/dmd/asttypename.d(72): Deprecation: dynamic cast is not implemented for `extern(C++)` classes
src/dmd/asttypename.d(72):        use `*cast(dmd.init.Initializer*) &object` instead
src/dmd/clone.d(60): Deprecation: dynamic cast is not implemented for `extern(C++)` classes
src/dmd/clone.d(60):        use `*cast(dmd.mtype.TypeFunction*) &object` instead
src/dmd/clone.d(180): Deprecation: dynamic cast is not implemented for `extern(C++)` classes
src/dmd/clone.d(180):        use `*cast(dmd.mtype.TypeStruct*) &object` instead
src/dmd/clone.d(286): Deprecation: dynamic cast is not implemented for `extern(C++)` classes
src/dmd/clone.d(286):        use `*cast(dmd.mtype.TypeStruct*) &object` instead
src/dmd/clone.d(317): Deprecation: dynamic cast is not implemented for `extern(C++)` classes
src/dmd/clone.d(317):        use `*cast(dmd.mtype.TypeFunction*) &object` instead
src/dmd/clone.d(438): Deprecation: dynamic cast is not implemented for `extern(C++)` classes
src/dmd/clone.d(438):        use `*cast(dmd.mtype.TypeStruct*) &object` instead
src/dmd/clone.d(566): Deprecation: dynamic cast is not implemented for `extern(C++)` classes
src/dmd/clone.d(566):        use `*cast(dmd.mtype.TypeFunction*) &object` instead
src/dmd/clone.d(641): Deprecation: dynamic cast is not implemented for `extern(C++)` classes
src/dmd/clone.d(641):        use `*cast(dmd.mtype.TypeFunction*) &object` instead
src/dmd/clone.d(765): Deprecation: dynamic cast is not implemented for `extern(C++)` classes
src/dmd/clone.d(765):        use `*cast(dmd.mtype.TypeStruct*) &object` instead
src/dmd/clone.d(806): Deprecation: dynamic cast is not implemented for `extern(C++)` classes
src/dmd/clone.d(806):        use `*cast(dmd.mtype.TypeFunction*) &object` instead
src/dmd/clone.d(906): Deprecation: dynamic cast is not implemented for `extern(C++)` classes
src/dmd/clone.d(906):        use `*cast(dmd.mtype.TypeStruct*) &object` instead
src/dmd/clone.d(1268): Deprecation: dynamic cast is not implemented for `extern(C++)` classes
src/dmd/clone.d(1268):        use `*cast(dmd.mtype.TypeStruct*) &object` instead
src/dmd/clone.d(1278): Deprecation: dynamic cast is not implemented for `extern(C++)` classes

[ntrel]

I think this PR is fine, since there's a good reason and a clear migration path.

Given that people are using these casts expecting them to work as C++ static_cast in working D code (including dmd), wouldn't it be better to specify that down casting extern(C++) classes is a type paint? Then we only need #15294 to make those not @safe.

It's also not ideal that the compiler has to recommend the type paint which only works with lvalue objects - so fixing rvalue expression casts is more awkward (though in practice, cast(Derived)cast(void*) rvalueExpression is often fine).