Skip to content

Commit

Permalink
Mechanism to exclude l:none from -opt:_ (#8454)
Browse files Browse the repository at this point in the history
Mechanism to exclude l:none from -opt:_
  • Loading branch information
lrytz authored Oct 9, 2019
2 parents 2d4381a + 2c5df00 commit 5892178
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 5 deletions.
7 changes: 4 additions & 3 deletions src/compiler/scala/tools/nsc/settings/MutableSettings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,7 @@ class MutableSettings(val errorFn: String => Unit)
*/
abstract class MultiChoiceEnumeration extends Enumeration {
case class Choice(name: String, help: String = "", expandsTo: List[Choice] = Nil) extends Val(name)
def wildcardChoices: ValueSet = values.filter { case c: Choice => c.expandsTo.isEmpty case _ => true }
}

/**
Expand Down Expand Up @@ -661,8 +662,8 @@ class MutableSettings(val errorFn: String => Unit)
/** (Re)compute from current yeas, nays, wildcard status. */
def compute() = {
def simple(v: domain.Value) = v match {
case ChoiceOrVal(_, _, Nil) => true
case _ => false
case c: domain.Choice => c.expandsTo.isEmpty
case _ => true
}

/**
Expand All @@ -677,7 +678,7 @@ class MutableSettings(val errorFn: String => Unit)
}

// yeas from _ or expansions are weak: an explicit nay will disable them
val weakYeas = if (sawAll) domain.values filter simple else expand(yeas filterNot simple)
val weakYeas = if (sawAll) domain.wildcardChoices else expand(yeas filterNot simple)
value = (yeas filter simple) | (weakYeas &~ nays)
}

Expand Down
8 changes: 6 additions & 2 deletions src/compiler/scala/tools/nsc/settings/ScalaSettings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ trait ScalaSettings extends StandardScalaSettings with Warnings {
val allowSkipClassLoading = Choice("allow-skip-class-loading", "Allow optimizations that can skip or delay class loading.")
val inline = Choice("inline", "Inline method invocations according to -Yopt-inline-heuristics and -opt-inline-from.")

// note: unlike the other optimizer levels, "l:none" appears up in the `opt.value` set because it's not an expanding option (expandsTo is empty)
// l:none is not an expanding option, unlike the other l: levels. But it is excluded from -opt:_ below.
val lNone = Choice("l:none",
"Disable optimizations. Takes precedence: `-opt:l:none,+box-unbox` / `-opt:l:none -opt:box-unbox` don't enable box-unbox.")

Expand All @@ -311,6 +311,9 @@ trait ScalaSettings extends StandardScalaSettings with Warnings {
val lInline = Choice("l:inline",
"Enable cross-method optimizations (note: inlining requires -opt-inline-from): " + inlineChoices.mkString("", ",", "."),
expandsTo = inlineChoices)

// "l:none" is excluded from wildcard expansion so that -opt:_ does not disable all settings
override def wildcardChoices = super.wildcardChoices.filter(_ ne lNone)
}

// We don't use the `default` parameter of `MultiChoiceSetting`: it specifies the default values
Expand All @@ -320,7 +323,8 @@ trait ScalaSettings extends StandardScalaSettings with Warnings {
name = "-opt",
helpArg = "optimization",
descr = "Enable optimizations",
domain = optChoices)
domain = optChoices,
)

private def optEnabled(choice: optChoices.Choice) = {
!opt.contains(optChoices.lNone) && {
Expand Down
7 changes: 7 additions & 0 deletions test/files/neg/t11746.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
t11746.scala:18: warning: failed to determine if e should be inlined:
The method e()Ljava/lang/Throwable; could not be found in the class java/lang/Object or any of its parents.
case Failure(e) => println(e.toString)
^
error: No warnings can be incurred under -Werror.
one warning found
one error found
20 changes: 20 additions & 0 deletions test/files/neg/t11746.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
//
// scalac: -Werror -opt:l:inline -opt-inline-from:'**' -opt-warnings:none,_
//
// compare -opt-warnings:none,at-inline-failed-summary

trait Try

object Try {
def apply(s: String): Try = Success(s)
}

case class Success(s: String) extends Try
case class Failure(e: Throwable) extends Try

class C {
private def get(a: String): Unit = Try(a) match {
case Failure(e: Exception) =>
case Failure(e) => println(e.toString)
}
}
23 changes: 23 additions & 0 deletions test/files/run/t11746.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@

import scala.tools.partest.DirectTest
import scala.tools.nsc.reporters.{Reporter, StoreReporter}
import scala.tools.nsc.Settings
import scala.tools.nsc.util.stringFromStream

object Test extends DirectTest {

override def extraSettings = "-usejavacp -opt:_ -opt-inline-from:** -Vinline _"

override def reporter(settings: Settings): Reporter = new StoreReporter(settings)

override def code = "class C { def f = locally(42) }"

override def show() = {
val res = stringFromStream { os =>
Console.withOut(os) {
assert(compile())
}
}
assert(res.startsWith("Inlining into C.f"))
}
}
12 changes: 12 additions & 0 deletions test/junit/scala/tools/nsc/settings/SettingsTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -224,4 +224,16 @@ class SettingsTest {
marginallyEquals(expected, m.help)
})
}
@Test def `wildcard doesn't disable everything`(): Unit = {
val settings = new Settings()
settings.processArguments("-opt:_" :: Nil, true)
assertTrue("has the choice", settings.opt.contains(settings.optChoices.inline))
assertTrue("is enabled", settings.optInlinerEnabled)
}
@Test def `kill switch can be enabled explicitly`(): Unit = {
val settings = new Settings()
settings.processArguments("-opt:inline,l:none" :: Nil, true)
assertTrue("has the choice", settings.opt.contains(settings.optChoices.inline))
assertFalse("is not enabled", settings.optInlinerEnabled)
}
}

0 comments on commit 5892178

Please sign in to comment.