-
Notifications
You must be signed in to change notification settings - Fork 72
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 (sub open ...)
and make it mandatory
#423
base: main
Are you sure you want to change the base?
Conversation
Instead of using final? in the syntax, use ext ::= open | final. Implements the design discussed in #413. Update the interpreter, the abstract syntax, the binary and text formats, and MVP.md.
Current dependencies on/for this PR: This comment was auto-generated by Graphite. |
A somewhat radical late-stage proposal, but my experience with this PR tells me that it would be feasible: what if we changed "final" to "closed", so we had |
I agree that the asymmetry is displeasing. Open and closed have are rather overloaded terms wrt to types. In particular, a closed type is normally understood as one that doesn't have any free names/variables in it, and indeed, the spec already uses it that way. Final on the other hand is the established terminology for this feature, and way clearer. Unfortunately it has no good antonym, and Would folks find it less confusing if there wasn't an extra attribute that can be omitted separately at all, but we just have two alternative keywords for subtyping, say |
I think that would more or less put us back where we started. I'm fine with the terminology in this PR even though |
Further bikeshed options: |
My favorite alternative pair so far is "final" and "nonfinal", but I'd also be fine going with "final" and "open" as in this PR. |
Well, |
Yeah, nonfinal also seems odd, since it makes it sound as if the special case (final) was the normal case. |
I think this is a feature, not a bug. Since the MVP type declarations expand to "final," I think it is helpful as a mental model to think of it as the normal case. |
It is the normal case for types that do not have (non-trivial) supertypes (thus the shorthand), but it is not the normal case for types that actually need to write out |
I agree that the grammatical asymmetry of "final | open" is a little unfortunate, but it seems to be the least objectionable to folks here. The exact wording to use isn't as important to me as avoiding the surprising inversion when you add a '(sub)' around a type, and I like that this PR addresses this. So I would vote that we just go with the grammar in this PR, and go with something that's an improvement over the status quo. |
@eqrion, I guess I'm still a bit confused about what exactly the surprise is — when you add a |
To readers of the text format, I think the word Also, the fact that At the end of the day, an empty |
I agree with @rossberg that we should avoid the use of |
Ah, I missed that this argument would apply to Thinking about the text format here though, the |
@eqrion, to clarify, "open" and "closed" types are meta-level notions, not features that would arise as keywords. The spec defines – as is common practice and basic standard terminology – a "closed type" to be one in which all type indices have been substituted by their definition, and that applies to all sorts of syntactic classes of types. "Open" is less commonly used in that context (e.g., the spec does not), so would be okayish, but it would still give rise to the notion of "closed open sub types", which could be confusing. I could probably live with that, but avoiding it would be preferable, especially when the motivation for it is somewhat borderline. @bvisness, I could see that point if A keyword for "non-final" should ideally be (1) short, (2) unsurprising, (3) not conflicting with other notions. Personally, I still think that the "empty keyword" is the best candidate, especially after I made various searches for antonyms of "final" or "complete" and synonyms of "open" or "extensible" — nothing compelling comes up. Choices like |
I think if you come at it from first knowing the abstract syntax tree the spec uses, it makes sense. But having explained it to multiple new-users within SpiderMonkey, it has always caused confusion along the lines that @bvisness has said.
I think there's a valid argument against this for verbosity reasons. But again from my actual experience teaching others, being explicit about this is less surprising. It's difficult to compare tradeoffs in 'surprising' with 'verbose', but I'd personally prefer 'verbose'.
If the keyword |
Yes, "nonfinal" is clumsy as a keyword, but it is also completely obvious that it is the counterpart to "final" and it is not overloaded with any other meaning. @eqrion's experience that that status quo is confusing to newcomers aligns with my experience as well. Could we all live with "final" and "nonfinal"? |
I generally don't have too strong a preference for considerations in the text format, but I do find that when the text format differs from internal engine representations, it's confusing juggling two concepts. So I would prefer that the text format use a convention that is likely to be reflected in engines/implementations/the spec, and AFAICT "final" and "nonfinal" fit that criteria well. Those have the added advantage of closely mirroring final classes in, e.g. Java, which is a concept that many people already know. |
@titzer, I believe we have already converged on keeping @ALL, considering I acknowledge that this isn’t exactly what some folks asked for, but so is the nature of a compromise under conflicting goals. You can be explicit, e.g., ignore the shorthand when explaining the concepts, but we also keep the syntax more consistent at large, and don't bother experienced users with nuisance. WDYT? |
Talked about this internally a bit and this is how we're feeling right now: We don't think this addresses the confusion we've seen around the default flag for an empty 'sub' being different from when you omit 'sub'. And that's really the main motivation for a change here. But we also don't think this is worth the continued energy discussing this, and it would at least be useful to have a 'non-final' (would it be possible to hyphenate it?) keyword we could use when teaching new users about this. We don't have any opinion on the other inverted keywords and would be fine if they are added or left out. |
Instead of using final? in the syntax, use ext ::= open | final. Implements the
design discussed in #413. Update the interpreter, the abstract syntax, the
binary and text formats, and MVP.md.