std.traits: Add support for 'in' storage class#7570
Conversation
|
Thanks for your pull request, @Geod24! Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. 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 "master + phobos#7570" |
6ef80e4 to
d66c05e
Compare
|
Can we get this merged ? Without the DMD PR it's harmless, so we can always revert it. |
For a long time, the 'in' storage class was only a second class citizen, merely an alias for 'const', which is a type constructor. While it was also documented for a long time as being 'scope', this was removed when 'scope' actually started to have an effect (when DIP1000 started to be implemented). Currently, a switch (-preview=in) allows to get back this 'scope'. However, the 'in' storage class does not really exists, it gets lowered to 'const [scope]' at an early stage by the compiler, which means that we expose what is essentially an implementation detail to the user. There is a PR in DMD (dlang/dmd#11474) that aims to give 'in' an actual identity, instead of it being lowered to 'const [scope]'. The underlying motivation is to allow for extending 'in''s functionality, giving it the ability to pass by 'ref' when necessary, and accept rvalues. However, regardless of the second goal, having proper support for 'in' would lead to less confusing messages, better code generation, and less confusion w.r.t. the behavior of `std.traits.ParameterStorageClass`.
| out_ = 0x04, /// ditto | ||
| lazy_ = 0x08, /// ditto | ||
| scope_ = 0x10, /// ditto | ||
| return_ = 0x20, /// ditto |
There was a problem hiding this comment.
@Geod24 @wilzbach @thewilsonator
Please next time consider that changing the values of enum members can have a large impact on end users, for no clear benefit. The value changes were not needed, just adding in_ = 0x20 would have sufficed, with less user pain. As a practical example, the reassigning of these values means that at Weka we will have to recalculate ABI hashes for many functions.
…ameters See related: dlang#7570
|
@Geod24 @wilzbach @thewilsonator none = 0,
scope_ = 1, /// ditto
out_ = 2, /// ditto
ref_ = 4, /// ditto
lazy_ = 8, /// ditto
return_ = 0x10, /// ditto
in_ = 0x20, /// ditto |
|
@JohanEngelen : We could change the order to that if that works better for you, I personally don't have a horse in this race, but wouldn't that potentially cause the same problems for other users now that this has been released for two releases ? |
|
I guess one can also argue that users shouldn't rely on the values of the enum, and only use the value identifiers... Afaik dlang gives no guarantees about ABI compatibility between versions (i.e. shared stdlibs will not work as one might hope). |
…ameters See related: dlang#7570
…ameters See related: dlang#7570
…ameters See related: #7570
In support of the DMD PR dlang/dmd#11474 , which will be red until this is pulled.
Commit message follows: