Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add trait_upcasting related languages changes #1622
base: master
Are you sure you want to change the base?
Add trait_upcasting related languages changes #1622
Changes from all commits
25e9e65
5e57afc
7451a0e
a2624a2
6c4f538
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does
as
not permit upcasting?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as
permits upcasting.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(more clarity, same meaning)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me your proposed change kinda flushes the cache, I had to re-read it 4 times before I figured out what it means 💀 (then again, maybe I shouldn't be reading it at 3am)
Since it's the same meaning I'd think that more concise is better? Do you disagree?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if something like this is what you meant to say or not. The Reference doesn't refer to "unsizing" anywhere -- even though, you're right, we often do in conversation -- so we'd need to introduce that somehow in this note.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm. There may not be the literal word "unsizing", but we do refer to this coercion as "unsize coercion".
I do feel like it's clear what this means, given that this note is surrounded by mentions of unsize coercions. (but maybe not see you seem to be confused?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although I'd also say that replacing "unsized coercion" to "unsizing coercion" everywhere in the reference would be a good idea. It sounds better and more closely matches how people actually talk about this feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In conversation with @traviscross, I was one of the confused ones. TC pointed out that "unsized coercion" can be interpreted as the codomain so to speak. That is, it doesn't seem like a misnomer to me because it is only referring to coercions to something not sized, and thus seemed accurate.
It is true the word "unsizing" implies "to remove the size of", that is the domain is sized, and thus is not always an accurate term. However, we don't use "unsizing" here.
Probably a bit of a muddled interpretation on my part, but does that make sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"unsized coercion" sounds like the coercion itself is unsized. I don't even know what that should mean.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there worth saying anything here about whether this can change the value of the data pointer, or whether it can only change the vtable pointer?
(Maybe it's just a non-normative note, but contrasting with
*const [i32]
→*const [i8]
that doesn't change the metadata -- even though people sometimes expect it to -- sounds potentially useful.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this whole section might need a retractor :p
Specifically I think it should more clearly state the difference between
Unsize
andCoerceUnsize
, mention that the former is not actually a coercion, and the latter is only for pointers.Then it would also make sense to add a note that unsizing coercion changes the metadata of a pointer, while all other coercions/casts don't.
But, if you don't mind I'll leave this as a follow up ^^'