-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Problem with Traits not being Matchable #16408
Comments
Reproduced on nightly: @bblfish Could you provide further minimization? It will be easier to address the problem if you provide a single scenario and make this issue self-contained. The best format would be the one that contains:
The code that you provided is fairly complex. There is a high possibility that there is a way to show the issue with a shorter, less complex snippet, that would make it much easier for us to address this problem. Editing this issue to contain all the information will also help massively, as one can get pretty lost following the previous repositories, issues, and comments in them. |
The code I posted currently stems from the use case I described in lib unification and match types so it has the advantage of describing a realistic situation. But I agree. I'll try to remove stuff slowly and see how small I can get. |
To be fair, it is not that valuable to represent a realistic situation. It's best to require as little context as possible. Representing a realistic situation often requires adding additional, unnecessary information. This is a bug report to the compiler, and it's best to stay on the same level of abstraction that the compiler is operating on - the syntactic and semantic structures. You can, of course, provide a rationale on why it is a bug by relating to a real-world problem - for example, argument that the compiler behaves confusingly in a common real-world case. But the problem itself should be kept as simple as possible, and having as narrow context as possible. |
Ok, @szymon-rd I created a new branch minimization and was able to remove quite a lot of code, as shown by the history. commit bblfish/DottyIssue16247@4159ccd removed too much, and commit bblfish/DottyIssue16247@eaf445a put back the minmum for the problem to reappear, as one can see in the github actions. The problem appears when I move from the functioning trait RDF:
rdf =>
type R = rdf.type
type Node <: Matchable
type URI <: Node
given rops: ROps[R]
end RDF to the code that works when the types are implemented with classes but trait RDF:
rdf =>
type R = rdf.type
type rNode <: Matchable
type rURI <: rNode
type Node <: rNode
type URI <: Node & rURI
given rops: ROps[R]
end RDF I should have saved the intermediate point which also compiled which I think was trait RDF:
rdf =>
type R = rdf.type
type rURI <: Matchable
type Node <: Matchable
type URI <: Node & rURI
given rops: ROps[R]
end RDF For some reason that type of hierarchy works ok for classes but not when traits or interfaces are involved. override opaque type rNode <: Matchable = cz.Node
override opaque type rURI <: rNode = cz.Uri
override opaque type Node <: rNode = cz.Node
override opaque type URI <: Node & rURI = cz.Uri and for the trait based implementation we have override opaque type rNode <: Matchable = rdfscala.TstNode
override opaque type rURI <: rNode = rdfscala.URI
override opaque type Node <: rNode = rdfscala.TstNode
override opaque type URI <: Node & rURI = rdfscala.URI Why do I need that?Well it allows me to use the concept that types are identified by the operations they support to make life of developers using our library easier. Jena and RDF4J don't really check carefully whether a node of type URI in a graph is a relative or absolute URLs, so we can easily allow both. But we would actually like to keep track of the difference.
|
Thank you for the additional information! This code is still really complex, and further minimization would be welcome. Making the code simpler and shorter will allow us to address this issue faster. |
I have simplified the code again now here in the https://github.com/bblfish/DottyIssue16247/blob/min2 Then I made sure that there was the least difference between the Class and Trait based code, which is now very small as can be seen by this opendiff snapshot. The code is run https://github.com/bblfish/DottyIssue16247/actions/runs/3594012563 |
I have now created a new branch min2-fix which sets out to fix the trait based code in two steps. The second commit to that branch involves just changing bblfish/DottyIssue16247@2b2e935 That should be a helpful way of looking at the problem as it narrows the change to one simple diff. |
There is a lot of information in this post right now. Could you put a self-contained summary with the current status of minimisation with the links and all the required information to look into the problem in the issue itself? By that I mean editing it so it only contains the information that is up-to date and required to debug the issue, removing all the outdated information and other background info like history of the problem. It would be very helpful for us, and again thanks for your work minimizing and simplifying the snippets! |
I have further simplified the code now, getting it down to one scala file, and removing the |
Ok, so we minimized the code to this snippet: object Helpers:
type NodeFun[R] = Matchable // compiles without [R] parameter
type URIFun[R] = R match
case GetURI[u] => u & NodeFun[R]
private type GetURI[U] = RDF { type URI = U }
end Helpers
trait RDF:
type URI
end RDF
trait ROps[R <: RDF]:
def auth(uri: Helpers.URIFun[R]): String
object TraitRDF extends RDF:
override type URI = TraitTypes.UriImpl
val z = new ROps[TraitRDF.type] {
override def auth(uri: Helpers.URIFun[TraitRDF.type]): String = ???
}
end TraitRDF
object TraitTypes:
trait UriImpl // doesn't compile
// class UriImpl // compiles |
Do you have any idea yet how complicated or deep a problem this bug has uncovered? Having this added to the "Future versions" milestone sounds very open-ended :-/ |
Minimized just a bit more, without requiring the object Helpers:
type NodeFun[R] = Matchable // compiles without [R] parameter
type URIFun[R] = R match
case RDF[u] => u & NodeFun[R]
end Helpers
trait RDF[URIParam]
trait ROps[R <: RDF[?]]:
def auth(uri: Helpers.URIFun[R]): String
object TraitRDF extends RDF[TraitTypes.UriImpl]:
val z = new ROps[TraitRDF.type] {
override def auth(uri: Helpers.URIFun[TraitRDF.type]): String = ???
}
end TraitRDF
object TraitTypes:
trait UriImpl // doesn't compile
//class UriImpl // compiles Errors:
For extra fun: if we replace @smarter Some weird erasure of intersection types thing perhaps? |
If UriImpl is a trait, then The problem is that match type reduction also performs some amount of simplification, so |
Specifically, the culprit is the call to .simplified here: |
The other piece of the puzzle here is that When we typecheck the |
In other words, we might want to wrap |
It would be great if the fix could make it into the next release so that I can continue my work with banana-rdf. :-) |
It's a bit awkward but you can probably work around the problem with some extra bounds and intersections with Object or Matchable, for example the following adapted from your original example compiles: import scala.util.Try
trait RDF:
rdf =>
type R = rdf.type
type Node <: Matchable
type URI <: Node
given rops: ROps[R]
end RDF
object RDF:
type Node[R <: RDF] <: Matchable = R match
case GetNode[n] => Matchable //n & rNode[R]
type URI[R <: RDF] <: Matchable & Node[R] = R match
case GetURI[u] => u & Node[R]
private type GetNode[N] = RDF { type Node = N }
private type GetURI[U] = RDF { type URI = U }
end RDF
trait ROps[R <: RDF]:
def mkUri(str: String): Try[RDF.URI[R]]
def auth(uri: RDF.URI[R] & Matchable): Try[String]
object TraitTypes:
trait Node:
def value: String
trait Uri extends Node
def mkUri(u: String): Uri =
new Uri { def value = u }
object TraitRDF extends RDF:
import TraitTypes as tz
override opaque type Node <: Matchable = tz.Node
override opaque type URI <: Node = tz.Uri
given rops: ROps[R] with
override def mkUri(str: String): Try[RDF.URI[R]] = Try(tz.mkUri(str))
override def auth(uri: RDF.URI[R]): Try[String] =
Try(java.net.URI.create(uri.value).getAuthority())
end TraitRDF
class Test[R <: RDF](using rops: ROps[R]):
import rops.given
lazy val uriT: Try[RDF.URI[R]] = rops.mkUri("https://bblfish.net/#i")
lazy val x: String = "uri authority=" + uriT.map(u => rops.auth(u))
@main def run =
val test = Test[TraitRDF.type]
println(test.x)
|
This comment was marked as outdated.
This comment was marked as outdated.
The pattern that is being used here is incredibly useful to tie different libraries togeteher efficiently in a unified interface. It is needed as a replacement to type projections discussed here |
Transcribing and paraphrasing from smarter's comment in scala#16408 (comment) : Type erasure assumes method signatures aren't simplified, since simplification logic is implementation-defined. For instance, some intersection types can be simplified down, but intersection types and their simplification can erase to different types - prefering classes over traits, for instance (for Java interop, as it matches Java's erasure). Also note, simplify doesn't simplify intersections and unions in Type mode. But Match Types will cache their reduction without considering the type mode as a cache input, thus the simplified reduction leaks even when called in Type mode. Perhaps we don't need to reach for simplified and can simplify normalise down the reduced case (allowing for recursive match types to reduce down). Also the desire is to have both reducing cases normalize, rather than just the first one.
Transcribing and paraphrasing from smarter's comment in scala#16408 (comment) : Type erasure assumes method signatures aren't simplified, since simplification logic is implementation-defined. For instance, some intersection types can be simplified down, but intersection types and their simplification can erase to different types - prefering classes over traits, for instance (for Java interop, as it matches Java's erasure). Also note, simplify doesn't simplify intersections and unions in Type mode. But Match Types will cache their reduction without considering the type mode as a cache input, thus the simplified reduction leaks even when called in Type mode. So we call simplified in Mode.Type, in both cases (another desire), so only that result is cached instead. Using normalise doesn't work because, for example, that doesn't normalise match types that are applied type args (e.g. args of Pair). And not caching the result of those reductions means that they'll get repeat over and over.
There are a number of calls to ".simplified", which changes behaviour based on Mode.Type. It does _less_ under Mode.Type, so we conservatively call it all under Mode.Type. Transcribing and paraphrasing from smarter's comment in scala#16408 (comment) : Type erasure assumes method signatures aren't simplified, since simplification logic is implementation-defined. For instance, some intersection types can be simplified down, but intersection types and their simplification can erase to different types - prefering classes over traits, for instance (for Java interop, as it matches Java's erasure). Also note, simplify doesn't simplify intersections and unions in Type mode. But Match Types will cache their reduction without considering the type mode as a cache input, thus the simplified reduction leaks even when called in Type mode. So we call simplified in Mode.Type, in both cases (another desire), so only that result is cached instead. Using normalise doesn't work because, for example, that doesn't normalise match types that are applied type args (e.g. args of Pair). And not caching the result of those reductions means that they'll get repeat over and over.
Transcribing and paraphrasing from @smarter's comment in #16408 (comment) : Type erasure assumes method signatures aren't simplified, since simplification logic is implementation-defined. For instance, some intersection types can be simplified down, but intersection types and their simplification can erase to different types - prefering classes over traits, for instance (for Java interop, as it matches Java's erasure). Also note, simplify doesn't simplify intersections and unions in Type mode. But Match Types will cache their reduction without considering the type mode as a cache input, thus the simplified reduction leaks even when called in Type mode. So we call simplified in Mode.Type, in both cases (another desire), so only that result is cached instead. Using normalise doesn't work because, for example, that doesn't normalise match types that are applied type args (e.g. args of Pair). And not caching the result of those reductions means that they'll get repeat over and over.
It seems to be fixed now with the nightly version and I can compile the DottyIssue16247 onepage branch scala-cli compile -j 20 -S 3.nightly scala and similarly with the original java interface-based test now works with the nightly scala-cli --scala 3.nightly scala/RDF_Java_Interface.scala scala/RDF.scala java/testorg/* and the trait based code also compiles with the nightly scala-cli --scala 3.nightly scala/RDF_Traits.scala scala/RDF_UsingScalaTrait.scala scala/RDF.scala |
A fully minimized version of code to duplicate the problem can be found in the comment below by Symon R..
Compiler version
scala 3.2.1 (older too) and nightly 3.3....
Minimized code
The code that does not compile is on this onepage branch of a repo specially for this Dotty issue. There is now only one file to look at: RDF.scala.
The code is duplicated in full here:
Output
The compilation error can be seen in the github actions failing check here.
But I could not get the
-explain
flag to work withscala-cli
.Running it on the command line one gets the full error explantion. (After setting the env variables
below
The following command
But the compiler only has a problem when the URI is implemented in terms of a trait, not when it is implemented in terms of a class.
Expectation
The expectation is that it should work with traits as well as with classes.
A fix is to change the
trait Uri
to aclass Uri
as in the commit that lead to the onepage-works branch commit as shown in this minimal diff:The problem is that it is not always possible use classes only and not traits. A major RDF library RDF4J uses traits for its RDF model, such as IRI.
The text was updated successfully, but these errors were encountered: