-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add extension instances #8318
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
Merged
anatoliykmetyuk
merged 11 commits into
scala:master
from
dotty-staging:add-extension-instance
Mar 3, 2020
Merged
Add extension instances #8318
anatoliykmetyuk
merged 11 commits into
scala:master
from
dotty-staging:add-extension-instance
Mar 3, 2020
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
071b0b4
to
d36f43b
Compare
d36f43b
to
ee3a2ac
Compare
ee3a2ac
to
865604a
Compare
Don't postulate a synthetic import when expanding collective extensions. Such an import would not typecheck. Postulate a direct substitution of co-defined method identifiers instead.
2152fba
to
2c88bb5
Compare
I believe error messages tests should be dropped, unless they test something really specific. E.g. a way information is represented that is easy to break. For normal tests we have check files, or (most of the time even better) `// error` annotations. The problem with over-engineered solutions like error messages tests is that they cause a huge friction when things change. I lost an inordinate amount of time getting the extensions PR over the line since in the meantime there landed an elaborate error messages commit. Fixing rebase breakages over this PR is not a good use of my time! Two things could improve that situation: - we go slower on inessential PRs like error messages, _in particular_ if there is a pending conflict with a pending PR - we go faster merging essential PRs.
anatoliykmetyuk
approved these changes
Mar 3, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Add a new
extension { ... }
syntax for extension instances without a collective parameter, andtweak the translation rules for collective extensions.
Motivation
Collective extensions and normal extension methods are conceptually closely related, but look far apart. The expansion of collective extensions into given instances that implement
AnyRef
is a cute technical trick, but few would want to write that.It is generally encouraged that extension methods should be put into given instances since that way they can be often be found in the implicit scope of the left-hand side argument, and do not require an import. Furthermore, it's better to import an implicit extension object instead of all extension methods since that leads to less namespace pollution.
But so far, the only pleasant way to do this was to use collective extensions. These are great where they fit, but they do not fit everything. In particular, one sometimes wants to define a bunch of extension methods that do not share a common left parameter. Then one would have to define a separate collective extension for each extension method, which is also awkward.
Summary
This PR adds extension instances, to address the above mentioned shortcomings. Extension instances are simply extensions without the
on
part. Example:This extension groups the two extension methods
longestStrings
andsecond
into a single given instanceops
.Collective extensions are mapped to extension instances by repeating the common parameter and extension instances are mapped in turn to given instances.
New doc page: https://github.com/lampepfl/dotty/blob/d36f43bc9ae1bc28fd5ade8d1fcfd7d14365301c/docs/docs/reference/contextual/extension-methods.md