Skip to content

given instance parameters cannot appear in the type of a member of the instance #8397

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

Closed
smarter opened this issue Feb 27, 2020 · 12 comments
Closed
Labels
area:desugar Desugaring happens after parsing but before typing, see desugar.scala itype:bug

Comments

@smarter
Copy link
Member

smarter commented Feb 27, 2020

minimized code

given foo(using x: Int) as AnyRef:
  type T = x.type

Compilation output

non-private type T in class foo refers to private value x
in its type signature  = (foo.this.x : Int)

expectation

This could compile if x was desugared to a public val in foo.

@smarter smarter added itype:bug area:desugar Desugaring happens after parsing but before typing, see desugar.scala labels Feb 27, 2020
@neko-kai
Copy link
Contributor

neko-kai commented Feb 28, 2020

@smarter
The equivalent code using anonymous classes compiles fine without having to make the val public, maybe given encoding could be changed to just a def without an associated public class?

implicit def foo(implicit x: Int): { type T = x.type } = new {
  type T = x.type
}

@odersky
Copy link
Contributor

odersky commented Mar 1, 2020

This could compile if x was desugared to a public val in foo.

Yes, but then x would be itself available as a given, which is probably not what is intended.

@odersky
Copy link
Contributor

odersky commented Mar 1, 2020

@neko-kai Unfortunately, "lifting out" the necessary type constraints into a refinement is a rats nest of complexity. At least if you look at the problem in general with all its corner cases.

@neko-kai
Copy link
Contributor

neko-kai commented Mar 1, 2020

@odersky This type is currently inferred automatically if you omit the signature, so it appears the mechanisms are already present, although I may not be aware of more complex cases

@smarter
Copy link
Member Author

smarter commented Mar 1, 2020

Yes, but then x would be itself available as a given

I'm not sure I follow, available as a given in what context ?

@odersky
Copy link
Contributor

odersky commented Mar 1, 2020

@smarter As a field of the class. OK, maybe that's not so much of a problem since the class would usually not be referred to by name. But then there's the issue that we'd create a field that's typically not used at runtime.

@odersky
Copy link
Contributor

odersky commented Mar 1, 2020

@neko-kai We'd have to try out the anonymous class encoding to see whether it would work in all cases.

@neko-kai
Copy link
Contributor

neko-kai commented Mar 1, 2020

As a field of the class. OK, maybe that's not so much of a problem since the class would usually not be referred to by name.

@odersky summon exposes the most precise type, so you could have a situation where a field x is chosen over a .x extension method for specific instances only if x is public. Of course that applies to all public members of a given (even for anonymous class instances with refinements)

@odersky
Copy link
Contributor

odersky commented Mar 1, 2020

@neko-kai The anonymous class encoding infers the right type only if there are no parents at all. Once you put in the AnyRef (or any other parent) it does not infer a refinement.

EDIT That's for Dotty. In scalac the refinement is computed. But that's the rat nest of complexity I was talking about. As far as I recall there we multiple issues and we never got it completely right.

So, one question is: should we go back and infer type and value refinements for the types of anonymous classes? So far we did not, because inferring type refinements is messy, and value refinements require reflection at runtime. We have accepted that we need value refinements to support Chisel and there was an idea to do this if the anonymous class inherits from Structural. But none of this is implemented yet.

@smarter
Copy link
Member Author

smarter commented Mar 1, 2020

But then there's the issue that we'd create a field that's typically not used at runtime.

If the given parameter is used in any method of the given instance, then we need a field anyway. So this is only sub-optimal when the parameter is used purely as a witness (in which case ideally you'd want the ability to mark it erased anyway).

summon exposes the most precise type, so you could have a situation where a field x is chosen over a .x extension method for specific instances only if x is public.

Good point. Instead of making it public, we could make it protected, Dotty would still be OK with having it appear in the type of a public member.

@odersky
Copy link
Contributor

odersky commented Mar 1, 2020

@smarter Good idea. Let's try with protected.

@odersky odersky closed this as completed in ad240d0 Mar 1, 2020
odersky added a commit that referenced this issue Mar 1, 2020
Fix #8397: Make context parameters of given instances protected
@neko-kai
Copy link
Contributor

neko-kai commented Mar 1, 2020

@odersky

value refinements require reflection at runtime.

Scala 2 can avoid reflection for refinements that are 'local enough', e.g.

def makeResource[A](acquire: => A)(release: A => Unit): Unit = makeResource(acquire)(release)

makeResource {
  object x {
    def close(): Unit = ()
  }
  x
}(_.close()) // no reflective access, even though there's no expressible type parameter for makeResource

This ability could be extended to public refinements coming from anonymous instances (i.e. the type stores the anonymous class also). Theoretically. I understand that it would probably be nonviable due to binary compatibility.

To be honest, what I'm irked about is the sudden and inconsistent namespace polution from given instances. This causes an error:

case class A[B](b: B)
given A[B: Show] as Show[A] {
  def show(a: A) = "A(${Show(B)})"
}
// A is already defined as class A

But this doesn't:

case class A[B](b: B)
given A as Show[A[Int]]

Because only a parameterized given takes up two public names, the rest of the givens only take up a value name!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:desugar Desugaring happens after parsing but before typing, see desugar.scala itype:bug
Projects
None yet
Development

No branches or pull requests

3 participants