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

Fix #2759: support params like @file.txt for dotc #2765

Merged
merged 3 commits into from
Jun 27, 2017

Conversation

liufengyun
Copy link
Contributor

@liufengyun liufengyun commented Jun 15, 2017

Fix #2759: support params like @file.txt for dotc

The command line parser is migrated from Scalac for handling possible quotes in params.

A test case for this feature is here, it works on Linux/Mac/Windows: lampepfl/packtest#4

Copy link
Contributor

@olafurpg olafurpg left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Thanks @liufengyun !

@liufengyun
Copy link
Contributor Author

Hmm, the legacy CI is not happy:

[info] Test dotc.tests.dottyBooted started
possible data race involving globally reachable variable root in class Pattern: java.util.regex.Pattern.Node
  use -Ylog:checkReentrant+ to find out more about why the variable is reachable.
possible data race involving globally reachable variable matchRoot in class Pattern: java.util.regex.Pattern.Node
  use -Ylog:checkReentrant+ to find out more about why the variable is reachable.
possible data race involving globally reachable variable buffer in class Pattern: Array[Int]
  use -Ylog:checkReentrant+ to find out more about why the variable is reachable.
possible data race involving globally reachable variable namedGroups in class Pattern: java.util.Map[String, Integer]
  use -Ylog:checkReentrant+ to find out more about why the variable is reachable.
possible data race involving globally reachable variable groupNodes in class Pattern: Array[java.util.regex.Pattern.GroupHead]
  use -Ylog:checkReentrant+ to find out more about why the variable is reachable.
possible data race involving globally reachable variable capturingGroupCount in class Pattern: Int
  use -Ylog:checkReentrant+ to find out more about why the variable is reachable.
possible data race involving globally reachable variable localCount in class Pattern: Int
  use -Ylog:checkReentrant+ to find out more about why the variable is reachable.
7 warnings found
7 errors found
[error] Test dotc.tests.dottyBooted failed: java.lang.AssertionError: assertion failed: Wrong # of errors. Expected: 0, found: 7

@smarter @felixmulder @DarkDimius could you please advise?

@smarter
Copy link
Member

smarter commented Jun 15, 2017

This tells me two things:

  • -Ycheck-reentrant is not passed to the compiler when compiling dotty in the new tests, that's an issue in the tests, that's bad! /cc @felixmulder
  • -Ycheck-reentrant detects that there is a globally reachable value (Word) whose class (Regex) contains a field of type Pattern which has mutable fields. In general this might causes problem in multi-threaded code, but Pattern is documented as thread-safe (https://docs.oracle.com/javase/8/docs/api/java/util/regex/Pattern.html) so this should be fine. You can disable the check by adding the annotation @sharable in front of the declaration of Word.

@felixmulder
Copy link
Contributor

It should indeed compile with check reentrant. Can it be enabled by this PR?

@smarter
Copy link
Member

smarter commented Jun 15, 2017

Opened #2769 to track this.

@@ -208,7 +208,8 @@ class CompilationTests extends ParallelTesting {
defaultOutputDir + "lib/src/:" +
// as well as bootstrapped compiler:
defaultOutputDir + "dotty1/dotty1/:" +
Jars.dottyInterfaces
Jars.dottyInterfaces,
"-Ycheck-reentrant"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@felixmulder could you please check if this is the right place to put the check?

@felixmulder felixmulder merged commit 04ac312 into scala:master Jun 27, 2017
@liufengyun liufengyun deleted the fix-2759 branch July 12, 2017 20:41
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.

4 participants