Skip to content

Invalid file name for class files on Windows #7492

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
liufengyun opened this issue Nov 4, 2019 · 5 comments · Fixed by #7601
Closed

Invalid file name for class files on Windows #7492

liufengyun opened this issue Nov 4, 2019 · 5 comments · Fixed by #7601
Assignees

Comments

@liufengyun
Copy link
Contributor

liufengyun commented Nov 4, 2019

minimized code

Given the following code:

class A  {
  given (self: Int) {
    def =:(x: Int) = ???
    def f: Int = ???
  }
}

Dotty generates a class file A$given_=:_of_Int$.class, which is invalid on Windows, as : is not allowed in file names.

A walkaround is to switch the order of the two methods.

expectation

Dotty should not generate the file name A$given_=:_of_Int$.class which depend on method names.

Thanks to @michelou for reporting the bug.

@smarter
Copy link
Member

smarter commented Nov 4, 2019

Yep, I'm working on fixing #5936 which should fix this.

@liufengyun
Copy link
Contributor Author

Or, can we avoid using the method name for anonymous given?

@smarter
Copy link
Member

smarter commented Nov 4, 2019

Yes I think that's a bug, there's no reason to use the name of the first method, but fixing that won't fix the encoding problem with windows since : could still show up in the name of a given parameter.

This is a separate discussion but my proposal for generating names of givens is to simply reuse the source code definition of the given, so for example:

given (self: Int)  { ... }

would desugar to:

given `given (self: Int)` { ... }

This has multiple advantages:

smarter added a commit to dotty-staging/dotty that referenced this issue Nov 21, 2019
There were several issues with the scheme we used so far:
- It's different from Scala 2, meaning that some Scala 2 methods could
not be called from Dotty and vice-versa (see the added
sbt-dotty/sbt-test/scala2-compat/akka/i3100.scala test for an example)
- It can lead to invalid filenames on Windows (scala#7492)
- The handling of backticks is broken: adding or removing backticks
around a name changes how it's encoded.

To maintain Scala 2 compat we don't have a lot of choices here, we must
use the same scheme, so this commit aligns NameTransformer.scala with
https://github.com/scala/scala/blob/2.13.x/src/library/scala/reflect/NameTransformer.scala

Fixes scala#3100. Fixes scala#5936. Fixes scala#7492.
smarter added a commit to dotty-staging/dotty that referenced this issue Nov 21, 2019
There were several issues with the scheme we used so far:
- It's different from Scala 2, meaning that some Scala 2 methods could
not be called from Dotty and vice-versa (see the added
sbt-dotty/sbt-test/scala2-compat/akka/i3100.scala test for an example)
- It can lead to invalid filenames on Windows (scala#7492)
- The handling of backticks is broken: adding or removing backticks
around a name changes how it's encoded.

To maintain Scala 2 compat we don't have a lot of choices here, we must
use the same scheme, so this commit aligns NameTransformer.scala with
https://github.com/scala/scala/blob/2.13.x/src/library/scala/reflect/NameTransformer.scala

Fixes scala#3100. Fixes scala#5936. Fixes scala#7492.
smarter added a commit to dotty-staging/dotty that referenced this issue Nov 21, 2019
There were several issues with the scheme we used so far:
- It's different from Scala 2, meaning that some Scala 2 methods could
not be called from Dotty and vice-versa (see the added
sbt-dotty/sbt-test/scala2-compat/akka/i3100.scala test for an example)
- It can lead to invalid filenames on Windows (scala#7492)
- The handling of backticks is broken: adding or removing backticks
around a name changes how it's encoded.

To maintain Scala 2 compat we don't have a lot of choices here, we must
use the same scheme, so this commit aligns NameTransformer.scala with
https://github.com/scala/scala/blob/2.13.x/src/library/scala/reflect/NameTransformer.scala

Fixes scala#3100. Fixes scala#5936. Fixes scala#7492.
smarter added a commit to dotty-staging/dotty that referenced this issue Nov 25, 2019
There were several issues with the scheme we used so far:
- It's different from Scala 2, meaning that some Scala 2 methods could
not be called from Dotty and vice-versa (see the added
sbt-dotty/sbt-test/scala2-compat/akka/i3100.scala test for an example)
- It can lead to invalid filenames on Windows (scala#7492)
- The handling of backticks is broken: adding or removing backticks
around a name changes how it's encoded.

To maintain Scala 2 compat we don't have a lot of choices here, we must
use the same scheme, so this commit aligns NameTransformer.scala with
https://github.com/scala/scala/blob/2.13.x/src/library/scala/reflect/NameTransformer.scala

Some examples:

Method name | Old encoding | New encoding
-----------------------------------------
a_+         |      a_$plus |      a_$plus
`a_+`       |          a_+ |      a_$plus
`+_a`       |          +_a |      $plus_a
a_/         |       a_$div |       a_$div
`a_/`       |     a_$u002F |       a_$div

If a Dotty method is called `def a_$plus` we won't misinterpret it as
`a_+` because the method name comes from the tasty tree which stores
unencoded names. On the other hand, names coming from Java / Scala 2 as
well as top-level classnames might be misinterpreted as encoded names if
they contain a user-written $, this is left unspecified.

Fixes scala#3100. Fixes scala#5936. Fixes scala#7492.
@sjrd
Copy link
Member

sjrd commented Nov 25, 2019

I don't think this is fixed. I just got the following error on Windows:

[error] java.nio.file.InvalidPathException: Illegal char <:> at index 23: TypeOrBoundsOps$given_=:=_of_Type$.class
[error]         at sun.nio.fs.WindowsPathParser.normalize(WindowsPathParser.java:182)
[error]         at sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:153)
[error]         at sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:77)
[error]         at sun.nio.fs.WindowsPath.parse(WindowsPath.java:94)
[error]         at sun.nio.fs.WindowsFileSystem.getPath(WindowsFileSystem.java:255)
[error]         at sun.nio.fs.AbstractPath.resolve(AbstractPath.java:53)
[error]         at dotty.tools.io.Path.$div(Path.scala:93)
[error]         at dotty.tools.io.PlainFile.lookupName(PlainFile.scala:78)
[error]         at dotty.tools.io.AbstractFile.fileOrSubdirectoryNamed(AbstractFile.scala:239)
[error]         at dotty.tools.io.AbstractFile.fileNamed(AbstractFile.scala:256)
[error]         at dotty.tools.backend.jvm.BytecodeWriters.getFile(BytecodeWriters.scala:31)
[error]         at dotty.tools.backend.jvm.GenBCodePipeline.getFile(GenBCode.scala:72)
...

@smarter
Copy link
Member

smarter commented Nov 25, 2019

The reference compiler is still using the old encoding, so this won't be truly fixed until we re-bootstrap (at the latest in a couple of weeks with the new release)

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