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

Make main methods invisible #11546

Merged
merged 6 commits into from
Mar 9, 2021
Merged

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Feb 26, 2021

Add a new flag Invisible that gets set for compiler-generated main methods.
Invisible symbols are skipped when resolving members during typechecking.
They become visible after typechecking.

With that mechanism, we can accept main methods that are themselves called main.

Add a new flag Invisible that gets set for compiler-generated main methods.
Invisible symbols are skipped when resolving members during typechecking.
They become visible after typechecking.

With that mechanism, we can accept main methods that are themselves called `main`.
Load bean properties in TreeUnpickler but make them invisible to Typer.
This looks like the safer choice.
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.

this approval is to acknowledge the changes to the Tasty version, I have not checked the rest

@bishabosha bishabosha removed their assignment Feb 28, 2021
@odersky
Copy link
Contributor Author

odersky commented Feb 28, 2021

@smarter Do you want to take a look?

val isScala2MacroDefinedInScala3 = flags.is(Macro, butNot = Inline) && flags.is(Erased)
ctx.owner match {
case cls: ClassSymbol if (!isScala2MacroDefinedInScala3 || cls == defn.StringContextClass) && !isSyntheticBeanAccessor =>
case cls: ClassSymbol if !isScala2MacroDefinedInScala3 || cls == defn.StringContextClass =>
// Enter all members of classes that are not Scala 2 macros or synthetic bean accessors.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Enter all members of classes that are not Scala 2 macros or synthetic bean accessors.
// Enter all members of classes that are not Scala 2 macros

@@ -670,6 +668,7 @@ class TreeUnpickler(reader: TastyReader,
case PARAMalias => addFlag(SuperParamAlias)
case EXPORTED => addFlag(Exported)
case OPEN => addFlag(Open)
case INVISIBLE => addFlag(Invisible)
Copy link
Member

Choose a reason for hiding this comment

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

This flag should be visible in tasty-reflect too /cc @nicolasstucki

Comment on lines 477 to 478
final val SPLITCLAUSE = 45
final val INVISIBLE = 46
Copy link
Member

Choose a reason for hiding this comment

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

isLegalTag in this file still says:

firstSimpleTreeTag <= tag && tag <= SPLITCLAUSE ||

presumably that should be replaced with INVISIBLE (we should really add something like final val LAST_SIMPLE_TAG = INVISIBLE to avoid having to update multiple places like this).

@@ -211,7 +211,8 @@ Standard-Section: "ASTs" TopLevelStat*
PARAMsetter -- The setter part `x_=` of a var parameter `x` which itself is pickled as a PARAM
PARAMalias -- Parameter is alias of a superclass parameter
EXPORTED -- An export forwarder
OPEN -- an open class
OPEN
INVISIBLE -- an open class
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment on wrong line (and inconsistently indented).

@Jasper-M
Copy link
Member

Jasper-M commented Mar 1, 2021

Does it make the generated class invisible? Or also the annotated method?

@julienrf
Copy link
Collaborator

julienrf commented Mar 2, 2021

I’ve noticed that when using the REPL I can’t call methods annotated with @main:

package example

def foo(): Unit = println("foo")

@main def run(): Unit = println("run")

Then, in the REPL,

scala> example.foo()
foo

scala> example.run()
1 |example.run()
  |^
  |Toplevel definition run is defined in
  |  /home/julien/example/target/scala-3.0.0-RC1/classes/example/Main$package.class
  |and also in
  |  /home/julien/example/target/scala-3.0.0-RC1/classes/example/run.class
  |One of these files should be removed from the classpath.

Would this PR solve the issue? Or should I open a new issue?

@julienrf
Copy link
Collaborator

julienrf commented Mar 2, 2021

BTW, it also makes the completion crash:

INFO: Error while finding completion candidates
dotty.tools.dotc.core.TypeError: Toplevel definition run is defined in
  /home/julien/example/target/scala-3.0.0-RC1/classes/example/Main$package.class
and also in
  /home/julien/example/target/scala-3.0.0-RC1/classes/example/run.class
One of these files should be removed from the classpath.
        at dotty.tools.dotc.core.SymDenotations$PackageClassDenotation.dropStale$1(SymDenotations.scala:2330)
        at dotty.tools.dotc.core.SymDenotations$PackageClassDenotation.recur$1(SymDenotations.scala:2294)
        at dotty.tools.dotc.core.SymDenotations$PackageClassDenotation.computeMembersNamed(SymDenotations.scala:2350)
        at dotty.tools.dotc.core.SymDenotations$ClassDenotation.membersNamed(SymDenotations.scala:1914)
        at dotty.tools.dotc.core.SymDenotations$ClassDenotation.findMember(SymDenotations.scala:1965)
        at dotty.tools.dotc.core.Types$Type.go$1(Types.scala:655)
        at dotty.tools.dotc.core.Types$Type.findMember(Types.scala:843)
        at dotty.tools.dotc.core.Types$Type.memberBasedOnFlags(Types.scala:638)
        at dotty.tools.dotc.core.Types$Type.member(Types.scala:622)
        at dotty.tools.dotc.interactive.Completion$CompletionBuffer.addMember(Completion.scala:292)
        at dotty.tools.dotc.interactive.Completion$CompletionBuffer.addAccessibleMembers$$anonfun$1(Completion.scala:345)
        at scala.runtime.function.JProcedure1.apply(JProcedure1.java:15)
        at scala.runtime.function.JProcedure1.apply(JProcedure1.java:10)
        at scala.collection.immutable.List.foreach(List.scala:333)
        at dotty.tools.dotc.interactive.Completion$CompletionBuffer.addAccessibleMembers(Completion.scala:345)
        at dotty.tools.dotc.interactive.Completion$CompletionBuffer.addMemberCompletions(Completion.scala:210)
        at dotty.tools.dotc.interactive.Completion$CompletionBuffer.addSelectionCompletions(Completion.scala:276)
        at dotty.tools.dotc.interactive.Completion$.computeCompletions(Completion.scala:122)
        at dotty.tools.dotc.interactive.Completion$.completions(Completion.scala:48)
        at dotty.tools.repl.ReplDriver.completions$$anonfun$1(ReplDriver.scala:188)
        at scala.util.Either.map(Either.scala:382)
        at dotty.tools.repl.ReplDriver.completions(ReplDriver.scala:189)
        at dotty.tools.repl.ReplDriver.$anonfun$3(ReplDriver.scala:113)
        at org.jline.reader.impl.LineReaderImpl.doComplete(LineReaderImpl.java:4397)
        at org.jline.reader.impl.LineReaderImpl.doComplete(LineReaderImpl.java:4363)
        at org.jline.reader.impl.LineReaderImpl.expandOrComplete(LineReaderImpl.java:4302)
        at org.jline.reader.impl.LineReaderImpl$1.apply(LineReaderImpl.java:3797)
        at org.jline.reader.impl.LineReaderImpl.readLine(LineReaderImpl.java:665)
        at org.jline.reader.impl.LineReaderImpl.readLine(LineReaderImpl.java:454)
        at dotty.tools.repl.JLineTerminal.readLine(JLineTerminal.scala:71)
        at dotty.tools.repl.ReplDriver.readLine$1(ReplDriver.scala:118)
        at dotty.tools.repl.ReplDriver.loop$1(ReplDriver.scala:128)
        at dotty.tools.repl.ReplDriver.runUntilQuit$$anonfun$1(ReplDriver.scala:133)
        at dotty.tools.repl.ReplDriver.withRedirectedOutput(ReplDriver.scala:152)
        at dotty.tools.repl.ReplDriver.runUntilQuit(ReplDriver.scala:133)
        at xsbt.ConsoleInterface.run(ConsoleInterface.java:52)

@bishabosha
Copy link
Member

bishabosha commented Mar 2, 2021

I’ve noticed that when using the REPL I can’t call methods annotated with @main:
Would this PR solve the issue? Or should I open a new issue?

I just tried it out and yes, with this PR the repl does call the method run and not error (I compiled the source separately and added the results to the classpath before I ran the repl)

@odersky
Copy link
Contributor Author

odersky commented Mar 5, 2021

@nicolasstucki Can you check the last commit please?

final val EMPTYCLAUSE = 44
final val SPLITCLAUSE = 45
final val INVISIBLE = 46
final val INVISIBLE = 44
Copy link
Member

Choose a reason for hiding this comment

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

normally this would require a MajorVersion bump, but as we have not yet had a stable ExperimentalVersion this is fine

@odersky
Copy link
Contributor Author

odersky commented Mar 9, 2021

@nicolasstucki Could you have a quick review of the last commit, then we can merge this.

Copy link
Contributor

@nicolasstucki nicolasstucki left a comment

Choose a reason for hiding this comment

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

The changes in the reflection interface look good

@nicolasstucki nicolasstucki removed their assignment Mar 9, 2021
@nicolasstucki
Copy link
Contributor

@smarter are all the requested changes Ok? Can we merge?

@smarter smarter merged commit e6be077 into scala:master Mar 9, 2021
@smarter smarter deleted the fix-main-shadowing branch March 9, 2021 12:31
@BarkingBad
Copy link
Member

I think this PR brought some regression. Having this branch BarkingBad:scaladoc/scala3-docs-in-jar rebased on top of 86e8fb7174e2cfca42067e8be1ccd7dcc9250cd1 commit and executing task tasty-core-bootstrapped/Compile/doc is successful, but rebasing onto e6be077a56aac88c6e714243e153e58b52eb0445 or any subsequent commit fails with given error message:

[error] error while loading package$,
[error] class file dotty/tools/package.class is broken, reading aborted with class dotty.tools.tasty.UnpickleException
[error] TASTy signature has wrong version.
[error]  expected: {majorVersion: 28, minorVersion: 0 [unstable release: 2]}
[error]  found   : {majorVersion: 28, minorVersion: 0 [unstable release: 1]}
[error] 
[error] This TASTy file was produced by an unstable release.
[error] To read this TASTy file, your tooling must be at the same version.
[error] The TASTy file was produced by Scala (unknown).
[error] Note that your tooling is currently using an unstable TASTy version.
[error] one error found
[error] (tasty-core-bootstrapped / Compile / doc) DottyDoc Compilation Failed
[error] Total time: 127 s (02:07), completed 12 mar 2021, 18:35:34

You can find more details in CI actions at this PR (rebased on the newest master) #11578

@smarter
Copy link
Member

smarter commented Mar 12, 2021

@BarkingBad This doesn't sound like a bug in this PR, it sounds like the scaladoc task is doing something wrong and trying to use tasty files generated by the reference compiler with the current compiler, but those are intentionally incompatible.

@Kordyjan Kordyjan added this to the 3.0.0 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.

None yet

10 participants