Conversation
|
Thanks for your pull request and interest in making D better, @pbackus! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
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 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#10651" |
|
CC @atilaneves this is a new public symbol. |
|
Would |
|
I wouldn't call |
|
It's a function. You've turned a thunk into a function call... What's the objection to Walter's PR? This is pointless... it's kinda like |
Added
On record here: dlang/dmd#20644 (comment)
I guess this is just a matter of taste. I would almost always write |
|
Well you can push your thing as you like, but why stand in the way of Walter's PR? |
|
Also worth asking; who is this intended to be used by? Did anybody ever ask for this? |
|
Looks a lot nicer than the casts I've been littering my code with lately. Sounds good to me! |
I'm not "standing in the way" of anything. I'm offering my opinion on his PR and presenting an alternative. I certainly hope he'll take my feedback seriously, but at the end of the day, it's his call.
I could ask the same about If we decide |
Ok. This got me to do a double-take. How is writing |
Good point, maybe I know I'm nitpicking names, but I gave ya'll fair warning I was going to be like this with V3 on the way. |
Personally, I'd prefer something like
It's obtuse syntax. Granted, I don't think that
I'm the same way. And if someone feels that way about |
|
@jmdavis I didn't use |
|
Hmmm, |
Well me at least, and Walter it turns out.
This logic is backwards... it's definitely not worth adding a library function or I would have done that decades ago. |
<sigh> It's nice and clear, and it's an ugly cast by nature that shouldn't be done very often anyway.
I like it better than talking about type painting.
It's objectionable because it's not even vaguely clear what it means, whereas having a function allows us to have a clear name. This is one area where I think that C++ improved on C and where we arguably regressed in comparison to C++. We have a single cast operator that does everything and the kitchen sink, making it harder to know what's going on in some cases, whereas C++ has named cast operators where it's clear what they're actually doing. |
|
@LightBender I have no personal objection to The most concise name for this concept that I've been able to find prior usage of is Edit: "bit cast" is also used in Swift's Edit: also .NET's Edit: also Zig's Edit: also LLVM IR's |
IMHO that would be too generic. Actually I think paint is the worse part of the name.
Yeah, both sound better to me – |
I might be misinterpreting said statement (or missing something entirely), but I read that as a critique on the syntax rather than the name. |
|
@0xEAB I think when taken together with his next reply, it's fairly clear that his main objection is to the name, but he does have other reasons for favoring built-in syntax over library code: |
Well you left out But it's the additional function calls for trivia that annoy me. But conceptual issues aside, it's just a practical deal-breaker in a number of ways. Most importantly, you can't highlight the text and have your debugger pop-up the evaluation. Debuggers will evaluate expressions, but they won't run functions, so despite being a single opcode, using pointless functions terminate your ability to evaluate such trivial expressions in your debugger. It's just shit all round. " |
Sorry, what? I couldn't care less about the name. (although I do think it's shit 😝) (EDIT: Sorry; I just realised you meant Walter) |
@pbackus Thank you for doing this research legwork, this is exactly the kind of naming process I am hoping we can achieve throughout the Phobos 3 process. I posted a poll in Discord before I saw this. I'll let it run, but I suspect I know how this ends. |
|
For anyone curious about the assembly this generates, here's a godbolt link: https://d.godbolt.org/z/a4nco9sh7 TL;DR is that LDC produces identical assembly for both the built-in and library versions, even with no optimizations. GDC apparently doesn't respect |
|
These are practical issues; optimisation is irrelevant, and so is inlining. To make cast into a function is to ruin cast expressions in the debugger for everyone everywhere. (just like I have no idea why this thread even exists; |
@pbackus My interpretation of “The shorter the name, and the less ugly it is, the more it risks breaking code.” would be that we have to call it But I have to admit that I might still be overlooking something as I couldn’t find the other reference you’ve mentioned. Also I’m not a native English speaker |
You know what solution has exactly zero risk of breaking code, and also completely avoids this kind of bike-shedding? ;) |
AFAICT it was the very thing that triggered this “bike-shedding” :P |
It exists because other folks don't agree that And I doubt that most of the rest of us agree that a function is a problem in the way that you do. Having functions in a debugger is perfectly normal and not something that most of us worry about, and if you really want to use an explicit cast, nothing is stopping you from using the explicit cast right now, even if it's uglier, just like you can avoid writing out Of course, we can add the obtuse casting syntax and add the library function. Personally, even then, I'd favor the library function, because then you get a clear name. I very much doubt that I will remember |
No. I mean, if you're saying this is in response to my thing; then it's a complete waste of time... I'll never use it, it's just not fit for purpose. The weird thing is that you all suddenly feel so strongly and excited about this matter, why didn't any of you write this pointless function decades ago? |
well… https://github.com/adamdruppe/arsd/blob/bd502fb91204de400b46607702cbde222e7bd297/core.d#L184-L218 |
I've thought for a while now that we should add casting functions to Phobos like C++ has with its casts (maybe not the exact same set, but something similar). It would make casts clearer and less error-prone. It just hasn't yet been a high enough priority for me to sit down and work through and propose a set of such functions. Either way, I really don't want the built-in cast to become yet more complicated and confusing, and IMHO, |
'Shoehorn'? I really feel like a clearer syntax is literally impossible. There's definitely no 'shoehorn'-ing here, it's the most perfect representation of the expression that I think is possible. I suggested it because I thought it was super-elegant, and it turned out Walter was also instantly persuaded.
If, as you say, it "doesn't come up often"... then strictly speaking; this doesn't affect you.
See, I couldn't disagree more; I think it's the most self-evident expression possible. It does exactly what I would guess it does if I saw it in the wild having never seen it before. I don't see how any other interpretation is possible, and I can't imagine an alternative expression that would be clearer. This is DEFINITELY not clearer: But like you say, you will never encounter this, so it doesn't affect you. You might be forgiven for having no intuition around this, since it's not a part of your world-view. |
It does affect me. No, it's not something that I run into daily, but if anything, that makes it that much worse if it's using obtuse syntax, because then when I do run into it, I have to figure out what it's doing. With a named function, the code actually tells me what it's doing.
It's clearer insofar as it's possible to work through what it's doing, whereas with something like |
This seems to be veering very far off topic. If you want your special cast syntax, then write a DIP and implement it. |
|
It's not obtuse... it's the most logical and clear expression that I think is actually possible. That said, maybe it's worth the nit-pick that this isn't a 'reinterpret cast'; at least, not in C++ terms that I think you're all relating back to.
I mean, this pre-violates your proposition (you have to assume to understand every feature of the complex expression) in the very next sentence...
Oh, come on! That's true for LITERALLY EVERY SINGLE THING if you take a purist point of view. Otherwise you have to accept that you lean on associated knowledge of things in principle and then make some reasonable educated guess about what the conjunction of concepts could mean when encountering anything new. I feel pretty confident there's no other reasonable conclusion about what that expression could possibly mean besides what it actually means. I know you're going to resist that in principle, but in a world where this shitfight didn't exist and you just happened to stumble upon that expression in the wild by surprise and had to guess what it did (because you couldn't look it up), I have high confidence you would reach the proper conclusion, even though you don't want to admit it. I personally think it's blindingly obvious, but even if you don't find it obvious, I don't think a wrong conclusion is realistically possible. |
It's already done, and it's sitting there hanging. You downvoted it. |
void foo(ref int bar);
foo( 0x1337); // cannot pass rvalue argument `4919` of type `int` to parameter `ref int bar`
foo(cast(ref int) 0x1337); // ok |
There is no DIP to downvote. |
Huh? That would emit the same error as above for the same reason. |
Indeed, that’s the issue. Why have |
I don't understand what you're trying to say... are you somehow surprised that you can't convert an rvalue to ref? |
|
We have a visual pattern that doesn’t work out in practice. void foo1( int bar) {}
void foo2(ref int bar) {}
foo1(cast( int) 1.337);
foo2(cast(ref int) 1.337); // looks so similar, yet happens to be utter nonsenseEdit: Keep in mind, we’re talking about “and had to guess what it did”. |
|
Well your example won't compile... I still don't get what you're trying to show. |
|
We have an outlier here: void foo1(const int bar) {}
void foo2(ref int bar) {}
foo1(cast( int) 1.0);
foo1(cast(const int) 1.0);
foo1(cast(immutable int) 1.0);
foo2(cast(ref int) 1.0); // This syntax does something else.
If we were to have just to stumble upon that expression in the wild by surprise and had to guess what it did (because we couldn't look it up), syntactic sugar for promoting an rvalue to a referenceable lvalue the inline way would be a fair guess. |
It's certainly true that with anything new in the language, you have something new to learn and remember, but IMHO So, if it were my decision, I'd definitely decide against But whether it's added to the language or not, I think that adding a templated function to Phobos which handles it - and thus actually names what it's doing - is a good thing to do have. If anything, we should have that with more kinds of casts, since it would not only allow for greater clarity in code, but it would allow us to catch some of the mistakes that can happen with casts given how blunt they are (e.g. if we had a function which cast away |
|
|
I spoke with @WalterBright a few hours ago and in the time honored tradition of design by committee we arrived at the solution that gives everyone something to hate. We're going to get both. This function will be merged with whatever name we settle on. |
|
I support |
|
@pbackus all this needs is a change-log entry and name change and we're good to merge. |
This helper function is a more readable alternative to the traditional pointer-cast-based syntax for reinterpreting casts. The name bitCast is used for this operation in several other languages, including C++ [1], Swift [2], C# [3], Zig [4], and LLVM IR [5]. [1] https://en.cppreference.com/w/cpp/numeric/bit_cast [2] https://developer.apple.com/documentation/swift/unsafebitcast(_:to:) [3] https://learn.microsoft.com/en-us/dotnet/api/system.runtime.compilerservices.unsafe.bitcast [4] https://ziglang.org/documentation/0.10.0/#bitCast [5] https://llvm.org/docs/LangRef.html#bitcast-to-instruction
|
Renamed and added changelog. |
This helper function is a more readable alternative to the traditional pointer-cast-based syntax for reinterpreting casts.
The name bitCast is used for this operation in several other languages, including C++ [1], Swift [2], C# [3], Zig [4], and LLVM IR [5].
[1] https://en.cppreference.com/w/cpp/numeric/bit_cast
[2] https://developer.apple.com/documentation/swift/unsafebitcast(_:to:)
[3] https://learn.microsoft.com/en-us/dotnet/api/system.runtime.compilerservices.unsafe.bitcast
[4] https://ziglang.org/documentation/0.10.0/#bitCast
[5] https://llvm.org/docs/LangRef.html#bitcast-to-instruction
Alternative to dlang/dmd#20728For related discussion, see dlang/dmd#20644