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

Chaining Java functions using -Yexplicit-nulls and scala.language.unsafeNulls fails compilation #14592

Closed
martingd opened this issue Mar 1, 2022 · 7 comments
Assignees

Comments

@martingd
Copy link

martingd commented Mar 1, 2022

Compiler version

Scala 3.1.1

Minimized code

Compiling this code with compiler option -Yexplicit-nulls results in a compilation error:

import scala.language.unsafeNulls

import java.nio.file.Files
import java.nio.file.Path
import java.nio.file.Paths

val x = Files.getFileStore(Paths.get("")).name
val y = Thread.currentThread.getContextClassLoader

Output

The result of compiling the above code:

% scala3-compiler -version
Scala compiler version 3.1.1 -- Copyright 2002-2022, LAMP/EPFL
% scala3-compiler -Yexplicit-nulls test.scala 
-- Error: test.scala:7:8 -------------------------------------------------------
7 |val x = Files.getFileStore(Paths.get("")).name
  |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |undefined: java.nio.file.Files.getFileStore(
  |  java.nio.file.Paths.get("", [ : String | Null]*)
  |).name # -1: TermRef(OrType(TypeRef(ThisType(TypeRef(NoPrefix,module class file)),class FileStore),TypeRef(ThisType(TypeRef(NoPrefix,module class scala)),class Null)),name) at sbt-api
-- Error: test.scala:8:8 -------------------------------------------------------
8 |val y = Thread.currentThread.getContextClassLoader
  |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |undefined: Thread.currentThread().getContextClassLoader # -1: TermRef(OrType(TypeRef(ThisType(TypeRef(NoPrefix,module class lang)),class Thread),TypeRef(ThisType(TypeRef(NoPrefix,module class scala)),class Null)),getContextClassLoader) at sbt-api
2 errors found

Expectation

The code should compile without error.

Additional comments

When compiling the above code, the compiler emits two errors. The error is only triggered when using compiler option -Yexplicit-nulls and also including import scala.language.unsafeNulls in the scope of the lines that fail compilation.

Any call to a Java function (returning T | Null for some type T) that is chained from a call to another Java function triggers this condition.

Making a similar chain of functions using code defined in Scala, like in the code below, does not trigger the compiler error:

import scala.language.unsafeNulls

object X:
    def f: Y | Null = ???
class Y:
    def g: Z | Null = ???
class Z

val z = X.f.g

Adding explicit .nn calls in the failing code works around the bug – with import scala.language.unsafeNulls still in place.

For example, this code compiles without error:

import scala.language.unsafeNulls

import java.nio.file.Files
import java.nio.file.Path
import java.nio.file.Paths

val x = Files.getFileStore(Paths.get("")).nn.name
val y = Thread.currentThread.nn.getContextClassLoader

Another workaround is to split the chained calls as the bug is only triggered when chaining calls.
This code compiles:

import scala.language.unsafeNulls

import java.nio.file.Files
import java.nio.file.Path
import java.nio.file.Paths

val x = Files.getFileStore(Paths.get(""))
val xx = x.name
val y = Thread.currentThread
val yy = y.getContextClassLoader
@martingd martingd added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Mar 1, 2022
@bishabosha bishabosha added stat:fix available and removed stat:needs triage Every issue needs to have an "area" and "itype" label itype:bug labels Mar 1, 2022
@bishabosha
Copy link
Member

this was actually fixed in #14523, (unreleased yet) we can add some regression tests

@bishabosha bishabosha self-assigned this Mar 1, 2022
@martingd
Copy link
Author

martingd commented Mar 1, 2022

@bishabosha I tried to search but I can see it is exactly the same issue as #14319
Should we close this issue?

@bishabosha
Copy link
Member

I'll add a test case to replicate the code, just so we cover more examples

@martingd
Copy link
Author

martingd commented Mar 1, 2022

@bishabosha IMHO my examples looks like exact replicas of the example on the original test so this will just make the tests run slower with no added benefit. But it is up to you.

@som-snytt
Copy link
Contributor

The fix, to "thread context" correctly, puns on this example, Thread.currentThread.getContextClassLoader. A test could prove quite punning.

@martingd
Copy link
Author

martingd commented Mar 1, 2022

The example I refer to is not from the PR but from issue #14319 which has this example:

// -Yforce-sbt-phases -Yexplicit-nulls
import java.nio.file.FileSystems

def directorySeparator: String = {
  import scala.language.unsafeNulls
  FileSystems.getDefault().getSeparator()
}

I would say that in the context of this bug, FileSystems.getDefault().getSeparator() is identical to Thread.currentThread.getContextClassLoader.

Do we really need another test?

@martingd martingd closed this as completed Mar 2, 2022
@martingd
Copy link
Author

martingd commented Mar 2, 2022

Duplicate of issue #14319.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants