Skip to content

String#split(Char) extension method stopped working on JDK 14 #8838

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

Closed
SethTisue opened this issue Apr 29, 2020 · 8 comments · Fixed by #9081
Closed

String#split(Char) extension method stopped working on JDK 14 #8838

SethTisue opened this issue Apr 29, 2020 · 8 comments · Fixed by #9081

Comments

@SethTisue
Copy link
Member

it's fine on 8, 11, and 13

sbt:euler> show scalaVersion
[info] 0.24.0-RC1
sbt:euler> console

scala> System.getProperty("java.version")                                                               
val res0: String = 14.0.1

scala> "foo".split(' ')
1 |"foo".split(' ')
  |            ^^^
  |            Found:    (' ' : Char)
  |            Required: String

this can also be seen when Dotty tries to build itself on 14:

[error] -- [E007] Type Mismatch Error: /Users/tisue/dotty/doc-tool/src/dotty/tools/dottydoc/staticsite/DefaultParams.scala:72:38 
[error] 72 |  val path: Array[String] = url.split('/').drop(1)
[error]    |                                      ^^^
[error]    |                                      Found:    ('/' : Char)
[error]    |                                      Required: String
[error] one error found

the problem also exists in 0.23.0-RC1; I didn't try testing farther back than that

@smarter
Copy link
Member

smarter commented Apr 29, 2020

Very weird. I can reproduce the compilation error in the dotty build, but once I fix it, I don't actually see any error when I attempt to compile a file that uses this extension method, but I get a println:

Caught: dotty.tools.dotc.core.CyclicReference:  while parsing annotations in /modules/java.base/java/lang/annotation/ElementType.class

In what repo exactly did you run sbt console? Can you reproduce this in github.com/lampepfl/dotty-example-project ?

@smarter
Copy link
Member

smarter commented Apr 29, 2020

The cycle in question does involve String:

          completing val stripIndent
            completing type PreviewFeature
              completing val annotation
                completing type annotation
                completed annotation in package java.lang
              completed annotation in package java.lang
              completing type ElementType
                completing type Enum
                  completing val <init>
                    completing type E
                      completing type E
                      completed E in class Enum
                    completed <init> in class Enum
                  completed Enum in package java.lang
                  completing val <init>
                  completed <init> in class ElementType
                  completing val TYPE
                  completed TYPE in object ElementType
                  completing val FIELD
                  completed FIELD in object ElementType
                  completing val METHOD
                  completed METHOD in object ElementType
                  completing val PARAMETER
                  completed PARAMETER in object ElementType
                  completing val CONSTRUCTOR
                  completed CONSTRUCTOR in object ElementType
                  completing val LOCAL_VARIABLE
                  completed LOCAL_VARIABLE in object ElementType
                  completing val ANNOTATION_TYPE
                  completed ANNOTATION_TYPE in object ElementType
                  completing val PACKAGE
                  completed PACKAGE in object ElementType
                  completing val TYPE_PARAMETER
                  completed TYPE_PARAMETER in object ElementType
                  completing val TYPE_USE
                  completed TYPE_USE in object ElementType
                  completing val MODULE
                  completed MODULE in object ElementType
                  completing val RECORD_COMPONENT
                    completing type PreviewFeature
  1. stripIdent in Java 14 is annotated @jdk.internal.PreviewFeature(feature=jdk.internal.PreviewFeature.Feature.TEXT_BLOCKS, essentialAPI=true)
  2. PreviewFeature itself is annotated @Target({ElementType.METHOD, ...
  3. ElementType is an enum with an entry RECORD_COMPONENT which is annotated @jdk.internal.PreviewFeature

So we need to be lazier when parsing annotations in some way, it'd be interesting to figure out what scalac ClassfileParser is doing differently here.

@smarter
Copy link
Member

smarter commented Apr 29, 2020

The part which seems different from scalac is that when loading the Java enum class ElementType, we currently force completion of all its members:
https://github.com/lampepfl/dotty/blob/c5a76f0db4c2908520a297414cf59a9a207fb11f/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala#L204-L208
@liufengyun do you remember why forcing completion at this point is needed? Do you think it could be done later, when we do the exhaustivity checking itself?

@SethTisue
Copy link
Member Author

SethTisue commented Apr 29, 2020

Can you reproduce this in github.com/lampepfl/dotty-example-project ?

yes, but only if I add -Xfatal-warnings to scalacOptions (!)

I guess this is related to #8688? (or is it a red herring that they both involve CyclicReference?)

@smarter
Copy link
Member

smarter commented Apr 29, 2020

The CyclicReference in that issue is the same thing yeah, it's just that I hadn't been able to reproduce it so far.

@liufengyun
Copy link
Contributor

@smarter I just noticed. It's for checking exhaustivity of java enums. If it's lazy, an enum may never register, thus the exhaustivity check may not be able to know all enums.

However, it might be possible to move the logic to exhaustivity check. Can you push your fix, remove the two lines, and I can fix the failed tests?

@smarter
Copy link
Member

smarter commented May 4, 2020

Can you push your fix

I don't have a specific fix except removing these lines, do you want me to open a PR that just removes these lines?

@liufengyun liufengyun self-assigned this May 4, 2020
@liufengyun
Copy link
Contributor

Then I'll have a look at the issue and propose a PR.

liufengyun added a commit to dotty-staging/dotty that referenced this issue May 29, 2020
liufengyun added a commit to dotty-staging/dotty that referenced this issue May 29, 2020
liufengyun added a commit to dotty-staging/dotty that referenced this issue May 29, 2020
smarter added a commit that referenced this issue May 29, 2020
Fix #8838: load Java enum members lazily to avoid cycles
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants