-
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
Allow exports in extension clauses #14497
Conversation
def m(c: C): Int = c.x + 1 | ||
export O.* | ||
// generates | ||
// type C = O.C |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this enough to be able to do C(1)
elsewhere? It reads like no to me since only the type is exported. Can we clarify that in the documentation and examples (show if it is allowed or disallowed explicitly)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One can allocate using type aliases (that's why we can write new StringBuilder
for instance, because we have
type StringBuilder = scala.collection.mutable.StringBuilder
in the Scala package. So that's a general principle, not related to exports.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it would be nice if exports of an extension method could unify the parameters, as it stands the extension parameters are appended after, and parameter names are not sanitised:
package app
object StrSyntax {
extension (s: String) {
def x2 = s * 2
}
}
object DeferredSyntax {
extension (s: String) {
private def syntax = StrSyntax
export syntax.x2 // ==> extension (s: String) def x2(s: String): String <--- there are 2 `s` parameters here?
}
}
@main def Test = {
locally {
import DeferredSyntax.*
val ref: String => String = "Hello, world!".x2 // can we drop the extension parameter list of `x2`?
}
locally {
import StrSyntax.*
val ref: String = "Hello, world!".x2
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will request changes due to the issue with hygiene of parameter names
Is it possible to express the Currently I use Scala 2 implicit conversions to a |
@raquo Not sure I understand. If you need to cross-compile, you need to use Scala 2 implicit conversions anyway, and these don't produce a use site warning. |
Ah sorry, I must have misunderstood, I thought scala 2 implicit conversions
will also throw a warning if cross compiled to Scala 3. All good then!
|
42e470f
to
fe8a11c
Compare
This will need a minor bump as it is a source forward breaking change - I think this could be considered a breaking tasty change (previously it was expected that the prefix should be a stable path) - we should have a forward tasty compatibility test added |
Yes, agreed.
In fact, the stability requirement is only dropped for paths in extension methods, and exports are expanded out after desugar, so I don't see how this will affect Tasty. I won't have the time to do a forward compatibility test, and believe none is necessary. In any case this should not be a stopper for getting this merged. |
Another question, should we avoid exporting extension methods (from inside extension block) using the export wildcard - it could be confusing - so best to require explicit export for them |
We kept exports in tasty since #10182, they are used by the doctool for example |
@bishabosha In fact, it might be better to simply exclude extensions from exports, as you suggested. I see not much use in treating them as normal methods with a new extension slapped on to them. That also opens up the possibility to unify the extension parameters in the future, if we so decide (although I think that future is unlikely since such unification looks like a can of worms). |
I'll add a commit for this |
I am already on it. |
09f9343
to
62255bb
Compare
No, the spec is not an implementation detail. There is no
separate conceptual scope, that's a central point for understanding
extension methods. The more we waffle about this, the worse things become.
…On Mon, Apr 18, 2022 at 10:21 PM som-snytt ***@***.***> wrote:
@adampauls <https://github.com/adampauls>
lampepfl/dotty-feature-requests#148
<lampepfl/dotty-feature-requests#148> and #9704
<#9704>
—
Reply to this email directly, view it on GitHub
<#14497 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAGCKVSNUQOMQQUS46ZS5CDVFW74BANCNFSM5ORUWO6A>
.
You are receiving this because you were assigned.Message ID:
***@***.***>
--
Martin Odersky
Professor, Programming Methods Group (LAMP)
Faculty IC, EPFL
Station 14, Lausanne, Switzerland
|
Well, there is definitely a separate something. Note that this works just fine: class Outer(x: Int) {self =>
def show(param1: Int, param2: Int): String = (x + param1 + param2).toString
class Inner()
extension (i: Inner)
private def __this = self
def show(param1: Int, param2: Int): String = self.show(param1, param2)
//export __this.show
}
def use: String = {
val o = new Outer(1)
val i = new o.Inner()
i.show(3, 4)
}
I can write |
And I guess to complete the thought, this does not work class Outer(x: Int) {self =>
def show(param1: Int, param2: Int): String = (x + param1 + param2).toString
def show(i: Inner)(param1: Int, param2: Int): String = show(param1, param2)
class Inner()
extension (i: Inner)
private def __this = self
def show(param1: Int, param2: Int): String = self.show(param1, param2)
//export __this.show
} I think this argues for it being okay to simply allow the But I now understand that this would at a minimum be a follow-on PR, so I'll stop the chatter here. Thanks for the clarification! |
To complete the thought: you currently can export functions that already have an overload defined in the export scope: object Other:
def show(param1: Int): String = "x"
class Outer(x: Int):
def show(param1: Int, param2: Int): String = (x + param1 + param2).toString
export Other.show
def use: String = {
val o = new Outer(1)
o.show(3)
o.show(3, 4)
} And of course if |
@adampauls That would make an interesting thread on Discussions. The restriction on eligibility of members for export has to do with the "motivation" in the reference, which is eloquent on composition over inheritance. Arguably, the pattern you advocate risks greater coupling. Also, export allows me to reason about symbols, and not about the erasure of signatures, and also not just about names: for example, beginners ask, If I Extension methods are also a tool for managing a name space, i.e., a scope. They introduce methods that are special with respect to applicability: how to invoke them, and the scope that makes them available. At the intersection of these two features, am I aware of I'd be interested in reading Discussions on these topics from people using the features. |
62255bb
to
04ad901
Compare
This is part of a long term strategy to get deprecate and remove general implicit conversions in Scala. In the thread https://contributors.scala-lang.org/t/can-we-wean-scala-off-implicit-conversions/4388 we identified two areas where implicit conversions or implicit classes were still essential. One was adding methods to new types in bulk. This can be achieved by defining an implicit class that inherits from some other classes that define the methods. Exports in extension methods provide a similar functionality without relying on implicit conversions under the covers.
Revert "Add note on export of extensions" This reverts commit 77447c8.
04ad901
to
3c55674
Compare
It should ideally be under an experimental import until SIP approval. Until it's officially back on its feet, though, I have bigger fights to fight; I would expect this change to be reasonably uncontroversial. Not that my opinion actually matters ... |
I was really, really enthusiastic about this capability but upon further examination it doesn't seem to help me at all 😢. |
in the thread https://contributors.scala-lang.org/t/proposed-changes-and-restrictions-for-implicit-conversions/4923 we discussed two changes that would make implicit conversions largely redundant. One proposed change was implemented in #14497. The other is implemented here. It's based on #14497. The idea of this PR is to have some way to specify that a method parameter accepts implicit conversions on its arguments. Then the feature warning on implicit conversion use would be suppressed in this case. In the previous thread I proposed `~` to mark convertible arguments but I now feel this is too cryptic. Instead there is a `into` prefix in the parameter type. E.g. ```scala def f(x: into T) = ... ``` For Scala 2, we introduce an annotation on the parameter that expresses the same thing: ```scala // proposed Scala-2 syntax: def f(@allowConversions x: T) = ``` A larger example: ```scala given Conversion[String, Text] = Text(_) @main def Test = def f(x: into Text, y: => into Text, zs: into Text*) = println(s"${x.str} ${y.str} ${zs.map(_.str).mkString(" ")}") f("abc", "def") // ok, no feature warning f("abc", "def", "xyz", "uvw") // also ok f("abc", "def", "xyz", Text("uvw")) // also ok ``` A larger example is a parser combinator library that works without unrestricted implicit conversions: [tests/run/Parser.scala](https://github.com/lampepfl/dotty/blob/0f113da1bacc01603740cde4a5bafbfae39895f0/tests/run/Parser.scala)
This is part of a long term strategy to deprecate and remove general implicit conversions in Scala.
In the thread https://contributors.scala-lang.org/t/can-we-wean-scala-off-implicit-conversions/4388
we identified two areas where implicit conversions or implicit classes were still essential. One was
adding methods to new types in bulk. This can be achieved by defining an implicit class that inherits
from some other classes that define the methods. Exports in extension clauses provide a similar
functionality without relying on implicit conversions under the covers.