Multivalued setting can be set multiply #9731
Conversation
Remove comment referring to ancient Scala 2 bug. Adjust imports to read: imports relative to packaging, then absolute dotty packages, then scala packages.
Permit `-language:foo -language:bar` to mean `-language:foo,bar`, and similarly for other settings which are multivalued (i.e., have a value which is a list). Such settings include `-Xplugin` and `-Xprint`. There is no mechanism to distinguish phase names from other string settings.
| @@ -17,7 +17,7 @@ import Diagnostic._ | |||
| * - The reporter is not flushed and the message containers capture a | |||
| * `Context` (about 4MB) | |||
| */ | |||
| class StoreReporter(outer: Reporter) extends Reporter { | |||
| class StoreReporter(outer: Reporter = null) extends Reporter { | |||
There was a problem hiding this comment.
I'd suggest remove this default value, as it is not common for outer to be null. Moreover, this creates more subtleties for the usage of outer inside StoreReporter. There is Reporter.NoReporter, which can be used where null is needed.
| if Files.notExists(outputDir) | ||
| Files.createDirectory(outputDir) | ||
| val options = Array("-encoding", "-d", outputDir.toString, source.toString) | ||
| val reporter = Main.process(options, reporter = StoreReporter()) |
There was a problem hiding this comment.
| val reporter = Main.process(options, reporter = StoreReporter()) | |
| val reporter = Main.process(options, reporter = StoreReporter(Reporter.NoReporter)) |
| val options = Array("-d", "not_here") | ||
| val reporter = Main.process(options) | ||
| val reporter = Main.process(options, reporter = StoreReporter()) |
There was a problem hiding this comment.
| val reporter = Main.process(options, reporter = StoreReporter()) | |
| val reporter = Main.process(options, reporter = StoreReporter(Reporter.NoReporter)) |
| val value0 = value.asInstanceOf[List[String]] | ||
| val current = valueIn(sstate).asInstanceOf[List[String]] | ||
| val newly = current ++ value0.filterNot(current.contains) | ||
| (newly, value0.exists(current.contains)) |
There was a problem hiding this comment.
Might be simpler: newly.size <= current.size?
| (newly, value0.exists(current.contains)) | ||
| else | ||
| (value, changed) | ||
| val dangers = if twicely then warnings :+ s"Flag $name set repeatedly" else warnings |
There was a problem hiding this comment.
This warning is not informative for multivalued settings, as it's OK to set them multiple times.
There was a problem hiding this comment.
Redundant -language:foo -language:foo or -Wconf:cat=deprecation:s -Wconf:cat=deprecation:s is probably unintended, although possibly harmless. Or, -Wconf:cat=deprecation:s -Wconf:msg='foo':e -Wconf:cat=deprecation:s may be ambiguous; or it may be intentional. I'll remove the filter completely instead of guessing. -language:dynamics,-dynamics enables, -language:_,-dynamics disables.
There was a problem hiding this comment.
Sorry I wasn't clear. I don't mean to suppress the warning. I mean to improve the warning to say a specific value is set multiple times.
Warn but don't ignore redundant setting. Prefer NoReporter to null default reporter.
| val reporter = Main.process(options) | ||
| if Files.notExists(outputDir) | ||
| Files.createDirectory(outputDir) | ||
| val options = Array("-encoding", "-d", outputDir.toString, source.toString) |
There was a problem hiding this comment.
The argument is missing for -encoding. Or remove it.
There was a problem hiding this comment.
The ticket is about the missing arg.
Permit
-language:foo -language:barto mean-language:foo,bar,and similarly for other settings which are multivalued (i.e.,
have a value which is a list). Such settings include
-Xpluginand
-Xprint. There is no mechanism to distinguish phase namesfrom other string settings.
Learning again how to run a junit test is a big win.
Fixes #9719