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.

Sign up for GitHub

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

Jump to bottom

Support implicit scope augmentation for Mirror #6879

Merged
merged 6 commits into from
Jul 25, 2019
Merged

Support implicit scope augmentation for Mirror #6879

merged 6 commits into from
Jul 25, 2019

Conversation

milessabin
Copy link
Contributor

@milessabin milessabin commented Jul 17, 2019 •
edited
Loading

Mirrors can now be materialized with additional type member refinements. Any additional type members will contribute to the implicit scope of any searches involving the type of the summoned Mirror. Typically such types will be the singleton types of objects containing implicit extension methods for the Mirror, or other related implicit definitions.

Given,

case class Foo(i: Int)

the[Mirror.Product { type MirroredType = Foo ; type Phantom = Other.type }]

then requested type (formal in the code) will be,

Mirror.Product { type MirroredType = Foo ; type Phantom = Other }

and the core synthesized mirror type will be,

Mirror.Product {
  type MirroredType = Foo
  type MirroedMonoType = Foo
  type Label = "Foo"
  type MirroredElemTypes = Tuple1[Int]
  type MirroredElemLabels = Tuple1["i"]
}

The result of intersecting these two is then,

Mirror.Product {
  type MirroredType = Foo
  type MirroedMonoType = Foo
  type Label = "Foo"
  type MirroredElemTypes = Tuple1[Int]
  type MirroredElemLabels = Tuple1["i"]
  type Phantom = Other.type
}

That's the refined type that we'll get for the mirror as a given argument, so if we have an object Other containing extension methods like so,

object Other {
  given Ops {
    def (m: Mirror.Product { type MirroredType = T }) extension [T]
  }
}

they will be found because the contents of Other is included in the implicit scope for the extension method search via the parts of the refined type of the mirror.

@milessabin milessabin requested a review from odersky July 17, 2019 19:01
@milessabin milessabin force-pushed the topic/mirror-implicit-scope branch 2 times, most recently from a2ed239 to b26f555 Compare July 18, 2019 13:37
@milessabin
Copy link
Contributor Author

Rebased.

smarter
smarter reviewed Jul 18, 2019
View reviewed changes
compiler/src/dotty/tools/dotc/typer/Implicits.scala Outdated
.refinedWith(tpnme.MirroredMonoType, TypeAlias(monoType))
.refinedWith(tpnme.MirroredType, TypeAlias(mirroredType))
.refinedWith(tpnme.MirroredLabel, TypeAlias(ConstantType(Constant(label.toString))))

extras.foldLeft(m) { case (m, (tpnme, tpe)) => m.refinedWith(tpnme, tpe) }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to just do formal & m ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll have a look ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it makes sense, and simplifies things significantly. Good call :-)

@milessabin
Mirrors can now be materialized with additional type member refinements.
7f826e9 
Any additional type members will contribute to the implicit scope of any
searches involving the type of the summoned Mirror.  Typically such
types will be the singleton types of objects containing implicit
extension methods for the Mirror, or other related implicit definitions.

Pass through all extra refinements
@milessabin
Intersect with formal rather than copying members
f6d4ddb
@milessabin milessabin force-pushed the topic/mirror-implicit-scope branch from 0bfc9d2 to f6d4ddb Compare July 18, 2019 17:02
@milessabin
Copy link
Contributor Author

Rebased for #6886.

@milessabin
Copy link
Contributor Author 

the[SomeClass & Mirror.ProductOf[ISB]]

gives me,

no implicit argument of type Test.SomeClass & deriving.Mirror.ProductOf[Test.ISB] was found ...

which I think is what we want.

@smarter
Copy link
Member

smarter commented Jul 19, 2019

Some neg tests exercising this and other weird combinations (e.g. refining with a def) would be nice

@milessabin
Copy link
Contributor Author

Added check rejecting methodic refinements and some neg tests.

@milessabin
Copy link
Contributor Author

@smarter done.

odersky
odersky reviewed Jul 19, 2019
View reviewed changes
compiler/src/dotty/tools/dotc/typer/Implicits.scala Outdated
if (!checkFormal(formal)) EmptyTree
else
formal.member(tpnme.MirroredType).info match {
case TypeBounds(mirroredType, _) if checkFormal(formal) => mirrorFor(mirroredType)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checkFormal(formal) is always true here

Copy link
Contributor Author

@milessabin milessabin Jul 19, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that can be the case. It being false for formals which include def/val definitions is what's responsible for the (intended) neg test failures.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it just got tested in line 968, so to reach the else part, it must be true.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh! Yes, sorry ... I completely missed the redundant guard there. Removed.

@milessabin
Copy link
Contributor Author

@odersky I think this is ready to merge now.

@milessabin milessabin merged commit 8095c23 into scala:master Jul 25, 2019
@milessabin milessabin mentioned this pull request Jul 26, 2019
Merged
@anatoliykmetyuk anatoliykmetyuk added this to the 0.18 Tech Preview milestone Aug 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smarter smarter smarter left review comments

@odersky odersky odersky approved these changes

Assignees

@odersky odersky

Labels
None yet
Projects
None yet
Milestone
0.18 Tech Preview
Development

Successfully merging this pull request may close these issues.
4 participants
@milessabin @smarter @odersky @anatoliykmetyuk