fix Issue 19785 - top level const types in function parameters should… #9556
Conversation
|
Thanks for your pull request, @WalterBright! Bugzilla references
Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub fetch digger
dub run digger -- build "master + dmd#9556" |
thewilsonator
added
the
Review:Needs Changelog
|
This is quite a serious change and needs a changelog. |
|
Not sure how serious this is. We don't offer any ABI compatibility. Anyone that expects that is going to be disappointed. |
WalterBright
force-pushed
the
fix19785
branch
2 times, most recently
from
9d2708b to
b8216e5
Compare
|
BTW, C++ does this, and so does D with |
WalterBright
force-pushed
the
fix19785
branch
2 times, most recently
from
a5d6aa1 to
dae9f87
Compare
changelog/fix19785.dd
Outdated
| Function Parameter Name Mangling Changes | ||
|
|
||
| If the parameter type can be implicitly converted to a type | ||
| that at the top level does not have `const` or `shared` or `inout` |
There was a problem hiding this comment.
Doesn't mention immutable.
changelog/fix19785.dd
Outdated
| then the converted type is mangled instead. This means | ||
| these functions mangle to the same string: | ||
|
|
||
| ``` |
There was a problem hiding this comment.
Please use ---. The markdown features are not enabled, as far as I know.
WalterBright
force-pushed
the
fix19785
branch
3 times, most recently
from
83992a6 to
52829f3
Compare
| This will break binary compatibility with existing code, | ||
| it will need to be recompiled. | ||
|
|
||
| It will also break code that relies on them being distinct |
| assert(foo60(k) == 3); | ||
| } | ||
| --- | ||
| Although arguably such code never made sense anyway. |
MSVC (stupidly) doesn't: https://godbolt.org/z/5P4vLv |
… not mangle as const
WalterBright
removed
the
Review:Needs Changelog
|
Although this is a better design, it turns out to require changing the implicit conversion rules for function overload matching, as shown in the This would be some significant work, and is a low priority, so I'm going to defer this for now. Note that the template9 failure would also likely exhibit if the functions were |
WalterBright
added
the
Review:Phantom Zone
| void foo(const(int)* p); | ||
| --- | ||
|
|
||
| Note that C++ compilers typically do this as well. |
There was a problem hiding this comment.
Not on Windows, e.g. void foo(const int* const ptr) is mangled as ?foo@@YAXQBH@Z by both cl and dmc. This translates back to void __cdecl foo(int const * const).
The change in this PR might help interfacing with void foo(const Class* ptr), though.
There was a problem hiding this comment.
DMC++ tries to duplicate what VC++ does for mangling. Where it would show up in semantics is how the two functions overload against each other. Obviously, if they mangle the same they have to be semantically regarded has having the same function ABI and cannot overload against each other.
… not mangle as const
If it doesn't break anything in the test suite, I'll need to add tests.