dmd
dmd copied to clipboard
Fix issue 22550 - tail const C++ class not usable on Windows
This changes the C++ mangling of const(Class) from "Class const * const" to "Class const *" for Windows, but only if the class type is used at the top of a type. It is a breaking change for code using "Class const * const" in C++, but this code seems to be less common.
Thanks for your pull request and interest in making D better, @tim-dlang! 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:
andReturns:
)
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
Auto-close | Bugzilla | Severity | Description |
---|---|---|---|
✓ | 22550 | normal | tail const C++ class not usable on Windows |
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#13369"
See also issue 21200 and the discussion at https://digitalmars.com/d/archives/digitalmars/D/const_Class_is_mangled_as_Class_const_const_299139.html.
Unfortunately this could also affect the D compilers, because DsymbolTable.lookup and DsymbolTable.insert use "Identifier const * const". The change could make bootstrapping harder.
Header generation would also need to be changed.
What's the behaviour on itanium? Windows should just mangle it in the same way instead of doing something different.
What's the behaviour on itanium? Windows should just mangle it in the same way instead of doing something different.
The outer const is already removed for itanium. See https://github.com/dlang/dmd/blob/master/src/dmd/cppmangle.d#L1464-L1465
But there seems to be a difference between GCC on Linux and Visual C++ on Windows: GCC mangles "void test(const C *c):" and "void test(const C *const c)" the same. Visual C++ uses a different mangling for them.
This seems like a major language change here. cc @WalterBright
While implementing header generation for the changed types, I realised that this change would break more code, than I initially thought. Previously I only thought about bindings to existing C++ libraries, but extern(C++) can also be used for D libraries, which are used from C++ (like the DMD frontend). Those C++ headers for D libraries already use declarations with full const and would break with this change.
To prevent breaking existing code, I'm now thinking about making the change optional. This could be done with a compiler recognized UDA, but that would also be limited to class types used directly as return or parameter types. A more general solution would be an extension of the type system. A new trait could be used to create tail const class types, like __trait(headMutable, const(C)). Allowing tail const/immutable/shared for classes has also been proposed in issue 5325 and could replace most uses of std.typecons.Rebindable. The semantics of tail const class types should match tail const pointers to structs. Would such a change require a DIP?