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.

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

Retain HasDefaultParams flag on export. #14051

Merged
merged 5 commits into from Dec 17, 2021

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Dec 6, 2021

Fixes #14020

Copy link
Member

@bishabosha bishabosha left a comment

Choose a reason for hiding this comment

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

If a given has default parameters, then the user has to export named members as well as given members, when I would expect that they only need to export the given members:

class Dog {
  given bark(using msg: String = "Woof!"): String = s"bark: $msg"
}

class Wolf {
  private val dog = Dog()
  export dog.given // needs to be `export dog.{given, *}` to export the default arguments
}

val w = Wolf()
import w.given

summon[String] // error: I found: w.bark(/* missing */summon[String])

@odersky
Copy link
Contributor Author

odersky commented Dec 6, 2021

@bishabosha Fixing either of these things would create quite a big mess. I propose we wait until we have fixed the default parameter scheme to make default parameters inline expressions and not use default getters at all, then these things should become free. Even though fixing the default parameter scheme will be hard, it looks like a better investment of effort than fiddling with default getters here.

I propose we merge this now, after transferring your comment to the issue (which should be left open).

EDIT: I changed my mind. We can't have a discrepancy between named and unnamed here since default getters are not a visible language feature. So let me try to address the issues.

@odersky odersky force-pushed the fix-14020 branch 2 times, most recently from 95457b2 to 57146f7 Compare December 7, 2021 12:27
@bishabosha bishabosha self-requested a review December 8, 2021 10:08
@@ -0,0 +1,35 @@
class A:
def greeting(name: String = "you") = s"Hello $name"
Copy link
Member

@bishabosha bishabosha Dec 8, 2021

Choose a reason for hiding this comment

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

I tried making greeting inline and now we get a crash when calling c.greeting(), (before this PR is just the standard missing argument error)

stack trace
scala> c.greeting()
[error] (run-main-2) java.lang.IllegalArgumentException: Could not find proxy for name: String in List(val name, method greeting, class A, module class rs$line$1$, module class repl$, module class <root>), encl = package repl, owners = package repl, package <root>; enclosures = package repl, package <root>
[error] java.lang.IllegalArgumentException: Could not find proxy for name: String in List(val name, method greeting, class A, module class rs$line$1$, module class repl$, module class <root>), encl = package repl, owners = package repl, package <root>; enclosures = package repl, package <root>
[error] 	at dotty.tools.dotc.transform.LambdaLift$Lifter.searchIn$1(LambdaLift.scala:148)
[error] 	at dotty.tools.dotc.transform.LambdaLift$Lifter.proxy(LambdaLift.scala:161)
[error] 	at dotty.tools.dotc.transform.LambdaLift$Lifter.proxyRef(LambdaLift.scala:179)
[error] 	at dotty.tools.dotc.transform.LambdaLift$Lifter.addFreeArgs$$anonfun$1(LambdaLift.scala:185)
[error] 	at scala.collection.immutable.List.map(List.scala:246)
[error] 	at dotty.tools.dotc.transform.LambdaLift$Lifter.addFreeArgs(LambdaLift.scala:185)
[error] 	at dotty.tools.dotc.transform.LambdaLift.transformApply(LambdaLift.scala:322)
[error] 	at dotty.tools.dotc.transform.LambdaLift.transformApply(LambdaLift.scala:321)
[error] 	at dotty.tools.dotc.transform.MegaPhase.goApply(MegaPhase.scala:644)
[error] 	at dotty.tools.dotc.transform.MegaPhase.transformUnnamed$1(MegaPhase.scala:281)
[error] 	at dotty.tools.dotc.transform.MegaPhase.transformTree(MegaPhase.scala:429)
[error] 	at dotty.tools.dotc.transform.MegaPhase.mapValDef$1(MegaPhase.scala:235)
[error] 	at dotty.tools.dotc.transform.MegaPhase.transformNamed$1(MegaPhase.scala:240)
[error] 	at dotty.tools.dotc.transform.MegaPhase.transformTree(MegaPhase.scala:427)
[error] 	at dotty.tools.dotc.transform.MegaPhase.transformStat$1(MegaPhase.scala:437)
[error] 	at dotty.tools.dotc.transform.MegaPhase.recur$1(MegaPhase.scala:442)
[error] 	at dotty.tools.dotc.transform.MegaPhase.transformStats(MegaPhase.scala:442)
[error] 	at dotty.tools.dotc.transform.MegaPhase.mapPackage$1(MegaPhase.scala:382)
[error] 	at dotty.tools.dotc.transform.MegaPhase.transformUnnamed$1(MegaPhase.scala:385)
[error] 	at dotty.tools.dotc.transform.MegaPhase.transformTree(MegaPhase.scala:429)
[error] 	at dotty.tools.dotc.transform.MegaPhase.transformUnit(MegaPhase.scala:448)
[error] 	at dotty.tools.dotc.transform.MegaPhase.run(MegaPhase.scala:460)
[error] 	at dotty.tools.dotc.core.Phases$Phase.runOn$$anonfun$1(Phases.scala:308)
[error] 	at scala.collection.immutable.List.map(List.scala:246)
[error] 	at dotty.tools.dotc.core.Phases$Phase.runOn(Phases.scala:309)
[error] 	at dotty.tools.dotc.Run.runPhases$1$$anonfun$1(Run.scala:261)
[error] 	at scala.runtime.function.JProcedure1.apply(JProcedure1.java:15)
[error] 	at scala.runtime.function.JProcedure1.apply(JProcedure1.java:10)
[error] 	at scala.collection.ArrayOps$.foreach$extension(ArrayOps.scala:1323)
[error] 	at dotty.tools.dotc.Run.runPhases$1(Run.scala:272)
[error] 	at dotty.tools.dotc.Run.compileUnits$$anonfun$1(Run.scala:280)
[error] 	at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.scala:18)
[error] 	at dotty.tools.dotc.util.Stats$.maybeMonitored(Stats.scala:68)
[error] 	at dotty.tools.dotc.Run.compileUnits(Run.scala:289)
[error] 	at dotty.tools.dotc.Run.compileUnits(Run.scala:228)
[error] 	at dotty.tools.repl.ReplCompiler.runCompilationUnit(ReplCompiler.scala:167)
[error] 	at dotty.tools.repl.ReplCompiler.compile(ReplCompiler.scala:178)
[error] 	at dotty.tools.repl.ReplDriver.compile(ReplDriver.scala:251)
[error] 	at dotty.tools.repl.ReplDriver.interpret(ReplDriver.scala:219)
[error] 	at dotty.tools.repl.ReplDriver.loop$1(ReplDriver.scala:153)
[error] 	at dotty.tools.repl.ReplDriver.runUntilQuit$$anonfun$1(ReplDriver.scala:156)
[error] 	at dotty.tools.repl.ReplDriver.withRedirectedOutput(ReplDriver.scala:175)
[error] 	at dotty.tools.repl.ReplDriver.runUntilQuit(ReplDriver.scala:156)
[error] 	at xsbt.ConsoleInterface.run(ConsoleInterface.java:52)
[error] 	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
[error] 	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
[error] 	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
[error] 	at java.lang.reflect.Method.invoke(Method.java:498)
[error] 	at sbt.internal.inc.AnalyzingCompiler.invoke(AnalyzingCompiler.scala:329)
[error] 	at sbt.internal.inc.AnalyzingCompiler.console(AnalyzingCompiler.scala:233)
[error] 	at sbt.Console.console0$1(Console.scala:64)
[error] 	at sbt.Console.$anonfun$apply$5(Console.scala:74)
[error] 	at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:23)
[error] 	at sbt.util.InterfaceUtil$$anon$1.get(InterfaceUtil.scala:17)
[error] 	at sbt.TrapExit$App.run(TrapExit.scala:258)
[error] 	at java.lang.Thread.run(Thread.java:748)
[error] Nonzero exit code: 1
[error] (scala3-compiler-bootstrapped / Compile / console) Nonzero exit code: 1
[error] Total time: 32 s, completed Dec 8, 2021 11:09:44 AM

@bishabosha bishabosha self-requested a review December 8, 2021 10:10
Copy link
Member

@bishabosha bishabosha left a comment

Choose a reason for hiding this comment

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

I don't think I can approve this before the issue with inline methods is resolved, maybe there is a separate underlying issue



class Dog:
given bark(using msg: String = "Woof!"): String = s"bark: $msg"
Copy link
Member

Choose a reason for hiding this comment

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

I have found one last issue which is by making bark inline:

class Dog:
   inline given bark(using msg: String = "Woof!"): String = s"bark: $msg"

gives

-- Error: ----------------------------------------------------------------------
31 |summon[String]
   |              ^
   |value dog cannot be accessed as a member of (Wolf_this : (w : Wolf)) from module class i14020$package$.
   | This location contains code that was inlined from i14020.scala:39
1 error found

Copy link
Member

Choose a reason for hiding this comment

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

you don't get the same error if you try to manually forward to dog.bark in Wolf:

class Wolf:
  private val dog = Dog()
  inline given bark(using msg: String = "Woof!"): String = dog.bark(using msg)

Copy link
Member

@bishabosha bishabosha left a comment

Choose a reason for hiding this comment

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

I found another issue which may be due to trying to access dog s default getter in the inlined code of w.bark

@odersky
Copy link
Contributor Author

odersky commented Dec 17, 2021

@bishabosha Can you open another issue for this? I don't have time to work on it, and I think as far as the original problem goes, everything is addressed. We should not let this fix wait indefinitely awaiting the resolution of the other issue.

@bishabosha
Copy link
Member

@bishabosha Can you open another issue for this?

made issue #14131

@bishabosha bishabosha self-requested a review December 17, 2021 20:02
@Kordyjan Kordyjan added this to the 3.1.2 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Export clauses don't forward the default values of function's parameters
3 participants