Skip to content

Commit

Permalink
processArgs is tailrec
Browse files Browse the repository at this point in the history 
Long lists of compiler args are not unusual,
so avoid running out of stack.
  • Loading branch information
@som-snytt
som-snytt committed May 25, 2021
1 parent e2e77b5 commit f1df8f3
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 49 deletions.
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/config/CliCommand.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ trait CliCommand:
}

sg.processArguments(expandedArguments, processAll = true, settingsState = ss)
end distill

/** Creates a help message for a subset of options based on cond */
protected def availableOptionsMsg(cond: Setting[?] => Boolean)(using settings: ConcreteSettings)(using SettingsState): String =
Expand Down Expand Up @@ -108,6 +109,7 @@ trait CliCommand:
""
s"${formatName(s.name)} ${formatDescription(s.description)}${formatSetting("Default", defaultValue)}${formatSetting("Choices", s.legalChoices)}"
ss.map(helpStr).mkString("", "\n", s"\n${formatName("@<file>")} ${formatDescription("A text file containing compiler arguments (options and source files).")}\n")
end availableOptionsMsg

protected def shortUsage: String = s"Usage: $cmdName <options> <source files>"

Expand Down
50 changes: 20 additions & 30 deletions compiler/src/dotty/tools/dotc/config/Settings.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,25 +21,22 @@ object Settings {
val OptionTag: ClassTag[Option[?]] = ClassTag(classOf[Option[?]])
val OutputTag: ClassTag[AbstractFile] = ClassTag(classOf[AbstractFile])

class SettingsState(initialValues: Seq[Any]) {
class SettingsState(initialValues: Seq[Any]):
private val values = ArrayBuffer(initialValues: _*)
private var _wasRead: Boolean = false

override def toString: String = s"SettingsState(values: ${values.toList})"

def value(idx: Int): Any = {
def value(idx: Int): Any =
_wasRead = true
values(idx)
}

def update(idx: Int, x: Any): SettingsState =
if (_wasRead)
new SettingsState(values.toSeq).update(idx, x)
else {
if (_wasRead) then SettingsState(values.toSeq).update(idx, x)
else
values(idx) = x
this
}
}
end SettingsState

case class ArgsSummary(
sstate: SettingsState,
Expand Down Expand Up @@ -67,18 +64,11 @@ object Settings {

private var changed: Boolean = false

def valueIn(state: SettingsState): T =
state.value(idx).asInstanceOf[T]
def valueIn(state: SettingsState): T = state.value(idx).asInstanceOf[T]

def updateIn(state: SettingsState, x: Any): SettingsState = x match {
def updateIn(state: SettingsState, x: Any): SettingsState = x match
case _: T => state.update(idx, x)
case _ =>
// would like to do:
// throw new ClassCastException(s"illegal argument, found: $x of type ${x.getClass}, required: ${implicitly[ClassTag[T]]}")
// but this runs afoul of primitive types. Concretely: if T is Boolean, then x is a boxed Boolean and the test will fail.
// Maybe this is a bug in Scala 2.10?
state.update(idx, x.asInstanceOf[T])
}
case _ => throw IllegalArgumentException(s"found: $x of type ${x.getClass.getName}, required: ${implicitly[ClassTag[T]]}")

def isDefaultIn(state: SettingsState): Boolean = valueIn(state) == default

Expand All @@ -94,7 +84,7 @@ object Settings {

def tryToSet(state: ArgsSummary): ArgsSummary = {
val ArgsSummary(sstate, arg :: args, errors, warnings) = state
def update(value: Any, args: List[String]) =
def update(value: Any, args: List[String]): ArgsSummary =
var dangers = warnings
val value1 =
if changed && isMultivalue then
Expand All @@ -107,6 +97,7 @@ object Settings {
value
changed = true
ArgsSummary(updateIn(sstate, value1), args, errors, dangers)
end update
def fail(msg: String, args: List[String]) =
ArgsSummary(sstate, args, errors :+ msg, warnings)
def missingArg =
Expand Down Expand Up @@ -209,7 +200,7 @@ object Settings {
/** Iterates over the arguments applying them to settings where applicable.
* Then verifies setting dependencies are met.
*
* This temporarily takes a boolean indicating whether to keep
* This takes a boolean indicating whether to keep
* processing if an argument is seen which is not a command line option.
* This is an expedience for the moment so that you can say
*
Expand All @@ -221,28 +212,27 @@ object Settings {
*
* to get their arguments.
*/
def processArguments(state: ArgsSummary, processAll: Boolean, skipped: List[String]): ArgsSummary = {
@tailrec
final def processArguments(state: ArgsSummary, processAll: Boolean, skipped: List[String]): ArgsSummary =
def stateWithArgs(args: List[String]) = ArgsSummary(state.sstate, args, state.errors, state.warnings)
state.arguments match {
state.arguments match
case Nil =>
checkDependencies(stateWithArgs(skipped))
case "--" :: args =>
checkDependencies(stateWithArgs(skipped ++ args))
case x :: _ if x startsWith "-" =>
@tailrec def loop(settings: List[Setting[?]]): ArgsSummary = settings match {
@tailrec def loop(settings: List[Setting[?]]): ArgsSummary = settings match
case setting :: settings1 =>
val state1 = setting.tryToSet(state)
if (state1 ne state) processArguments(state1, processAll, skipped)
if state1 ne state then state1
else loop(settings1)
case Nil =>
processArguments(state.warn(s"bad option '$x' was ignored"), processAll, skipped)
}
loop(allSettings.toList)
state.warn(s"bad option '$x' was ignored")
processArguments(loop(allSettings.toList), processAll, skipped)
case arg :: args =>
if (processAll) processArguments(stateWithArgs(args), processAll, skipped :+ arg)
if processAll then processArguments(stateWithArgs(args), processAll, skipped :+ arg)
else state
}
}
end processArguments

def processArguments(arguments: List[String], processAll: Boolean, settingsState: SettingsState = defaultState): ArgsSummary =
processArguments(ArgsSummary(settingsState, arguments, Nil, Nil), processAll, Nil)
Expand Down
94 changes: 75 additions & 19 deletions compiler/test/dotty/tools/dotc/SettingsTests.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,14 @@ class SettingsTests {
assertEquals(1, reporter.errorCount)
assertEquals("'not_here' does not exist or is not a directory or .jar file", reporter.allErrors.head.message)

@Test def jarOutput: Unit = {
@Test def jarOutput: Unit =
val source = "tests/pos/Foo.scala"
val out = Paths.get("out/jaredFoo.jar").normalize
if (Files.exists(out)) Files.delete(out)
val options = Array("-classpath", TestConfiguration.basicClasspath, "-d", out.toString, source)
val reporter = Main.process(options)
assertEquals(0, reporter.errorCount)
assertTrue(Files.exists(out))
}

@Test def `t8124 Don't crash on missing argument`: Unit =
val source = Paths.get("tests/pos/Foo.scala").normalize
Expand All @@ -45,15 +44,70 @@ class SettingsTests {
val foo = StringSetting("-foo", "foo", "Foo", "a")
val bar = IntSetting("-bar", "Bar", 0)

inContext {
val args = List("-foo", "b", "-bar", "1")
val summary = Settings.processArguments(args, true)
assertTrue(summary.errors.isEmpty)
given SettingsState = summary.sstate
val args = List("-foo", "b", "-bar", "1")
val summary = Settings.processArguments(args, true)
assertTrue(summary.errors.isEmpty)
withProcessedArgs(summary) {
assertEquals("b", Settings.foo.value)
assertEquals(1, Settings.bar.value)
}

@Test def `workaround dont crash on many files`: Unit =
object Settings extends SettingGroup

val args = "--" :: List.fill(6000)("file.scala")
val summary = Settings.processArguments(args, processAll = true)
assertTrue(summary.errors.isEmpty)
assertEquals(6000, summary.arguments.size)

@Test def `dont crash on many files`: Unit =
object Settings extends SettingGroup

val args = List.fill(6000)("file.scala")
val summary = Settings.processArguments(args, processAll = true)
assertTrue(summary.errors.isEmpty)
assertEquals(6000, summary.arguments.size)

@Test def `dont crash on many options`: Unit =
object Settings extends SettingGroup:
val option = BooleanSetting("-option", "Some option")

val limit = 6000
val args = List.fill(limit)("-option")
val summary = Settings.processArguments(args, processAll = true)
assertTrue(summary.errors.isEmpty)
assertEquals(limit-1, summary.warnings.size)
assertTrue(summary.warnings.head.contains("repeatedly"))
assertEquals(0, summary.arguments.size)
withProcessedArgs(summary) {
assertTrue(Settings.option.value)
}

@Test def `bad option warning consumes an arg`: Unit =
object Settings extends SettingGroup:
val option = BooleanSetting("-option", "Some option")

val args = List("-adoption", "dogs", "cats")
val summary = Settings.processArguments(args, processAll = true)
assertTrue(summary.errors.isEmpty)
assertFalse(summary.warnings.isEmpty)
assertEquals(2, summary.arguments.size)

@Test def `bad option settings throws`: Unit =
object Settings extends SettingGroup:
val option = BooleanSetting("-option", "Some option")

def checkMessage(s: String): (Throwable => Boolean) = t =>
if t.getMessage == s then true
else
println(s"Expected: $s, Actual: ${t.getMessage}")
false

val default = Settings.defaultState
assertThrows[IllegalArgumentException](checkMessage("found: not an option of type java.lang.String, required: Boolean")) {
Settings.option.updateIn(default, "not an option")
}

@Test def validateChoices: Unit =
object Settings extends SettingGroup:
val foo = ChoiceSetting("-foo", "foo", "Foo", List("a", "b"), "a")
Expand All @@ -63,25 +117,27 @@ class SettingsTests {
val quux = ChoiceSetting("-quux", "quux", "Quux", List(), "")
val quuz = IntChoiceSetting("-quuz", "Quuz", List(), 0)

inContext {
locally {
val args = List("-foo", "b", "-bar", "1", "-baz", "5")
val summary = Settings.processArguments(args, true)
assertTrue(summary.errors.isEmpty)
given SettingsState = summary.sstate
assertEquals("b", Settings.foo.value)
assertEquals(1, Settings.bar.value)
assertEquals(5, Settings.baz.value)
withProcessedArgs(summary) {
assertEquals("b", Settings.foo.value)
assertEquals(1, Settings.bar.value)
assertEquals(5, Settings.baz.value)
}
}

inContext {
locally {
val args = List("-foo:b")
val summary = Settings.processArguments(args, true)
assertTrue(summary.errors.isEmpty)
given SettingsState = summary.sstate
assertEquals("b", Settings.foo.value)
withProcessedArgs(summary) {
assertEquals("b", Settings.foo.value)
}
}

inContext {
locally {
val args = List("-foo", "c", "-bar", "3", "-baz", "-1")
val summary = Settings.processArguments(args, true)
val expectedErrors = List(
Expand All @@ -92,14 +148,14 @@ class SettingsTests {
assertEquals(expectedErrors, summary.errors)
}

inContext {
locally {
val args = List("-foo:c")
val summary = Settings.processArguments(args, true)
val expectedErrors = List("c is not a valid choice for -foo")
assertEquals(expectedErrors, summary.errors)
}

inContext {
locally {
val args = List("-quux", "a", "-quuz", "0")
val summary = Settings.processArguments(args, true)
val expectedErrors = List(
Expand All @@ -109,7 +165,7 @@ class SettingsTests {
assertEquals(expectedErrors, summary.errors)
}

private def inContext(f: Context ?=> Unit) = f(using (new ContextBase).initialCtx.fresh)
private def withProcessedArgs(summary: ArgsSummary)(f: SettingsState ?=> Unit) = f(using summary.sstate)

extension [T](setting: Setting[T])
private def value(using ss: SettingsState): T = setting.valueIn(ss)
Expand Down
16 changes: 16 additions & 0 deletions compiler/test/dotty/tools/utils.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ import java.io.File
import java.nio.charset.StandardCharsets.UTF_8

import scala.io.Source
import scala.reflect.ClassTag
import scala.util.Using.resource
import scala.util.chaining.given
import scala.util.control.{ControlThrowable, NonFatal}

def scripts(path: String): Array[File] = {
val dir = new File(getClass.getResource(path).getPath)
Expand All @@ -17,3 +20,16 @@ private def withFile[T](file: File)(action: Source => T): T =

def readLines(f: File): List[String] = withFile(f)(_.getLines.toList)
def readFile(f: File): String = withFile(f)(_.mkString)

private object Unthrown extends ControlThrowable

def assertThrows[T <: Throwable: ClassTag](p: T => Boolean)(body: => Any): Unit =
try
body
throw Unthrown
catch
case Unthrown => throw AssertionError("Expression did not throw!")
case e: T if p(e) => ()
case failed: T => throw AssertionError(s"Exception failed check: $failed").tap(_.addSuppressed(failed))
case NonFatal(other) => throw AssertionError(s"Wrong exception: expected ${implicitly[ClassTag[T]]} but was ${other.getClass.getName}").tap(_.addSuppressed(other))
end assertThrows

0 comments on commit f1df8f3

Please sign in to comment.