Skip to content

Port Constant Types for Literal Final Static Java Fields and Implicit Conversions from Scalac #7483

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

Merged
merged 3 commits into from
Nov 7, 2019

Conversation

noti0na1
Copy link
Member

@noti0na1 noti0na1 commented Nov 1, 2019

Currently, the ClassParser will give ConstantTypes to constant fields, but the JavaParser will not, because the JavaParser in Dotty doesn’t parse the expressions of fields. This PR ports the parsing literals part from Scalac, so that ConstantTypes will be assigned to constant Java fields with simple literals.

SI-3236 constant types for literal final static java fields
scala/scala@81d2c61

Support implicit converstions from java literals
scala/scala@74c613b

This is also useful for the explicit nulls #6344. With this feature, we don’t need to nullify constant fields.

I am not able to port the test of SI-3236 directly, since the annotation arguments in Dotty can only refers to enum values (in DottyBackendInterface.emitArgument)? If I’m wrong, please correct me.

Edit: The SI-3236 test is now ported.

@noti0na1 noti0na1 requested a review from smarter November 1, 2019 17:44
Copy link
Member

@dottybot dottybot left a comment

Choose a reason for hiding this comment

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

Hello, and thank you for opening this PR! 🎉

All contributors have signed the CLA, thank you! ❤️

Have an awesome day! ☀️

@smarter
Copy link
Member

smarter commented Nov 2, 2019

I am not able to port the test of SI-3236 directly, since the annotation arguments in Dotty can only refers to enum values (in DottyBackendInterface.emitArgument)?

Not sure I understand, what error are you seeing with that test exactly ?

@noti0na1
Copy link
Member Author

noti0na1 commented Nov 4, 2019

@smarter Here is the code which triggers the same error:

J.java

import java.lang.annotation.*;

@Retention(RetentionPolicy.RUNTIME)
public @interface J {
    String value();
}

S.scala

object C {
  val cs: String = "cs"
}

object S {
  @J(C.cs)
  def f(): Int = 1
}

dotc J.java S.scala:

Error while emitting S.scala
exception occurred while compiling mytest/annot/S.scala
java.lang.AssertionError: assertion failed: not an enum: C.cs / getter cs / object C / false / final module <touched> while compiling mytest/annot/J.java, mytest/annot/S.scala
Exception in thread "main" java.lang.AssertionError: assertion failed: not an enum: C.cs / getter cs / object C / false / final module <touched>
	at dotty.DottyPredef$.assertFail(DottyPredef.scala:17)
	at dotty.tools.backend.jvm.DottyBackendInterface.emitArgument(DottyBackendInterface.scala:261)
	at dotty.tools.backend.jvm.DottyBackendInterface.emitAssocs$$anonfun$2(DottyBackendInterface.scala:325)
	at dotty.runtime.function.JProcedure1.apply(JProcedure1.java:15)
	at dotty.runtime.function.JProcedure1.apply(JProcedure1.java:10)
	at scala.collection.IterableOnceOps.foreach(IterableOnce.scala:553)
	at scala.collection.IterableOnceOps.foreach$(IterableOnce.scala:551)
	at scala.collection.AbstractIterable.foreach(Iterable.scala:921)
	at scala.collection.IterableOps$WithFilter.foreach(Iterable.scala:891)
	at dotty.tools.backend.jvm.DottyBackendInterface.emitAssocs(DottyBackendInterface.scala:325)
	at dotty.tools.backend.jvm.DottyBackendInterface.emitAnnotations$$anonfun$4(DottyBackendInterface.scala:335)
	at dotty.runtime.function.JProcedure1.apply(JProcedure1.java:15)
	at dotty.runtime.function.JProcedure1.apply(JProcedure1.java:10)
	at scala.collection.IterableOnceOps.foreach(IterableOnce.scala:553)
	at scala.collection.IterableOnceOps.foreach$(IterableOnce.scala:551)
	at scala.collection.AbstractIterable.foreach(Iterable.scala:921)
	at scala.collection.IterableOps$WithFilter.foreach(Iterable.scala:891)
	at dotty.tools.backend.jvm.DottyBackendInterface.emitAnnotations(DottyBackendInterface.scala:336)
	at dotty.tools.backend.jvm.BCodeHelpers$BCAnnotGen.emitAnnotations(BCodeHelpers.scala:274)
	at dotty.tools.backend.jvm.BCodeHelpers$JCommonBuilder.emitAnnotations(BCodeHelpers.scala:440)
	at dotty.tools.backend.jvm.BCodeHelpers$BCForwardersGen.addForwarder(BCodeHelpers.scala:342)
	at dotty.tools.backend.jvm.BCodeHelpers$BCForwardersGen.addForwarders$$anonfun$2(BCodeHelpers.scala:393)
	at dotty.runtime.function.JProcedure1.apply(JProcedure1.java:15)
	at dotty.runtime.function.JProcedure1.apply(JProcedure1.java:10)
	at scala.collection.immutable.List.foreach(List.scala:305)
	at dotty.tools.backend.jvm.BCodeHelpers$BCForwardersGen.addForwarders(BCodeHelpers.scala:395)
	at dotty.tools.backend.jvm.BCodeHelpers$JCommonBuilder.addForwarders(BCodeHelpers.scala:440)
	at dotty.tools.backend.jvm.BCodeHelpers$JMirrorBuilder.genMirrorClass(BCodeHelpers.scala:488)
	at dotty.tools.backend.jvm.GenBCodePipeline$Worker1.visit(GenBCode.scala:208)
	at dotty.tools.backend.jvm.GenBCodePipeline$Worker1.run(GenBCode.scala:184)
	at dotty.tools.backend.jvm.GenBCodePipeline.buildAndSendToDisk(GenBCode.scala:514)
	at dotty.tools.backend.jvm.GenBCodePipeline.run(GenBCode.scala:480)
	at dotty.tools.backend.jvm.GenBCode.run(GenBCode.scala:51)
	at dotty.tools.dotc.core.Phases$Phase.runOn$$anonfun$1(Phases.scala:315)
	at scala.collection.immutable.List.map(List.scala:219)
	at dotty.tools.dotc.core.Phases$Phase.runOn(Phases.scala:316)
	at dotty.tools.backend.jvm.GenBCode.runOn(GenBCode.scala:55)
	at dotty.tools.dotc.Run.runPhases$4$$anonfun$4(Run.scala:162)
	at dotty.runtime.function.JProcedure1.apply(JProcedure1.java:15)
	at dotty.runtime.function.JProcedure1.apply(JProcedure1.java:10)
	at scala.collection.ArrayOps$.foreach$extension(ArrayOps.scala:1323)
	at dotty.tools.dotc.Run.runPhases$5(Run.scala:172)
	at dotty.tools.dotc.Run.compileUnits$$anonfun$1(Run.scala:180)
	at dotty.runtime.function.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:12)
	at dotty.tools.dotc.util.Stats$.maybeMonitored(Stats.scala:65)
	at dotty.tools.dotc.Run.compileUnits(Run.scala:187)
	at dotty.tools.dotc.Run.compileSources(Run.scala:124)
	at dotty.tools.dotc.Run.compile(Run.scala:107)
	at dotty.tools.dotc.Driver.doCompile(Driver.scala:36)
	at dotty.tools.dotc.Driver.process(Driver.scala:189)
	at dotty.tools.dotc.Driver.process(Driver.scala:158)
	at dotty.tools.dotc.Driver.process(Driver.scala:170)
	at dotty.tools.dotc.Driver.main(Driver.scala:197)
	at dotty.tools.dotc.Main.main(Main.scala)

@smarter
Copy link
Member

smarter commented Nov 4, 2019

That code emits a "annotation argument is not a constant" in scalac, I've just created a PR to do the same for dotty: #7495

@noti0na1
Copy link
Member Author

noti0na1 commented Nov 4, 2019

@smarter Thanks for the crash fix!

However, with this fix, the annotation arguments still cannot be constants.
If we modify the code above: val cs: String = "cs" -> val cs: "cs" = "cs", then it can be compiled by scalac but not by dotty.

That's why I am not able to port the test of SI-3236.

@smarter
Copy link
Member

smarter commented Nov 4, 2019

OK, I've pushed an extra commit to #7495 which handles this.

@smarter
Copy link
Member

smarter commented Nov 7, 2019

If you rebase on master you should be able to add these tests now.

@noti0na1
Copy link
Member Author

noti0na1 commented Nov 7, 2019

@smarter Thanks, the SI-3236 test is now added.

@smarter smarter merged commit 3cb93f3 into scala:master Nov 7, 2019
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.

3 participants