Skip to content
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 named extensions #4114

Closed
wants to merge 33 commits into from
Closed

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Mar 14, 2018

Following suggestions from @sjrd, a scheme to have named extensions, using

extension <name> for <type> { ... }

syntax.

Based on #4028. The first commit of the new PR is 5dedefa. This one contains worked out docs but no implementation yet.

This is a third attempt, after #4043 and #4085.

@odersky odersky force-pushed the add-extension branch 4 times, most recently from 72be90e to 17059f0 Compare March 15, 2018 15:27
@odersky
Copy link
Contributor Author

odersky commented Mar 15, 2018

Rebased to profit from the FromTasty fixes for opaque.

@odersky
Copy link
Contributor Author

odersky commented Mar 15, 2018

This PR is complete now. It depends on #4028 (opaque types) being accepted, since otherwise there's less motivation in having an extension mechanism separate from value classes. If #4028 is accepted, we have a good case for this PR as a replacement of value classes and implicit classes.

@odersky odersky mentioned this pull request Mar 15, 2018
@odersky odersky self-assigned this Mar 15, 2018
@japgolly
Copy link
Contributor

Something to consider: as it currently stands with implicit value classes, there is a restriction that value classes can only reside in objects and not traits (nor classes if I remember correctly). This restriction can be bypassed by separating the class and the implicit conversion (example). This leads to two questions:

  1. Will extensions be allowed to exist in traits?
  2. Have you considered one would alias/export an extension that resides somewhere else?

If not, it might be as easy as

object SomewhereElse {
  extension ListOps[T] for List[T] {
    def second: T = tail.head
  }
}

trait MyExports {
  extension ListOps[T] = SomewhereElse.ListOps[T]
}

Note: Ideally we'd have a export keyword (dual to import) but unfortunately that never seems to get off the ground despite community enthusiasm :(

@odersky
Copy link
Contributor Author

odersky commented Mar 16, 2018

@japgolly Good point about aliasing, but not sure about the syntax.

I am also quite positive about export, but did not get around to think about it deeply and have not seen other worked out proposals either. I would encourage a SIP, though.

@japgolly
Copy link
Contributor

I am also quite positive about export, but did not get around to think about it deeply and have not seen other worked out proposals either. I would encourage a SIP, though.

Thanks, that's fantastic to know! It kind of makes me wonder if I should try my luck at creating a SIP...

@odersky
Copy link
Contributor Author

odersky commented Mar 29, 2018

Thanks, that's fantastic to know! It kind of makes me wonder if I should try my luck at creating a SIP...

I would welcome a SIP on that topic!

Add `opaque` to syntax. Let it be parsed and stored/pickled as a flag.
An opaque type becomes an abstract type, with the alias stored in an
OpaqueAlias annotation
maintain the link from a module class to its opaque type companion,
using the same technique as for companion classes.
Higher-kinded comparisons did not account for the fact that a type constructor
in a higher-kinded application could have a narrowed GADT bound.
The previous scheme, based on "magic" methods could not accommodate links
from opaque types to their companion objects because there was no way to
put a magic method on the opaque type.

So we now use Annotations instead, which leads to some simplifications.
Note: when comparing the old and new scheme I noted that the links from
companion object of a value class to the value class itself broke after
erasure, until they were restored in RestoreScopes. This looked unintended
to me. The new scheme keeps the links unbroken throughout.
FirstTransform rewrites opaque type aliases to normal aliases,
so no boxing is introduced in erasure.
It's overall simpler to just define them internally in Definitions
Make them part of the OpaqueAlias annotation. This is has two benefits

 - opaque types stop being companions after FirstTransform. Previously,
   the LinkedType annotations would persist even though the Opaque flag
   was reset.

 - we gain the freedom to implement companions between classes differently.
Use a field in ClassDenotation instead of an annotation
odersky and others added 15 commits March 31, 2018 13:32
When computing the member denotation of a selection of a TypeRef `T`,
if the normal scheme fails and `T` has GADT bounds, compute the member
in the upper bound instead.

This is needed to make the opaque-probability test work.
Add this test, as well as some others coming from SIP 35.
The `findMember` and `underlying` methods on `TermRef` now take GADT bounds into account.
This replaces the previous special case in `secectionType`. The previous case did always
pass -Ycheck.

Currently, the treatment is restricted to TypeRefs of opaque types. We should extend
that to all GADT-bound TypeRefs. The question is how this will affect performance.
The GADT bounds set in an opaque companion also need to be established for
any inlined code coming from the companion.

Test case in opaque-immutable-array.scala.
Following suggestions by @sjrd, a worked out scheme to have named extensions, using

    extension <name> for <type> { ... }

syntax.
It's now a keyword, need to enclose in backticks when used as an identifier.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants