Tweak java getlitch not to skip zero #18491

som-snytt · 2023-08-31T09:27:21Z

The "skip zero" behavior was to accommodate escaped EOL in a text block, but also skips zero-valued escaped char \0.

This fix is less precious and uses a flag called skip.

Scala 2 previously received the complementary fix, because it wasn't preserving the char before the escaped EOL (after backporting the dotty code):
scala/scala#10024

Vulpix doesn't work for me locally, as I just get InterruptedException, like it shuts down before my test finishes.

This seems to not crash in manual test.

The test passes under sbt test, so that's nice.

Obligatory pun that getlitch is pronounced glitch.

Fixes #18490

som-snytt · 2023-09-01T00:05:43Z

I thought I was so clever to write %2c but it doesn't work on scalajs ffffffc3ffffffbf.

sjrd · 2023-09-01T01:50:54Z

I thought I was so clever to write %2c but it doesn't work on scalajs ffffffc3ffffffbf.

Huh. Are you sure it was the %2c that did that? If it was the f"$b%02x" instead, it's a known, accepted discrepancy: x and o on Byte and Short sign-extend. Use f"${b & 0xff}%02x" to make it portable.

som-snytt · 2023-09-01T08:52:53Z

@sjrd thanks, I was so quick with the self-deprecating humor that I wasn't paying attention.

Also I have to relearn printf. Is that my "learn a programming language during lunch" for today?

I remembered sign extension but was hoping to clip it.

That would be f"${x.toHexString}%2.2s for field width. How disappointing. And on dotty, byte.toInt.toHexString.

Come to think of it, IIRC you told me to mask it already some years ago, unless that is deja-vu.

I just got brave and consulted the doc, which I'd even forgotten is at java.util.Formatter:

For character, integral, and date/time argument types and the percent and line separator conversions, the precision is not applicable

The underlying reason for my haste was that I can't run a single test without interrupted exception (worse, the stack trace says it's reading the using pragma from the test source), so after a 20 min test run to update check file, I was out of gas and unsure whether to look at the test rig problem. This is on WSL.

som-snytt · 2023-09-01T08:54:08Z

tests/run/t12290/Test.scala

+val printable = raw"\p{Print}".r
+
+def hexdump(s: String) = s.getBytes(io.Codec.UTF8.charSet) // java.nio.charset.StandardCharsets.UTF_8
+                          .map(b => b.toInt.toHexString.takeRight(2)).mkString


creative...

sjrd · 2023-09-01T09:43:10Z

The underlying reason for my haste was that I can't run a single test without interrupted exception (worse, the stack trace says it's reading the using pragma from the test source), so after a 20 min test run to update check file, I was out of gas and unsure whether to look at the test rig problem. This is on WSL.

That sounds awful. :s I'm not sure we have anyone building dotty on WSL on a regular basis. However, I do build it on standard Windows, without WSL, and it works fine. Have you tried building outside of WSL, in the standard Windows console?

jchyb

Looks really good. Thank you! I only left some NIT comments for some leftover code

jchyb · 2023-09-08T08:35:25Z

tests/run/t12290/Test.scala

 }
+
+val printable = raw"\p{Print}".r


I think this can be removed

jchyb · 2023-09-08T08:40:48Z

tests/run/t12290/Test.scala

+                          /*
+                          .map(b => b.toChar match {
+                            case c @ printable() => f"$c%2c"
+                            case _ => f"$b%02x"
+                          }).mkString
+                          */


Same here, I think hexdump method is pretty self explanatory already

som-snytt · 2023-09-08T14:23:25Z

Until I return to it, the stacktrace I see on clean repo.

sbt:scala3> testCompilation t12290
[info] compiling 94 Scala sources and 25 Java sources...

[info] Test dotty.tools.dotc.CompilationTests.runAll started
[                                        ] completed (0/1, 0 failed, 2s)java.nio.channels.ClosedByInterruptException
        at java.base/java.nio.channels.spi.AbstractInterruptibleChannel.end(AbstractInterruptibleChannel.java:199)
        at java.base/sun.nio.ch.FileChannelImpl.endBlocking(FileChannelImpl.java:171)
        at java.base/sun.nio.ch.FileChannelImpl.size(FileChannelImpl.java:430)
        at java.base/java.nio.file.Files.createFileChannelLinesStream(Files.java:4123)
        at java.base/java.nio.file.Files.lines(Files.java:4109)
        at dotty.tools.utils$package$.toolArgsFor$$anonfun$1(utils.scala:68)
        at scala.collection.LinearSeqOps.foldLeft(LinearSeq.scala:183)
        at scala.collection.LinearSeqOps.foldLeft$(LinearSeq.scala:179)
        at scala.collection.immutable.List.foldLeft(List.scala:79)
        at dotty.tools.utils$package$.toolArgsFor(utils.scala:74)
        at dotty.tools.vulpix.ParallelTesting$TestSource.allToolArgs(ParallelTesting.scala:103)
        at dotty.tools.vulpix.ParallelTesting$TestSource.allToolArgs$(ParallelTesting.scala:66)
        at dotty.tools.vulpix.ParallelTesting$SeparateCompilationSource.allToolArgs$lzyINIT2(ParallelTesting.scala:180)
        at dotty.tools.vulpix.ParallelTesting$SeparateCompilationSource.allToolArgs(ParallelTesting.scala:180)
        at dotty.tools.vulpix.ParallelTesting$RunTest.verifyOutput(ParallelTesting.scala:838)
        at dotty.tools.vulpix.ParallelTesting$RunTest.onSuccess(ParallelTesting.scala:857)
        at dotty.tools.vulpix.ParallelTesting$CompilationLogic.dotty$tools$vulpix$ParallelTesting$CompilationLogic$$onComplete(ParallelTesting.scala:298)
        at dotty.tools.vulpix.ParallelTesting$$anon$4.checkTestSource$$anonfun$1(ParallelTesting.scala:286)
        at dotty.tools.vulpix.ParallelTesting$$anon$4.checkTestSource$$anonfun$adapted$1(ParallelTesting.scala:288)
        at scala.Function0.apply$mcV$sp(Function0.scala:42)
        at dotty.tools.vulpix.ParallelTesting$Test.tryCompile(ParallelTesting.scala:462)
        at dotty.tools.vulpix.ParallelTesting$$anon$4.checkTestSource(ParallelTesting.scala:288)
        at dotty.tools.vulpix.ParallelTesting$Test$LoggedRunnable.run(ParallelTesting.scala:358)
        at dotty.tools.vulpix.ParallelTesting$Test$LoggedRunnable.run$(ParallelTesting.scala:340)
        at dotty.tools.vulpix.ParallelTesting$$anon$4.run(ParallelTesting.scala:283)

som-snytt · 2023-09-08T19:43:01Z

I updated the test as it was supposed to have been written.

I see scala.tools.testkit.AssertUtil.hexdump is used to show failed string comparison.

jchyb

Thank you once again!

Backports #18491 to the LTS branch. PR submitted by the release tooling. [skip ci]

som-snytt force-pushed the issue/18490-java-parser-charlit branch from 03d55b0 to d7ddd3e Compare August 31, 2023 18:52

som-snytt changed the title ~~Provisional tweak~~ Tweak java getlitch not to skip zero Aug 31, 2023

som-snytt force-pushed the issue/18490-java-parser-charlit branch from d7ddd3e to e340c62 Compare September 1, 2023 01:00

som-snytt commented Sep 1, 2023

View reviewed changes

som-snytt marked this pull request as ready for review September 1, 2023 08:55

jchyb self-assigned this Sep 4, 2023

jchyb requested changes Sep 8, 2023

View reviewed changes

tests/run/t12290/Test.scala Outdated

}

val printable = raw"\p{Print}".r

Copy link

Contributor

jchyb Sep 8, 2023

Choose a reason for hiding this comment

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

I think this can be removed

jchyb reviewed Sep 8, 2023

View reviewed changes

Tweak java getlitch not to skip zero

5f29c21

som-snytt force-pushed the issue/18490-java-parser-charlit branch from e340c62 to 5f29c21 Compare September 8, 2023 14:27

jchyb approved these changes Sep 11, 2023

View reviewed changes

jchyb merged commit 1be790c into scala:main Sep 11, 2023
17 checks passed

som-snytt deleted the issue/18490-java-parser-charlit branch September 11, 2023 12:35

Kordyjan added this to the 3.4.0 milestone Dec 20, 2023

WojciechMazur mentioned this pull request Jun 19, 2024

Backport "Tweak java getlitch not to skip zero" to LTS #20613

Merged

WojciechMazur added a commit that referenced this pull request Jun 20, 2024

Backport "Tweak java getlitch not to skip zero" to LTS (#20613)

90928a4

Backports #18491 to the LTS branch. PR submitted by the release tooling. [skip ci]

WojciechMazur mentioned this pull request Jun 20, 2024

Backport "Tweak java getlitch not to skip zero" to LTS #20672

Merged

WojciechMazur added a commit that referenced this pull request Jun 20, 2024

Backport "Tweak java getlitch not to skip zero" to LTS (#20672)

45d832c

Backports #18491 to the LTS branch. PR submitted by the release tooling. [skip ci]

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tweak java getlitch not to skip zero #18491

Tweak java getlitch not to skip zero #18491

som-snytt commented Aug 31, 2023 •

edited

Loading

som-snytt commented Sep 1, 2023

sjrd commented Sep 1, 2023

som-snytt commented Sep 1, 2023

som-snytt Sep 1, 2023

sjrd commented Sep 1, 2023

jchyb left a comment

jchyb Sep 8, 2023

jchyb Sep 8, 2023

som-snytt commented Sep 8, 2023

som-snytt commented Sep 8, 2023

jchyb left a comment

Tweak java getlitch not to skip zero #18491

Tweak java getlitch not to skip zero #18491

Conversation

som-snytt commented Aug 31, 2023 • edited Loading

som-snytt commented Sep 1, 2023

sjrd commented Sep 1, 2023

som-snytt commented Sep 1, 2023

som-snytt Sep 1, 2023

Choose a reason for hiding this comment

sjrd commented Sep 1, 2023

jchyb left a comment

Choose a reason for hiding this comment

jchyb Sep 8, 2023

Choose a reason for hiding this comment

jchyb Sep 8, 2023

Choose a reason for hiding this comment

som-snytt commented Sep 8, 2023

som-snytt commented Sep 8, 2023

jchyb left a comment

Choose a reason for hiding this comment

som-snytt commented Aug 31, 2023 •

edited

Loading