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
Type deduction RFC #4
Type deduction RFC #4
Changes from all commits
5dcf742
8a5dcb0
c2c4fad
f0f9e38
74d9214
4efe203
11b86fc
d985c8e
474a08e
46e0a30
fa85308
983bc30
ad3c2cb
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.
I wonder if this would be simpler to reason about if we expanded
SyntaxShape::Any
into the set of types it can represent, and then didchecked_insert
like above (just a set intersection) 🤔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.
Good hint :)
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 was just wondering that too. A kind of
SyntaxShape::OneOf(a, b, c)
form.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.
After thinking this through, I am not sure whether it would simplify the implementation.
Looking closes at the pseudo code, it is obvious that:
is the same as:
The reason I have written it down like this is, that meta information (where was this deduction be made?) needs to get merged. If
SyntaxShape::Any
would be expanded into every type, handling the meta data correctly might be a pain in the concrete implementation.OneOf should then probably contain a vector of SyntaxShape variants? From my point of view this is almost the same as a vector of SyntaxShape's. It may make the code a little more declarative, but at the added cost of having a different size for the SyntaxShape enum. I would like to stick with the current pseudo code.