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
Extension types page #5508
Extension types page #5508
Changes from 5 commits
2264edc
8c260d6
982206c
edb3f8f
0c1eb04
659bb56
e522864
80e6884
1659e41
92d57ca
69ea7b2
916cc23
7558e2d
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.
The "of the underlying type" here makes it sound (at least also) like the instance variable is on the underlying type, not that its declared type of the variable.
Needs at least "instance variable of the underlying type's type" which gets weird.
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 removed the second clause because it seems too confusing no matter what. I initially included it to clarify the purpose of the representation object for people who are familiar with wrapper classes, but maybe that's not valuable enough to warrant any possible confusion?
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.
Or just
i
, since it's in the lexical scope.(In fact, I think our lints will likely suggest removing the
this
.)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.
Just checked, that is true. Thanks
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.
@lrhn would it make sense to say:
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.
Then say what that means, because it's not totally clear from the words.
Or maybe that's too much to say here. And I didn't even use the word "virtual".
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 like the explanation but yes probably too much to say here :/ (I still added some of it though)
Where are annotations typically defined? It would be great to have this explanation wherever that is, and point to it from here.
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.
Perhaps we do need an explicit definition of 'more specific extension type member declaration'. ;-)
If we have that then we would just use the concept in section 'Implements' above.
However, the metadata annotation
@redeclare
makes no difference for the static analysis of the declaration or its invocations: The compiler will always choose the most specific instance member declaration for any given member name, metadata or not, based on the static type of the receiver.@redeclare
does only one thing: It turns off lint messages fromannotate_redeclares
if the declaration does redeclare some other declaration (which would then necessarily be in a superinterface, which could be an extension type or a non-extension type). If there is a redeclaration, but no@redeclare
, then we get a lint message because this situation is considered to be somewhat error prone.It is possible that a spurious
@redeclare
is also reported as a lint (that is, a@redeclare
on a declaration that doesn't redeclare anything). This is not error-prone, just irritating, because we might waste some time looking for the declaration which is being "turned off" for this type of receiver, because we now have a more specific declaration with the same name—but there is no such declaration, so there's nothing to worry about.The point is in any case that statically resolved member implementations are very different from OO dispatched member implementations: The latter will always fit the receiver perfectly (we're always calling the most specific implementation of the instance member, relative to the run-time type of the receiver, except when we use
super
). In contrast, the former is resolved based on the static type of the receiver, and the same receiver can have many different static types.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 had linked to this section in a previous commit, should I put that back?
I'm starting to think it'd be best to just point to
@redeclare
/annotate_redeclares
documentation from the "Implements" section, and skip the dedicated@redelcare
section altogether. The lint documentation isn't very descriptive though. Fixing that could be one option. But where are annotations documented (if at all)? I can't find anything for@redeclare
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 I want this to be the term we use for an extension type implementing its representation type.
It's also possible to have extension types that implement a supertype of their representation type. (Say
ExtMyFuture<T>._(_MyFuture<T> _) implements Future<T>
where_MyFuture
is my own special implementation ofFuture
that I don't want to expose, andExtMyFuture
is an extension type wrapper around it (for some reason).Is that class "Transparent"? (Not with this definition.)
An extension type which implements precisely its representation type is a common and reasonable use-case, but it's not special to the extension type feature. It's just another "implements a supertype of the extension type" case that happens to not be a proper supertype.
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.
If I'm understanding correctly (from this and your other comments on this section) would it be better to say:
"An extension type that implements its representation type, or a supertype of its representation type, can be thought of as transparent, because it provides a kind of overarching access to underlying type members (at least somewhere in the hierarchy / either those of the direct representation type or somewhere higher in the type hierarchy) that is otherwise not the default for the intrinsic isolated nature of extension type interfaces"?
Not that that's exactly what I'll write, but is my understanding correct?
Edit: Or, are you saying that naming this scenario at all is not worthwhile? Maybe it should just be a sub-point of the implements section?
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.
True. I did not push on emphasizing this point because I think it's a natural refinement. The crucial message to communicate is that there are two equally valid, but very different, ways to use an extension type: It can be used (1) to extend the interface of an existing type (adding extra members), and (2) to create an unrelated type with an unrelated interface.
An example of the former is
FancyString
that adds extra methods toString
; we surely want to be able to use an expression of this type in all the ways thatString
supports. An example of the latter isIdNumber
that provides an interface which is suitable for ID numbers, but allows us to use plain (and cheap)int
objects for that purpose; in this case we definitely do not want to silently include the interface ofint
as part of the interface ofIdNumber
.I think those two modes of operation are so different that they should be established conceptually, thus clearly communicating that each of them is OK, but each of them comes with its own conceptualization and software engineering rules.
It would be helpful to support this conceptualization by means of a well-chosen terminology, and 'transparent' seems fine to me. So
FancyString
is transparent because we "can see" the underlying typeString
, andIdNumber
is 'not transparent' because it is an important part of being an ID number that it does not do all theint
things.On top of that, we can refine: It's possible for an extension type
E
to have a representation typeR1
and a clauseimplements ... R2 ...
whereR1
is a proper subtype ofR2
, e.g., becauseR1
is private andR2
defines the part of the interface ofR1
that we wish to provide to clients. We can also have a mostly-transparent extension type that adds some new members and adapts others by redeclaring the given name (e.g., in order to use stricter types on some parameters of a method, or different default values, etc). Another way to be mostly transparent is to implement a type which is a proper supertype of the representation type.My take on the refinements was that there may or may not be room for that level of detail here. If we can help developers to firmly establish the notion of an extension type as transparent or not transparent then it won't be a problem to deal with everything in between.
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.
Ok, I think I figured out how to handle this (commit coming up!).
I'll include Lasse's points about the different potential
implements
scenarios with examples in the "Implements" section. Since that section comes first, this will hopefully set the precedent that we are not implying implementing the representation type or nothing at all are your only options.Then later on in the "Usage" section, I'll describe Erik's crucial use cases, transparent and non-transparent. I'll include information about the other
implements
situations in the "transparent" case section to not gatekeep the scenario too much. I'll generally try not to be too strict about "transparency" as a title, but more just "you can think of these as transparent because..." (also based on Erik's comment here).Thanks for the attention to detail!
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.
Technically not true. It can implement a superclass of its representation type, and then the members of that supertype are available, while not necessarily all the members of representation type.
Generally, this section assume either implementing the representation type, or implementing nothing.
The other options are implementing a supertype of the representation type (even if just
Object
, so that the extension type is non-nullable andExtensionType?
becomes useful), and implementing another extension type which has a super-representation type.My canonical example of the latter is:
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.
Maybe also show:
which shows that the static type of the matched value is
NumberE
here.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 added a lead-in to this example saying:
But I don't think that's makes sense and I don't really understand the example. What are we showing here? Is it just another way to show that
NumberE
isint
at run time? It doesn't seem like "the static type of the matched value (is thati
?) isNumberE
" to me, but rather that the runtime type ofNumberE
isint