Change into scheme to be fully type-based
#23014
Conversation
| ``` | ||
| This inserts the given conversion on the `ys` argument in `xs ++ ys`. It typechecks without a feature warning since the formal parameter of `++` is of type `into[IterableOnce]`, which is also the expected type of `ys`. | ||
|
|
||
| The |
There was a problem hiding this comment.
Looks like this paragraph went astray.
There was a problem hiding this comment.
Thanks. I fixed it.
|
At first glance, I really like this -- it feels like it strikes a nice practical balance, and I suspect I'd use it moderately often. I agree that it's quite flexible, but IMO requiring the |
| Since `into` is a regular type constructor, it can be used anywhere, including in type aliases and type parameters. This gives a lot of flexibility to enable implicit conversions for user-visible types. For instance, the Laminar framework | ||
| defined a type `Modifier` that is commonly used as a parameter type of user-defined methods and that should support implicit conversions into it. Pattern like this can be supported by defining a type alias such as | ||
| ```scala | ||
| type Modifier = into[ModifierClass] |
There was a problem hiding this comment.
allowing abstraction like this seems to lose the benefit of keeping it clear that a method argument might be implicitly converted - perhaps tooling™️ can fix this if scaladoc could track if argument types can be converted to
There was a problem hiding this comment.
Yes, maybe. But it was a concern raised by @sjrd, that it would be too cumbersome to require explicit into in user-defined Laminar functions. We can now address that concern. But we should do it only if there's a clear DSL-like pattern, IMO. Laminar modifiers might meet that requirement, but maybe not many other scenarios do.
There was a problem hiding this comment.
Personally i think this is fine, at least it communicates at the definition of the type that its a target
odersky
force-pushed
the
change-into-2
branch
2 times, most recently
from
09cdfca to
843ac25
Compare
We now make into be just a type alias that the compiler knows about. No syntax changes are necessary.
Refactor dealias to make it easier to add further keep conditions
Drop the restriction that instantiated TypeVars cannot be conversion targets
And drop duplicate test into-inferred.scala
Instead use a hack to convert the Into flag into the $into annotation for pickling. This means we can merge the scheme without waiting for the next minor version. When the scheme is stabilized we should revert this commit again to get the cleaner treatment where the Into fag maps to the INTO tag.
Erase all covariant and invariant into occurrences in parameter types
|
This is best reviewed commit by commit. |
|
SIP 71 was accepted for implementation in the last SIP meeting on May 23rd. So this PR can now be merged. |
|
|
||
| We can support implicit conversions to `Modifier`s simply by making `Modifier` an `into` trait: | ||
| ```scala | ||
| into trait Modifier ... |
There was a problem hiding this comment.
how would Laminar maintainers preview integrating the feature without having to make typical users set experimental flag? I guess create a new branch with the mod and release under special version
There was a problem hiding this comment.
That would be a way to do it. I agree our experimental scheme does not lend itself well to multi-group experiments. Of course Laminar could turn the flag on for themselves and use it in their code base and tests. That would already tell them something.
|
|
||
| /** The position of the modifier associated with given flag in this definition. */ | ||
| def flagSourcePos(mdef: DefTree, flag: FlagSet): SrcPos = | ||
| mdef.mods.mods.find(_.flags == flag).getOrElse(mdef).srcPos |
There was a problem hiding this comment.
I was always confused by the mods.mods. Do you know why we have two sets of modes with similar items?
There was a problem hiding this comment.
I believe it's historical. The first mods is the Modifier structure which contains the actual modifier set as a mods field, and also contains annotations and an optional private-within name.
A new version of the
intoscheme to allow certain implicit conversions without requiring a language import.This is a lot simpler than previous schemes since it makes use of the power of the type system instead of building up a parallel structure based on modifiers. It is also considerably more flexible than the previous scheme. One open question might be whether it's too flexible.