From a40af3e42fdfff62901dbc807ed3d899272d2b37 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Fri, 28 Feb 2014 05:25:08 -0800 Subject: SI-5905 Sanity check -language options The option names are hardcoded, but checked by a test. There are no hooks to verify options after the compiler is constructed. Introduced a `MultiChoiceSetting` required for the setting creation framework. --- .../tools/nsc/settings/AbsScalaSettings.scala | 2 ++ .../scala/tools/nsc/settings/MutableSettings.scala | 22 ++++++++++++++--- .../scala/tools/nsc/settings/ScalaSettings.scala | 10 ++++++-- test/files/run/t5905-features.flags | 1 + test/files/run/t5905-features.scala | 28 ++++++++++++++++++++++ test/files/run/t5905b-features.check | 1 + test/files/run/t5905b-features.scala | 15 ++++++++++++ 7 files changed, 74 insertions(+), 5 deletions(-) create mode 100644 test/files/run/t5905-features.flags create mode 100644 test/files/run/t5905-features.scala create mode 100644 test/files/run/t5905b-features.check create mode 100644 test/files/run/t5905b-features.scala diff --git a/src/compiler/scala/tools/nsc/settings/AbsScalaSettings.scala b/src/compiler/scala/tools/nsc/settings/AbsScalaSettings.scala index 8b897b83b2..38a7525862 100644 --- a/src/compiler/scala/tools/nsc/settings/AbsScalaSettings.scala +++ b/src/compiler/scala/tools/nsc/settings/AbsScalaSettings.scala @@ -16,6 +16,7 @@ trait AbsScalaSettings { type ChoiceSetting <: Setting { type T = String } type IntSetting <: Setting { type T = Int } type MultiStringSetting <: Setting { type T = List[String] } + type MultiChoiceSetting <: Setting { type T = List[String] } type PathSetting <: Setting { type T = String } type PhasesSetting <: Setting { type T = List[String] } type StringSetting <: Setting { type T = String } @@ -28,6 +29,7 @@ trait AbsScalaSettings { def ChoiceSetting(name: String, helpArg: String, descr: String, choices: List[String], default: String): ChoiceSetting def IntSetting(name: String, descr: String, default: Int, range: Option[(Int, Int)], parser: String => Option[Int]): IntSetting def MultiStringSetting(name: String, helpArg: String, descr: String): MultiStringSetting + def MultiChoiceSetting(name: String, helpArg: String, descr: String, choices: List[String]): MultiChoiceSetting def OutputSetting(outputDirs: OutputDirs, default: String): OutputSetting def PathSetting(name: String, descr: String, default: String): PathSetting def PhasesSetting(name: String, descr: String, default: String): PhasesSetting diff --git a/src/compiler/scala/tools/nsc/settings/MutableSettings.scala b/src/compiler/scala/tools/nsc/settings/MutableSettings.scala index 3590254128..c22dc564b2 100644 --- a/src/compiler/scala/tools/nsc/settings/MutableSettings.scala +++ b/src/compiler/scala/tools/nsc/settings/MutableSettings.scala @@ -211,6 +211,11 @@ class MutableSettings(val errorFn: String => Unit) add(new ChoiceSetting(name, helpArg, descr, choices, default)) def IntSetting(name: String, descr: String, default: Int, range: Option[(Int, Int)], parser: String => Option[Int]) = add(new IntSetting(name, descr, default, range, parser)) def MultiStringSetting(name: String, arg: String, descr: String) = add(new MultiStringSetting(name, arg, descr)) + def MultiChoiceSetting(name: String, helpArg: String, descr: String, choices: List[String]): MultiChoiceSetting = { + val fullChoix = choices.mkString(": ", ",", ".") + val fullDescr = s"$descr$fullChoix" + add(new MultiChoiceSetting(name, helpArg, fullDescr, choices)) + } def OutputSetting(outputDirs: OutputDirs, default: String) = add(new OutputSetting(outputDirs, default)) def PhasesSetting(name: String, descr: String, default: String = "") = add(new PhasesSetting(name, descr, default)) def StringSetting(name: String, arg: String, descr: String, default: String) = add(new StringSetting(name, arg, descr, default)) @@ -548,8 +553,16 @@ class MutableSettings(val errorFn: String => Unit) } } + class MultiChoiceSetting private[nsc]( + name: String, + arg: String, + descr: String, + override val choices: List[String]) + extends MultiStringSetting(name, arg, descr) + /** A setting that accumulates all strings supplied to it, - * until it encounters one starting with a '-'. */ + * until it encounters one starting with a '-'. + */ class MultiStringSetting private[nsc]( name: String, val arg: String, @@ -558,11 +571,14 @@ class MutableSettings(val errorFn: String => Unit) type T = List[String] protected var v: T = Nil def appendToValue(str: String) { value ++= List(str) } + def badChoice(s: String, n: String) = errorFn(s"'$s' is not a valid choice for '$name'") def tryToSet(args: List[String]) = { val (strings, rest) = args span (x => !x.startsWith("-")) - strings foreach appendToValue - + strings foreach { + case s if choices.isEmpty || (choices contains s) => appendToValue(s) + case s => badChoice(s, name) + } Some(rest) } override def tryToSetColon(args: List[String]) = tryToSet(args) diff --git a/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala b/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala index a643a08614..20ccc30ff6 100644 --- a/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala +++ b/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala @@ -62,8 +62,14 @@ trait ScalaSettings extends AbsScalaSettings /*val argfiles = */ BooleanSetting ("@", "A text file containing compiler arguments (options and source files)") val classpath = PathSetting ("-classpath", "Specify where to find user class files.", defaultClasspath) withAbbreviation "-cp" val d = OutputSetting (outputDirs, ".") - val nospecialization = BooleanSetting ("-no-specialization", "Ignore @specialize annotations.") - val language = MultiStringSetting("-language", "feature", "Enable one or more language features.") + val nospecialization = BooleanSetting ("-no-specialization", "Ignore @specialize annotations.") + + // Would be nice to build this dynamically from scala.languageFeature. + // The two requirements: delay error checking until you have symbols, and let compiler command build option-specific help. + val language = { + val features = List("dynamics", "postfixOps", "reflectiveCalls", "implicitConversions", "higherKinds", "existentials", "experimental.macros") + MultiChoiceSetting("-language", "feature", "Enable one or more language features", features) + } /* * The previous "-source" option is intended to be used mainly diff --git a/test/files/run/t5905-features.flags b/test/files/run/t5905-features.flags new file mode 100644 index 0000000000..ad51758c39 --- /dev/null +++ b/test/files/run/t5905-features.flags @@ -0,0 +1 @@ +-nowarn diff --git a/test/files/run/t5905-features.scala b/test/files/run/t5905-features.scala new file mode 100644 index 0000000000..fbffddf114 --- /dev/null +++ b/test/files/run/t5905-features.scala @@ -0,0 +1,28 @@ + +import tools.partest.DirectTest + +// verify that all languageFeature names are accepted by -language +object Test extends DirectTest { + override def code = "class Code { def f = (1 to 10) size }" // exercise a feature + + override def extraSettings = s"-usejavacp -d ${testOutput.path}" + + override def show() = { + val global = newCompiler("-language:postfixOps", "-Ystop-after:typer") + compileString(global)(code) + import global._ + exitingTyper { + def isFeature(s: Symbol) = s.annotations.exists((a: AnnotationInfo) => a.tpe <:< typeOf[scala.annotation.meta.languageFeature]) + val langf = definitions.languageFeatureModule.typeSignature + val feats = langf.declarations filter (s => isFeature(s)) map (_.name.decoded) + val xmen = langf.member(TermName("experimental")).typeSignature.declarations filter (s => isFeature(s)) map (s => s"experimental.${s.name.decoded}") + val all = (feats ++ xmen) mkString "," + + assert(feats.nonEmpty, "Test must find feature flags.") + + //dynamics,postfixOps,reflectiveCalls,implicitConversions,higherKinds,existentials,experimental.macros + compile(s"-language:$all") + } + } +} + diff --git a/test/files/run/t5905b-features.check b/test/files/run/t5905b-features.check new file mode 100644 index 0000000000..08c76d74aa --- /dev/null +++ b/test/files/run/t5905b-features.check @@ -0,0 +1 @@ +'noob' is not a valid choice for '-language' diff --git a/test/files/run/t5905b-features.scala b/test/files/run/t5905b-features.scala new file mode 100644 index 0000000000..627df8334b --- /dev/null +++ b/test/files/run/t5905b-features.scala @@ -0,0 +1,15 @@ + +import tools.partest.DirectTest + +// verify that only languageFeature names are accepted by -language +object Test extends DirectTest { + override def code = "class Code" + + override def extraSettings = s"-usejavacp -d ${testOutput.path}" + + override def show() = { + //compile("-language", "--") // no error + compile(s"-language:noob") + } +} + -- cgit v1.2.3 From 534cadc762a5dc3a5d211ad3733df91c7b1bd931 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Sat, 1 Mar 2014 09:26:50 -0800 Subject: SI-5905 Restore -language:_ Underscore means all. -x:c,b,a,_ results in value c,b,a,a,b,c,d,... Currently, -Xprint does not present phases as a closed set of choices; there is ad hoc checking in Global. That would be a nice unification. (You don't know the list of choices until after global is constructed.) --- src/compiler/scala/tools/nsc/settings/MutableSettings.scala | 3 +++ src/compiler/scala/tools/nsc/typechecker/Typers.scala | 2 +- test/files/run/t5905-features.scala | 3 ++- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/compiler/scala/tools/nsc/settings/MutableSettings.scala b/src/compiler/scala/tools/nsc/settings/MutableSettings.scala index c22dc564b2..54e444decf 100644 --- a/src/compiler/scala/tools/nsc/settings/MutableSettings.scala +++ b/src/compiler/scala/tools/nsc/settings/MutableSettings.scala @@ -576,6 +576,7 @@ class MutableSettings(val errorFn: String => Unit) def tryToSet(args: List[String]) = { val (strings, rest) = args span (x => !x.startsWith("-")) strings foreach { + case "_" if choices.nonEmpty => choices foreach appendToValue case s if choices.isEmpty || (choices contains s) => appendToValue(s) case s => badChoice(s, name) } @@ -586,6 +587,8 @@ class MutableSettings(val errorFn: String => Unit) def clear(): Unit = (v = Nil) def unparse: List[String] = value map (name + ":" + _) + def contains(s: String) = value contains s + withHelpSyntax(name + ":<" + arg + ">") } diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index 1004777f23..14aa25eeb8 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -738,7 +738,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper val featureName = (nestedOwners map (_.name + ".")).mkString + featureTrait.name def action(): Boolean = { def hasImport = inferImplicit(EmptyTree: Tree, featureTrait.tpe, reportAmbiguous = true, isView = false, context).isSuccess - def hasOption = settings.language.value exists (s => s == featureName || s == "_") + def hasOption = settings.language contains featureName val OK = hasImport || hasOption if (!OK) { val Some(AnnotationInfo(_, List(Literal(Constant(featureDesc: String)), Literal(Constant(required: Boolean))), _)) = diff --git a/test/files/run/t5905-features.scala b/test/files/run/t5905-features.scala index fbffddf114..a3848eef2a 100644 --- a/test/files/run/t5905-features.scala +++ b/test/files/run/t5905-features.scala @@ -12,7 +12,8 @@ object Test extends DirectTest { compileString(global)(code) import global._ exitingTyper { - def isFeature(s: Symbol) = s.annotations.exists((a: AnnotationInfo) => a.tpe <:< typeOf[scala.annotation.meta.languageFeature]) + //def isFeature(s: Symbol) = s.annotations.exists((a: AnnotationInfo) => a.tpe <:< typeOf[scala.annotation.meta.languageFeature]) + def isFeature(s: Symbol) = s hasAnnotation definitions.LanguageFeatureAnnot val langf = definitions.languageFeatureModule.typeSignature val feats = langf.declarations filter (s => isFeature(s)) map (_.name.decoded) val xmen = langf.member(TermName("experimental")).typeSignature.declarations filter (s => isFeature(s)) map (s => s"experimental.${s.name.decoded}") -- cgit v1.2.3 From 78bd175afcc89878ca1c00cce69d0517909c6ff3 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Wed, 7 May 2014 08:11:14 -0700 Subject: SI-5905 Clarify test case The language feature options are discovered reflectively, but it is nice to enforce that expected options are supplied. Short of that, the code string includes a rowdy postfix operator. It still does enforce that at least one option was discovered. Delete -nowarn flags file. Let's see if that was to suppress a warning in the standard build. --- test/files/run/t5905-features.flags | 1 - test/files/run/t5905-features.scala | 8 +++++--- 2 files changed, 5 insertions(+), 4 deletions(-) delete mode 100644 test/files/run/t5905-features.flags diff --git a/test/files/run/t5905-features.flags b/test/files/run/t5905-features.flags deleted file mode 100644 index ad51758c39..0000000000 --- a/test/files/run/t5905-features.flags +++ /dev/null @@ -1 +0,0 @@ --nowarn diff --git a/test/files/run/t5905-features.scala b/test/files/run/t5905-features.scala index a3848eef2a..547a521cb5 100644 --- a/test/files/run/t5905-features.scala +++ b/test/files/run/t5905-features.scala @@ -3,13 +3,13 @@ import tools.partest.DirectTest // verify that all languageFeature names are accepted by -language object Test extends DirectTest { - override def code = "class Code { def f = (1 to 10) size }" // exercise a feature + override def code = "class Code { def f = (1 to 10) size }" // exercise a feature to sanity-check coverage of -language options override def extraSettings = s"-usejavacp -d ${testOutput.path}" override def show() = { - val global = newCompiler("-language:postfixOps", "-Ystop-after:typer") - compileString(global)(code) + val global = newCompiler("-Ystop-after:typer") + compileString(global)("") // warm me up, scotty import global._ exitingTyper { //def isFeature(s: Symbol) = s.annotations.exists((a: AnnotationInfo) => a.tpe <:< typeOf[scala.annotation.meta.languageFeature]) @@ -21,6 +21,8 @@ object Test extends DirectTest { assert(feats.nonEmpty, "Test must find feature flags.") + //compile("junk") // tragically, does not fail the test + //dynamics,postfixOps,reflectiveCalls,implicitConversions,higherKinds,existentials,experimental.macros compile(s"-language:$all") } -- cgit v1.2.3 From bdb1258d2ab8ff10d8e9f4924069a227ae82e690 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Fri, 9 May 2014 16:50:08 +0200 Subject: Revert "SI-5905 Clarify test case" This reverts commit 78bd175afcc89878ca1c00cce69d0517909c6ff3. See discussion: https://github.com/scala/scala/pull/3597#commitcomment-6270375 --- test/files/run/t5905-features.flags | 1 + test/files/run/t5905-features.scala | 8 +++----- 2 files changed, 4 insertions(+), 5 deletions(-) create mode 100644 test/files/run/t5905-features.flags diff --git a/test/files/run/t5905-features.flags b/test/files/run/t5905-features.flags new file mode 100644 index 0000000000..ad51758c39 --- /dev/null +++ b/test/files/run/t5905-features.flags @@ -0,0 +1 @@ +-nowarn diff --git a/test/files/run/t5905-features.scala b/test/files/run/t5905-features.scala index 547a521cb5..a3848eef2a 100644 --- a/test/files/run/t5905-features.scala +++ b/test/files/run/t5905-features.scala @@ -3,13 +3,13 @@ import tools.partest.DirectTest // verify that all languageFeature names are accepted by -language object Test extends DirectTest { - override def code = "class Code { def f = (1 to 10) size }" // exercise a feature to sanity-check coverage of -language options + override def code = "class Code { def f = (1 to 10) size }" // exercise a feature override def extraSettings = s"-usejavacp -d ${testOutput.path}" override def show() = { - val global = newCompiler("-Ystop-after:typer") - compileString(global)("") // warm me up, scotty + val global = newCompiler("-language:postfixOps", "-Ystop-after:typer") + compileString(global)(code) import global._ exitingTyper { //def isFeature(s: Symbol) = s.annotations.exists((a: AnnotationInfo) => a.tpe <:< typeOf[scala.annotation.meta.languageFeature]) @@ -21,8 +21,6 @@ object Test extends DirectTest { assert(feats.nonEmpty, "Test must find feature flags.") - //compile("junk") // tragically, does not fail the test - //dynamics,postfixOps,reflectiveCalls,implicitConversions,higherKinds,existentials,experimental.macros compile(s"-language:$all") } -- cgit v1.2.3