Require pattern selections in -opt:inline

src/compiler/scala/tools/nsc/settings/MutableSettings.scala

@@ -555,7 +555,7 @@ class MutableSettings(val errorFn: String => Unit, val pathFactory: PathFactory)
    * not present in the multiChoiceSetting.value set, only their expansion.
    */
   abstract class MultiChoiceEnumeration extends Enumeration {
-    case class Choice(name: String, help: String = "", expandsTo: List[Choice] = Nil) extends Val(name) {
+    case class Choice(name: String, help: String = "", expandsTo: List[Choice] = Nil, requiresSelections: Boolean = false) extends Val(name) {
       var selections: List[String] = Nil
     }
     def wildcardChoices: ValueSet = values.filter { case c: Choice => c.expandsTo.isEmpty case _ => true }
@@ -613,6 +613,7 @@ class MutableSettings(val errorFn: String => Unit, val pathFactory: PathFactory)
 
     private def badChoice(s: String) = errorFn(s"'$s' is not a valid choice for '$name'")
     private def isChoice(s: String) = s == "_" || choices.contains(pos(s))
+    private def choiceOf(s: String): domain.Choice = domain.withName(pos(s)).asInstanceOf[domain.Choice]
 
     private def pos(s: String) = s.stripPrefix("-")
     private def isPos(s: String) = !s.startsWith("-")
@@ -711,6 +712,7 @@ class MutableSettings(val errorFn: String => Unit, val pathFactory: PathFactory)
             else {
               if (isChoice(argx)) {
                 if (selections.nonEmpty) add(argx, selections.split(",").toList)
+                else if (argx != "_" && isPos(argx) && choiceOf(argx).requiresSelections) errorFn(s"'$argx' requires '$argx:<selection>'. See '$name:help'.")
                 else add(argx)  // this case also adds "_"
                 postSetHook()   // support -opt:l:method
               }

src/compiler/scala/tools/nsc/settings/ScalaSettings.scala

@@ -307,13 +307,12 @@ trait ScalaSettings extends StandardScalaSettings with Warnings { _: MutableSett
     val allowSkipCoreModuleInit = Choice("allow-skip-core-module-init", "Allow eliminating unused module loads for core modules of the standard library (e.g., Predef, ClassTag).")
     val assumeModulesNonNull    = Choice("assume-modules-non-null",     "Assume loading a module never results in null (happens if the module is accessed in its super constructor).")
     val allowSkipClassLoading   = Choice("allow-skip-class-loading",    "Allow optimizations that can skip or delay class loading.")
-    val inline                  = Choice("inline",                      "Inline method invocations from specified sites; also see -Yopt-inline-heuristics.")
     val ell                     = Choice("l",                           "Deprecated l:none, l:default, l:method, l:inline.")
 
     // none is not an expanding option. It is excluded from -opt:_ below.
     val lNone = Choice("none", "Disable all optimizations, including explicit options.")
 
-    private val defaultOptimizations = List(unreachableCode)
+    val defaultOptimizations = List(unreachableCode)
     val lDefault = Choice(
       "default",
       defaultOptimizations.mkString("Enable default optimizations: ", ",", "."),
@@ -325,10 +324,12 @@ trait ScalaSettings extends StandardScalaSettings with Warnings { _: MutableSett
       localOptimizations.mkString("Enable intra-method optimizations: ", ",", "."),
       expandsTo = defaultOptimizations ::: localOptimizations)
 
-    val lInline = Choice(
-      "all",
-      "Enable inlining, cross-method and local optimizations. To restrict inlining, use -opt:inline:sites as follows:\n" + inlineHelp,
-      expandsTo = inline :: defaultOptimizations ::: localOptimizations)
+    val inlineFrom = Choice(
+      "inline",
+      s"Inline method invocations from specified sites; enables all optimizations; see -Yopt-inline-heuristics.\n$inlineHelp",
+      expandsTo = defaultOptimizations ::: localOptimizations,
+      requiresSelections = true
+    )
 
     // "none" is excluded from wildcard expansion so that -opt:_ does not disable all settings
     override def wildcardChoices = super.wildcardChoices.filter(_ ne lNone)
@@ -350,15 +351,15 @@ trait ScalaSettings extends StandardScalaSettings with Warnings { _: MutableSett
         case "none"    => "none"
         case "default" => "default"
         case "method"  => "local"
-        case "inline"  => "all"
+        case "inline"  => "local"   // enable all except inline, see -opt-inline-from
       }
       optChoices.ell.selections = Nil
       ss.tryToSetColon(todo)
     }
   }
 
   private def optEnabled(choice: optChoices.Choice) =
-    !opt.contains(optChoices.lNone) && {
+    !optNone && {
       opt.contains(choice) ||
       !opt.isSetByUser && optChoices.lDefault.expandsTo.contains(choice)
     }
@@ -375,17 +376,13 @@ trait ScalaSettings extends StandardScalaSettings with Warnings { _: MutableSett
   def optAllowSkipCoreModuleInit = optEnabled(optChoices.allowSkipCoreModuleInit)
   def optAssumeModulesNonNull    = optEnabled(optChoices.assumeModulesNonNull)
   def optAllowSkipClassLoading   = optEnabled(optChoices.allowSkipClassLoading)
-  def optInlinerEnabled          = optEnabled(optChoices.inline)
+  def optInlinerEnabled          = !optInlineFrom.isEmpty && !optNone
 
   def optBuildCallGraph          = optInlinerEnabled || optClosureInvocations
   def optAddToBytecodeRepository = optBuildCallGraph || optInlinerEnabled || optClosureInvocations
   def optUseAnalyzerCache        = opt.isSetByUser && !optNone && (optBuildCallGraph || opt.value.size > 1)
 
-  def optInlineFrom: List[String] =
-    optChoices.inline.selections match {
-      case Nil  => List("**")
-      case sels => sels
-    }
+  def optInlineFrom: List[String] = optChoices.inlineFrom.selections
 
   def inlineHelp =
       """Patterns for classfile names from which the inliner is allowed to pull in code.
@@ -401,12 +398,12 @@ trait ScalaSettings extends StandardScalaSettings with Warnings { _: MutableSett
         |  <sources>      Classes defined in source files compiled in the current compilation, either
         |                 passed explicitly to the compiler or picked up from the `-sourcepath`
         |
-        |The setting accepts a list of patterns: `-opt:inline:p1,p2`. The setting can be passed
-        |multiple times, the list of patterns gets extended. A leading `!` marks a pattern excluding.
+        |The setting requires a list of patterns: `-opt:inline:p1,p2`. The setting can be passed
+        |multiple times, the list of patterns gets extended. A leading `!` marks a pattern as excluding.
         |The last matching pattern defines whether a classfile is included or excluded (default: excluded).
         |For example, `a.**,!a.b.**` includes classes in a and sub-packages, but not in a.b and sub-packages.
         |
-        |Note: on the command-line you might need to quote patterns containing `*` to prevent the shell
+        |Note: on the command line you might need to quote patterns containing `*` to prevent the shell
         |from expanding it to a list of files in the current directory.""".stripMargin
 
   @deprecated("Deprecated alias", since="2.13.8")
@@ -416,10 +413,7 @@ trait ScalaSettings extends StandardScalaSettings with Warnings { _: MutableSett
     "Patterns for classfile names from which to allow inlining, `help` for details.",
     helpText = Some(inlineHelp))
       //.withDeprecationMessage("use -opt:inline:**")
-      .withPostSetHook { from =>
-        opt.add("inline")
-        optChoices.inline.selections = from.value
-      }
+      .withPostSetHook(from => opt.add("inline", from.value))
 
   val YoptInlineHeuristics = ChoiceSetting(
     name = "-Yopt-inline-heuristics",
@@ -557,10 +551,7 @@ trait ScalaSettings extends StandardScalaSettings with Warnings { _: MutableSett
   val optimise      = BooleanSetting("-optimize", "Enables optimizations.")
     .withAbbreviation("-optimise")
     .withDeprecationMessage("Since 2.12, enables -opt:inline:**. This can be dangerous.")
-    .withPostSetHook { _ =>
-      opt.enable(optChoices.lInline)
-      optChoices.inline.selections = List("**")
-    }
+    .withPostSetHook(_ => opt.add("inline", List("**")))
   val Xexperimental = BooleanSetting("-Xexperimental", "Former graveyard for language-forking extensions.")
     .withDeprecationMessage("Not used since 2.13.")
 

test/files/neg/deprecated.scala

@@ -1,4 +1,4 @@
-// scalac: -Xlint:deprecation -Xfatal-warnings -opt:inline -opt-inline-from:<sources>
+// scalac: -Xlint:deprecation -Werror -opt:inline:<sources>
 //
 
 trait T {

test/files/neg/inlineIndyLambdaPrivate/Test_2.scala

@@ -1,4 +1,4 @@
-// scalac: -opt:all -opt:inline:** -Yopt-inline-heuristics:everything -Wopt:_ -Werror
+// scalac: -opt:inline:** -Yopt-inline-heuristics:everything -Wopt:_ -Werror
 class Test {
   def foo = A_1.test
 }

test/files/neg/t11746.scala

@@ -1,5 +1,5 @@
 //
-// scalac: -Werror -opt:all -opt:inline:** -Wopt:none,_
+// scalac: -Werror -opt:inline:** -Wopt:none,_
 //
 // compare -opt-warnings:none,at-inline-failed-summary
 

test/files/pos/t11663/B_2.scala

@@ -1,4 +1,4 @@
-// scalac: -opt:inline:** -Wopt
+// scalac: -opt:inline:** -Wopt:_
 class B {
    def bar(c: A) = c.m
 }

test/files/run/indyLambdaKinds/Test_2.scala

@@ -3,7 +3,7 @@ import reflect.internal.util._
 
 object Test extends DirectTest {
 
-  override def extraSettings: String = s"-usejavacp -cp ${testOutput.path} -opt:all -Vinline _"
+  override def extraSettings: String = s"-usejavacp -cp ${testOutput.path} -opt:inline:** -Vinline _"
 
   override def code = """object Main {
   @noinline def t1a(a: A_1) = a.a(): @inline

test/files/run/noInlineUnknownIndy/Test.scala

@@ -11,7 +11,7 @@ object Test extends DirectTest {
 
   override def extraSettings = {
     val classpath = List(sys.props("partest.lib"), testOutput.path).mkString(sys.props("path.separator"))
-    s"-cp $classpath -opt:all -opt:inline:** -Yopt-inline-heuristics:everything -Wopt:_"
+    s"-cp $classpath -opt:inline:** -Yopt-inline-heuristics:everything -Wopt:_"
   }
 
   override def show(): Unit = {

test/junit/scala/tools/nsc/backend/jvm/InnerClassAttributeTest.scala

@@ -14,7 +14,7 @@ import scala.tools.testkit.BytecodeTesting._
 class InnerClassAttributeTest extends BytecodeTesting {
   import compiler._
 
-  val optCompiler = cached("optCompiler", () => newCompiler(extraArgs = "-opt:all -opt:inline:**"))
+  val optCompiler = cached("optCompiler", () => newCompiler(extraArgs = "-opt:inline:**"))
 
   @Test
   def javaInnerClassInGenericSignatureOnly(): Unit = {

test/junit/scala/tools/nsc/backend/jvm/OptimizedBytecodeTest.scala

@@ -11,7 +11,7 @@ import scala.tools.testkit.BytecodeTesting._
 
 @RunWith(classOf[JUnit4])
 class OptimizedBytecodeTest extends BytecodeTesting {
-  override def compilerArgs = "-opt:all -Wopt"
+  override def compilerArgs = "-opt:inline:** -Wopt"
   import compiler._
 
   @Test

test/junit/scala/tools/nsc/backend/jvm/opt/BTypesFromClassfileTest.scala

@@ -15,7 +15,7 @@ import scala.tools.testkit.BytecodeTesting
 @RunWith(classOf[JUnit4])
 class BTypesFromClassfileTest extends BytecodeTesting {
   // inliner enabled -> inlineInfos are collected (and compared) in ClassBTypes
-  override def compilerArgs = "-opt:inline -opt-inline-from:**"
+  override def compilerArgs = "-opt:inline:**"
 
   import compiler.global._
   import definitions._

test/junit/scala/tools/nsc/backend/jvm/opt/BoxUnboxAndInlineTest.scala

@@ -17,7 +17,7 @@ import scala.tools.testkit.BytecodeTesting._
   */
 @RunWith(classOf[JUnit4])
 class BoxUnboxAndInlineTest extends BytecodeTesting {
-  override def compilerArgs = "-opt:all -opt:inline:**/*"
+  override def compilerArgs = "-opt:inline:**/*"
   import compiler._
 
   // Was crashing in 2.13.x once https://github.com/scala/scala/pull/9433 was merged in.

test/junit/scala/tools/nsc/backend/jvm/opt/CallGraphTest.scala

@@ -16,7 +16,7 @@ import scala.tools.testkit.BytecodeTesting
 import scala.tools.testkit.BytecodeTesting._
 
 class CallGraphTest extends BytecodeTesting {
-  override def compilerArgs = "-opt:inline -opt-inline-from:** -opt-warnings"
+  override def compilerArgs = "-opt:inline:** -Wopt"
   import compiler._
   import global.genBCode.{bTypes, postProcessor}
   import postProcessor.{byteCodeRepository, callGraph}

test/junit/scala/tools/nsc/backend/jvm/opt/ClosureOptimizerTest.scala

@@ -13,7 +13,7 @@ import scala.tools.testkit.BytecodeTesting._
 
 @RunWith(classOf[JUnit4])
 class ClosureOptimizerTest extends BytecodeTesting {
-  override def compilerArgs = "-opt:all -opt:inline:** -Wopt"
+  override def compilerArgs = "-opt:inline:** -Wopt:_"
   import compiler._
 
   @Test

test/junit/scala/tools/nsc/backend/jvm/opt/InlineSourceMatcherTest.scala

@@ -15,10 +15,18 @@ import scala.tools.testkit.BytecodeTesting._
 class InlineSourceMatcherTest extends BytecodeTesting {
   import compiler._
 
-  override def compilerArgs = "-opt:inline -Wopt"
-  def setInlineFrom(s: String): Unit = {
-    //global.settings.optInlineFrom.value = s.split(':').toList
-    global.settings.optChoices.inline.selections = s.split(':').toList
+  override def compilerArgs = "-Wopt"
+
+  // takes non-standard colon-separated patterns
+  def withInlineFrom(s: String)(body: => Unit): Unit = {
+    val saved = global.settings.optChoices.inlineFrom.selections
+    global.settings.optChoices.inlineFrom.selections = s.split(':').toList
+    try body finally global.settings.optChoices.inlineFrom.selections = saved
+  }
+  def withNoInline(body: => Unit): Unit = {
+    val saved = global.settings.optChoices.inlineFrom.selections
+    global.settings.optChoices.inlineFrom.selections = Nil
+    try body finally global.settings.optChoices.inlineFrom.selections = saved
   }
 
   case class E(regex: String, negated: Boolean = false, terminal: Boolean = true)
@@ -186,10 +194,10 @@ class InlineSourceMatcherTest extends BytecodeTesting {
     def n(): Unit = assertInvoke(getMethod(compileClass(code), "t"), "C", "f")
     def y(): Unit = assertNoInvoke(getMethod(compileClass(code), "t"))
 
-    setInlineFrom(""); n()
-    setInlineFrom("C"); y()
-    setInlineFrom("**:!**.C"); n()
-    setInlineFrom("**:!**.C:C"); y()
+    withNoInline(n())
+    withInlineFrom("C")(y())
+    withInlineFrom("**:!**.C")(n())
+    withInlineFrom("**:!**.C:C")(y())
   }
 
   @Test
@@ -205,26 +213,22 @@ class InlineSourceMatcherTest extends BytecodeTesting {
         |}}
       """.stripMargin
 
-    {
-      setInlineFrom("")
+    withNoInline {
       val List(_, _, e) = compileClasses(code)
       assertInvoke(getMethod(e, "t1"), "a/C", "f")
       assertInvoke(getMethod(e, "t2"), "a/C$D$", "f")
     }
-    {
-      setInlineFrom("a.C")
+    withInlineFrom("a.C") {
       val List(_, _, e) = compileClasses(code)
       assertNoInvoke(getMethod(e, "t1"))
       assertInvoke(getMethod(e, "t2"), "a/C$D$", "f")
     }
-    {
-      setInlineFrom("a.C*")
+    withInlineFrom("a.C*") {
       val List(_, _, e) = compileClasses(code)
       assertNoInvoke(getMethod(e, "t1"))
       assertDoesNotInvoke(getMethod(e, "t2"), "f") // t2 still has an invocation to the getter `D`
     }
-    {
-      setInlineFrom("a.C*:!a.C*$")
+    withInlineFrom("a.C*:!a.C*$") {
       val List(_, _, e) = compileClasses(code)
       assertNoInvoke(getMethod(e, "t1"))
       assertInvoke(getMethod(e, "t2"), "a/C$D$", "f")
@@ -235,13 +239,11 @@ class InlineSourceMatcherTest extends BytecodeTesting {
   def inlineFromSources(): Unit = {
     val a = "class A { @inline final def f = 1 }"
     val b = "class B { def t(a: A) = a.f }"
-    setInlineFrom("<sources>")
-
-    {
+    withInlineFrom("<sources>") {
       val List(_, cb) = compileClasses(s"$a\n$b")
       assertNoInvoke(getMethod(cb, "t"))
     }
-    {
+    withInlineFrom("<sources>") {
       val List(_, cb) = compileClassesSeparately(List(a, b))
       assertInvoke(getMethod(cb, "t"), "A", "f")
     }

test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala

@@ -18,13 +18,13 @@ import scala.tools.testkit.BytecodeTesting._
 
 @RunWith(classOf[JUnit4])
 class InlinerTest extends BytecodeTesting {
-  override def compilerArgs = "-opt:all -opt:inline:** -Wopt"
-
-  val inlineOnlyCompiler = cached("inlineOnlyCompiler", () => newCompiler(extraArgs = "-opt:inline -opt-inline-from:**"))
-
   import compiler._
   import global.genBCode.{bTypes, postProcessor}
 
+  override def compilerArgs = "-opt:inline:** -Wopt"
+
+  val inlineOnly = global.settings.optChoices.inlineFrom.expandsTo.map(c => s"-${c.name}").mkString("-opt:inline:** -opt:", ",", "")
+  val inlineOnlyCompiler = cached("inlineOnlyCompiler", () => newCompiler(extraArgs = inlineOnly))
 
   compiler.keepPerRunCachesAfterRun(List(
     JavaClearable.forMap(bTypes.classBTypeCache),

test/junit/scala/tools/nsc/settings/SettingsTest.scala

@@ -252,13 +252,20 @@ class SettingsTest {
   @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)
+    assertTrue("has the choice", settings.opt.contains(settings.optChoices.unreachableCode))
+    assertTrue("is enabled", settings.optUnreachableCode)
+    assertFalse("inliner is not enabled", settings.optInlinerEnabled)
   }
   @Test def `kill switch can be enabled explicitly`(): Unit = {
     val settings = new Settings()
-    settings.processArguments("-opt:inline,none" :: Nil, true)
-    assertTrue("has the choice", settings.opt.contains(settings.optChoices.inline))
+    settings.processArguments("-opt:unreachable-code,none" :: Nil, true)
+    assertTrue("has the choice", settings.opt.contains(settings.optChoices.unreachableCode))
+    assertFalse("is not enabled", settings.optUnreachableCode)
+  }
+  @Test def `kill switch disables inline`(): Unit = {
+    val settings = new Settings()
+    settings.processArguments("-opt:inline:**" :: "-opt:none" :: Nil, true)
+    assertTrue("has the choice", settings.optInlineFrom.nonEmpty)
     assertFalse("is not enabled", settings.optInlinerEnabled)
   }
   @Test def `t12036 don't consume dash option as arg`(): Unit = {

test/junit/scala/tools/nsc/transform/patmat/PatmatBytecodeTest.scala

@@ -15,7 +15,7 @@ import PartialFunction.cond
 
 @RunWith(classOf[JUnit4])
 class PatmatBytecodeTest extends BytecodeTesting {
-  val optCompiler = cached("optCompiler", () => newCompiler(extraArgs = "-opt:all -opt:inline:**"))
+  val optCompiler = cached("optCompiler", () => newCompiler(extraArgs = "-opt:inline:**"))
 
   import compiler._