From 14d1fe0c35f4ab07a0535adbdf8acbcbd1208363 Mon Sep 17 00:00:00 2001 From: Denys Shabalin Date: Wed, 2 Jul 2014 14:10:42 +0200 Subject: SI-8703 add support for blocks with just a single expression to quasiquotes Previously it was impossible to match a block that was constructed as Block(Nil, term) Due to the fact that quasiquotes always flatten those into just term. This is a correct behaviour for construction (for sake of consistency with parser) but doing it in deconstruction mode make it impossible to match such blocks which could have been constructed manually somewhere. To fix this we just disable block flattening in deconstruction mode. Interestingly enough this doesn't break existing code due to the fact that quasiquote's block matcher also matches expressions as single-element blocks. This allows to match single-element blocks with patterns like q"{ $foo }". --- src/compiler/scala/tools/reflect/quasiquotes/Parsers.scala | 11 ++++++++--- src/reflect/scala/reflect/internal/TreeGen.scala | 4 ++-- .../scalacheck/quasiquotes/TermDeconstructionProps.scala | 7 +++++++ 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/compiler/scala/tools/reflect/quasiquotes/Parsers.scala b/src/compiler/scala/tools/reflect/quasiquotes/Parsers.scala index b68022afd9..460d02c218 100644 --- a/src/compiler/scala/tools/reflect/quasiquotes/Parsers.scala +++ b/src/compiler/scala/tools/reflect/quasiquotes/Parsers.scala @@ -69,9 +69,14 @@ trait Parsers { self: Quasiquotes => override def makeTupleType(trees: List[Tree]): Tree = TupleTypePlaceholder(trees) // q"{ $x }" - override def makeBlock(stats: List[Tree]): Tree = stats match { - case (head @ Ident(name)) :: Nil if isHole(name) => Block(Nil, head) - case _ => super.makeBlock(stats) + override def makeBlock(stats: List[Tree]): Tree = method match { + case nme.apply => + stats match { + case (head @ Ident(name)) :: Nil if isHole(name) => Block(Nil, head) + case _ => super.makeBlock(stats) + } + case nme.unapply => gen.mkBlock(stats, doFlatten = false) + case other => global.abort("unreachable") } // tq"$a => $b" diff --git a/src/reflect/scala/reflect/internal/TreeGen.scala b/src/reflect/scala/reflect/internal/TreeGen.scala index 9066c73393..6e8e992d16 100644 --- a/src/reflect/scala/reflect/internal/TreeGen.scala +++ b/src/reflect/scala/reflect/internal/TreeGen.scala @@ -451,10 +451,10 @@ abstract class TreeGen { def mkSyntheticUnit() = Literal(Constant(())).updateAttachment(SyntheticUnitAttachment) /** Create block of statements `stats` */ - def mkBlock(stats: List[Tree]): Tree = + def mkBlock(stats: List[Tree], doFlatten: Boolean = true): Tree = if (stats.isEmpty) mkSyntheticUnit() else if (!stats.last.isTerm) Block(stats, mkSyntheticUnit()) - else if (stats.length == 1) stats.head + else if (stats.length == 1 && doFlatten) stats.head else Block(stats.init, stats.last) /** Create a block that wraps multiple statements but don't diff --git a/test/files/scalacheck/quasiquotes/TermDeconstructionProps.scala b/test/files/scalacheck/quasiquotes/TermDeconstructionProps.scala index 49ffaff630..07e8f3faac 100644 --- a/test/files/scalacheck/quasiquotes/TermDeconstructionProps.scala +++ b/test/files/scalacheck/quasiquotes/TermDeconstructionProps.scala @@ -246,4 +246,11 @@ object TermDeconstructionProps extends QuasiquoteProperties("term deconstruction assert(f ≈ `new`) assert(argss.isEmpty) } + + property("SI-8703 extract block with single expression") = test { + val q"{ $a }" = Block(Nil, q"1") + val Literal(Constant(1)) = a + val q"{ $b }" = q"2" + val Literal(Constant(2)) = b + } } -- cgit v1.2.3 From 4c2217e8a0e67ccd675aa4f1be253f87697c9025 Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Tue, 10 Jun 2014 10:55:06 +0200 Subject: Minor cleanups and comments in GenBCode --- .../scala/tools/nsc/backend/jvm/BCodeHelpers.scala | 3 ++ .../scala/tools/nsc/backend/jvm/BCodeTypes.scala | 46 +++++++++++++++++----- .../scala/tools/nsc/backend/jvm/BTypes.scala | 22 +++++++++-- .../scala/tools/nsc/backend/jvm/GenASM.scala | 3 ++ .../scala/tools/nsc/backend/jvm/GenBCode.scala | 2 +- src/reflect/scala/reflect/internal/Symbols.scala | 28 ++++++++++--- 6 files changed, 83 insertions(+), 21 deletions(-) diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala index 51bfdf0027..07890dd5f8 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala @@ -460,6 +460,7 @@ abstract class BCodeHelpers extends BCodeTypes with BytecodeWriters { // Can't call .toInterface (at this phase) or we trip an assertion. // See PackratParser#grow for a method which fails with an apparent mismatch // between "object PackratParsers$class" and "trait PackratParsers" + // TODO @lry do we have a test for that? if (sym.isImplClass) { // pos/spec-List.scala is the sole failure if we don't check for NoSymbol val traitSym = sym.owner.info.decl(tpnme.interfaceName(sym.name)) @@ -469,6 +470,8 @@ abstract class BCodeHelpers extends BCodeTypes with BytecodeWriters { } } + // TODO @lry: code duplication between here and method asmClassType. + assert(hasInternalName(sym), s"Invoked for a symbol lacking JVM internal name: ${sym.fullName}") assert(!phantomTypeMap.contains(sym), "phantom types not supposed to reach here.") diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeTypes.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeTypes.scala index 62dfb4917d..22cb189c70 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeTypes.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeTypes.scala @@ -21,8 +21,9 @@ abstract class BCodeTypes extends BCodeIdiomatic { import global._ import bTypes._ - // when compiling the Scala library, some assertions don't hold (e.g., scala.Boolean has null superClass although it's not an interface) - val isCompilingStdLib = !(settings.sourcepath.isDefault) + // Used only for assertions. When compiling the Scala library, some assertions don't hold + // (e.g., scala.Boolean has null superClass although it's not an interface) + private val isCompilingStdLib = !(settings.sourcepath.isDefault) // special names var StringReference : ClassBType = null @@ -175,12 +176,21 @@ abstract class BCodeTypes extends BCodeIdiomatic { // ------------------------------------------------ /** - * TODO @lry should probably be a map form ClassBType to Tracked + * Type information for classBTypes. + * + * TODO rename Tracked */ - val exemplars = new java.util.concurrent.ConcurrentHashMap[BType, Tracked] + val exemplars = new java.util.concurrent.ConcurrentHashMap[ClassBType, Tracked] /** * Maps class symbols to their corresponding `Tracked` instance. + * + * This map is only used during the first backend phase (Worker1) where ClassDef trees are + * transformed into ClassNode asm trees. In this phase, ClassBTypes and their Tracked are created + * and added to the `exemplars` map. The `symExemplars` map is only used to know if a symbol has + * already been visited. + * + * TODO move this map to the builder class. it's only used during building. can be gc'd with the builder. */ val symExemplars = new java.util.concurrent.ConcurrentHashMap[Symbol, Tracked] @@ -313,7 +323,7 @@ abstract class BCodeTypes extends BCodeIdiomatic { final def isDeprecated(sym: Symbol): Boolean = { sym.annotations exists (_ matches definitions.DeprecatedAttr) } /* must-single-thread */ - final def hasInternalName(sym: Symbol) = { sym.isClass || (sym.isModule && !sym.isMethod) } + final def hasInternalName(sym: Symbol) = sym.isClass || sym.isModuleNotMethod /* must-single-thread */ def getSuperInterfaces(csym: Symbol): List[Symbol] = { @@ -702,6 +712,10 @@ abstract class BCodeTypes extends BCodeIdiomatic { var x = ics while (x ne NoSymbol) { assert(x.isClass, s"not a class symbol: ${x.fullName}") + // Uses `rawowner` because `owner` reflects changes in the owner chain due to flattening. + // The owner chain of a class only contains classes. This is because the lambdalift phase + // changes the `rawowner` destructively to point to the enclosing class. Before, the owner + // might be for example a method. val isInner = !x.rawowner.isPackageClass if (isInner) { chain ::= x @@ -794,14 +808,23 @@ abstract class BCodeTypes extends BCodeIdiomatic { * must-single-thread */ def javaFlags(sym: Symbol): Int = { - // constructors of module classes should be private - // PP: why are they only being marked private at this stage and not earlier? + // constructors of module classes should be private. introduced in b06edbc, probably to prevent + // creating module instances from java. for nested modules, the constructor needs to be public + // since they are created by the outer class and stored in a field. a java client can create + // new instances via outerClassInstance.new InnerModuleClass$(). + // TODO: do this early, mark the symbol private. val privateFlag = sym.isPrivate || (sym.isPrimaryConstructor && isTopLevelModule(sym.owner)) - // Final: the only fields which can receive ACC_FINAL are eager vals. - // Neither vars nor lazy vals can, because: + // Symbols marked in source as `final` have the FINAL flag. (In the past, the flag was also + // added to modules and module classes, not anymore since 296b706). + // Note that the presence of the `FINAL` flag on a symbol does not correspond 1:1 to emitting + // ACC_FINAL in bytecode. + // + // Top-level modules are marked ACC_FINAL in bytecode (even without the FINAL flag). Nested + // objects don't get the flag to allow overriding (under -Yoverride-objects, SI-5676). // + // For fields, only eager val fields can receive ACC_FINAL. vars or lazy vals can't: // Source: http://docs.oracle.com/javase/specs/jls/se7/html/jls-17.html#jls-17.5.3 // "Another problem is that the specification allows aggressive // optimization of final fields. Within a thread, it is permissible to @@ -818,7 +841,6 @@ abstract class BCodeTypes extends BCodeIdiomatic { // we can exclude lateFINAL. Such symbols are eligible for inlining, but to // avoid breaking proxy software which depends on subclassing, we do not // emit ACC_FINAL. - // Nested objects won't receive ACC_FINAL in order to allow for their overriding. val finalFlag = ( (((sym.rawflags & symtab.Flags.FINAL) != 0) || isTopLevelModule(sym)) @@ -845,6 +867,10 @@ abstract class BCodeTypes extends BCodeIdiomatic { if (sym.isVarargsMethod) ACC_VARARGS else 0, if (sym.hasFlag(symtab.Flags.SYNCHRONIZED)) ACC_SYNCHRONIZED else 0 ) + // TODO @lry should probably also check / add "deprectated" + // all call sites of "javaFlags" seem to check for deprecation rigth after. + // Exception: the call below in javaFieldFlags. However, the caller of javaFieldFlags then + // does the check. } /* diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala b/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala index 5b0fa6f7a8..aac4f9cbfc 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala @@ -235,10 +235,24 @@ abstract class BTypes[G <: Global](val __global_dont_use: G) { /** * Class or Interface type. * - * Classes are represented using their name as a slice of the `chrs` array. This representation is - * efficient because the JVM class name is initially created using `classSymbol.javaBinaryName`. - * This already adds the necessary string to the `chrs` array, so it makes sense to reuse the same - * name table in the backend. + * The information for creating a ClassBType (superClass, interfaces, etc) is obtained + * - either from a ClassSymbol, for classes being compiled or referenced from source (see + * BCodeTypes) + * - or, during inlining, from ASM ClassNodes that are parsed from class files. + * + * The class name is represented as a slice of the `chrs` array. This representation is efficient + * because the JVM class name is obtained through `classSymbol.javaBinaryName`. This already adds + * the necessary string to the `chrs` array, so it makes sense to reuse the same name table in the + * backend. + * + * Not a case class because that would expose the constructor that takes (offset, length) + * parameters (I didn't find a way to make it private, also the factory in the companion). + * + * @param offset See below + * @param length The class name is represented as offset and length in the `chrs` array. + * The (public) constructors of ClassBType take a BTypeName, which are + * hash-consed. This ensures that two ClassBType instances for the same name + * have the same offset and length. * * Not a case class because that would expose the (Int, Int) constructor (didn't find a way to * make it private, also the factory in the companion). diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala index 988c04f514..ce7e7c49ff 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala @@ -636,6 +636,9 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM { innerSym.rawname + innerSym.moduleSuffix // add inner classes which might not have been referenced yet + // TODO @lry according to the spec, all nested classes should be added, also local and + // anonymous. This seems to add only member classes - or not? it's exitingErasure, so maybe + // local / anonymous classes have been lifted by lambdalift. are they in the "decls" though? exitingErasure { for (sym <- List(csym, csym.linkedClassOfClass); m <- sym.info.decls.map(innerClassSymbolFor) if m.isClass) innerClassBuffer += m diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala index 9b292fee7f..6b192556d9 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala @@ -271,7 +271,7 @@ abstract class GenBCode extends BCodeSyncAndTry { override def run() { arrivalPos = 0 // just in case - scalaPrimitives.init + scalaPrimitives.init() initBCodeTypes() // initBytecodeWriter invokes fullName, thus we have to run it before the typer-dependent thread is activated. diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala index 2ce54d2259..cc750fb88a 100644 --- a/src/reflect/scala/reflect/internal/Symbols.scala +++ b/src/reflect/scala/reflect/internal/Symbols.scala @@ -1099,13 +1099,28 @@ trait Symbols extends api.Symbols { self: SymbolTable => // ------ owner attribute -------------------------------------------------------------- - /** In general when seeking the owner of a symbol, one should call `owner`. - * The other possibilities include: - * - call `safeOwner` if it is expected that the target may be NoSymbol - * - call `assertOwner` if it is an unrecoverable error if the target is NoSymbol + /** + * The owner of a symbol. Changes over time to adapt to the structure of the trees: + * - Up to lambdalift, the owner is the lexically enclosing definition. For definitions + * in a local block, the owner is also the next enclosing definition. + * - After lambdalift, all local method and class definitions (those not owned by a class + * or package class) change their owner to the enclosing class. This is done through + * a destructive "sym.owner = sym.owner.enclClass". The old owner is saved by + * saveOriginalOwner for the backend (needed to generate the EnclosingMethod attribute). + * - After flatten, all classes are owned by a PackageClass. This is done through a + * phase check (if after flatten) in the (overridden) method "def owner" in + * ModuleSymbol / ClassSymbol. The `rawowner` field is not modified. + * - Owners are also changed in other situations, for example when moving trees into a new + * lexical context, e.g. in the named/default arguments tranformation, or when translating + * extension method definitions. + * + * In general when seeking the owner of a symbol, one should call `owner`. + * The other possibilities include: + * - call `safeOwner` if it is expected that the target may be NoSymbol + * - call `assertOwner` if it is an unrecoverable error if the target is NoSymbol * - * `owner` behaves like `safeOwner`, but logs NoSymbol.owner calls under -Xdev. - * `assertOwner` aborts compilation immediately if called on NoSymbol. + * `owner` behaves like `safeOwner`, but logs NoSymbol.owner calls under -Xdev. + * `assertOwner` aborts compilation immediately if called on NoSymbol. */ def owner: Symbol = { if (Statistics.hotEnabled) Statistics.incCounter(ownerCount) @@ -2811,6 +2826,7 @@ trait Symbols extends api.Symbols { self: SymbolTable => override def owner = { if (Statistics.hotEnabled) Statistics.incCounter(ownerCount) + // a module symbol may have the lateMETHOD flag after refchecks, see isModuleNotMethod if (!isMethod && needsFlatClasses) rawowner.owner else rawowner } -- cgit v1.2.3 From 91c7be16288e060aadb8aa7b0315b12e98621f02 Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Tue, 10 Jun 2014 10:59:51 +0200 Subject: Comment summarizing the JVM spec for InnerClass / EnclosingMethod --- .../scala/tools/nsc/backend/jvm/BTypes.scala | 199 +++++++++++++++++++++ 1 file changed, 199 insertions(+) diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala b/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala index aac4f9cbfc..508c585393 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala @@ -232,6 +232,205 @@ abstract class BTypes[G <: Global](val __global_dont_use: G) { } } + /** + * InnerClass and EnclosingMethod attributes (EnclosingMethod is displayed as OUTERCLASS in asm). + * + * In this summary, "class" means "class or interface". + * + * JLS: http://docs.oracle.com/javase/specs/jls/se8/html/index.html + * JVMS: http://docs.oracle.com/javase/specs/jvms/se8/html/index.html + * + * Terminology + * ----------- + * + * - Nested class (JLS 8): class whose declaration occurs within the body of another class + * + * - Top-level class (JLS 8): non-nested class + * + * - Inner class (JLS 8.1.3): nested class that is not (explicitly or implicitly) static + * + * - Member class (JLS 8.5): class directly enclosed in the body of a class (and not, for + * example, defined in a method). Member classes cannot be anonymous. May be static. + * + * - Local class (JLS 14.3): nested, non-anonymous class that is not a member of a class + * - cannot be static (therefore they are "inner" classes) + * - can be defined in a method, a constructor or in an initializer block + * + * - Initializer block (JLS 8.6 / 8.7): block of statements in a java class + * - static initializer: executed before constructor body + * - instance initializer: exectued when class is initialized (instance creation, static + * field access, ...) + * + * + * InnerClass + * ---------- + * + * The JVMS 4.7.6 requires an entry for every class mentioned in a CONSTANT_Class_info in the + * constant pool (CP) that is not a member of a package (JLS 7.1). + * + * The JLS 13.1, points 9. / 10. requires: a class must reference (in the CP) + * - its immediately enclosing class + * - all of its member classes + * - all local and anonymous classes that appear elsewhere (method, constructor, initializer + * block, field initializer) + * + * In a comment, the 4.7.6 spec says: this implies an entry in the InnerClass attribute for + * - All enclosing classes (except the outermost, which is top-level) + * - My comment: not sure how this is implied, below (*) a Java counter-example. + * In any case, the Java compiler seems to add all enclosing classes, even if they are not + * otherwise mentioned in the CP. So we should do the same. + * - All nested classes (including anonymous and local, but not transitively) + * + * Fields in the InnerClass entries: + * - inner class: the (nested) class C we are talking about + * - outer class: the class of which C is a member. Has to be null for non-members, i.e. for + * local and anonymous classes. + * - inner name: A string with the simple name of the inner class. Null for anonymous classes. + * - flags: access property flags, details in JVMS, table in 4.7.6. + * + * + * Note 1: when a nested class is present in the InnerClass attribute, all of its enclosing + * classes have to be present as well (by the rules above). Example: + * + * class Outer { class I1 { class I2 { } } } + * class User { Outer.I1.I2 foo() { } } + * + * The return type "Outer.I1.I2" puts "Outer$I1$I2" in the CP, therefore the class is added to the + * InnerClass attribute. For this entry, the "outer class" field will be "Outer$I1". This in turn + * adds "Outer$I1" to the CP, which requires adding that class to the InnerClass attribute. + * (For local / anonymous classes this would not be the case, since the "outer class" attribute + * would be empty. However, no class (other than the enclosing class) can refer to them, as they + * have no name.) + * + * In the current implementation of the Scala compiler, when adding a class to the InnerClass + * attribute, all of its enclosing classes will be added as well. Javac seems to do the same, + * see (*). + * + * + * Note 2: If a class name is mentioned only in a CONSTANT_Utf8_info, but not in a + * CONSTANT_Class_info, the JVMS does not require an entry in the InnerClass attribute. However, + * the Java compiler seems to add such classes anyway. For example, when using an annotation, the + * annotation class is stored as a CONSTANT_Utf8_info in the CP: + * + * @O.Ann void foo() { } + * + * adds "const #13 = Asciz LO$Ann;;" in the constant pool. The "RuntimeInvisibleAnnotations" + * attribute refers to that constant pool entry. Even though there is no other reference to + * `O.Ann`, the java compiler adds an entry for that class to the InnerClass attribute (which + * entails adding a CONSTANT_Class_info for the class). + * + * + * + * EnclosingMethod + * --------------- + * + * JVMS 4.7.7: the attribute must be present "if and only if it represents a local class + * or an anonymous class" (i.e. not for member classes). + * + * Fields: + * - class: the enclosing class + * - method: the enclosing method (or constructor). Null if the class is not enclosed by a + * method, i.e. for + * - local or anonymous classes defined in (static or non-static) initializer blocks + * - anonymous classes defined in initializer blocks or field initializers + * + * Note: the field is required for anonymous classes defined within local variable + * initializers (within a method), Java example below (**). + * + * Currently, the Scala compiler sets "method" to the class constructor for classes + * defined in initializer blocks or field initializers. This is probably OK, since the + * Scala compiler desugars these statements into to the primary constructor. + * + * + * (*) + * public class Test { + * void foo() { + * class Foo1 { + * // constructor statement block + * { + * class Foo2 { + * class Foo3 { } + * } + * } + * } + * } + * } + * + * The class file Test$1Foo1$1Foo2$Foo3 has no reference to the class Test$1Foo1, however it + * still contains an InnerClass attribute for Test$1Foo1. + * Maybe this is just because the Java compiler follows the JVMS comment ("InnerClasses + * information for each enclosing class"). + * + * + * (**) + * void foo() { + * // anonymous class defined in local variable initializer expression. + * Runnable x = true ? (new Runnable() { + * public void run() { return; } + * }) : null; + * } + * + * The EnclosingMethod attribute of the anonymous class mentions "foo" in the "method" field. + * + * + * Java Compatibility + * ------------------ + * + * In the InnerClass entry for classes in top-level modules, the "outer class" is emitted as the + * mirror class (or the existing companion class), i.e. C1 is nested in T (not T$). + * For classes nested in a nested object, the "outer class" is the module class: C2 is nested in T$N$ + * object T { + * class C1 + * object N { class C2 } + * } + * + * Reason: java compat. It's a "best effort" "solution". If you want to use "C1" from Java, you + * can write "T.C1", and the Java compiler will translate that to the classfile T$C1. + * + * If we would emit the "outer class" of C1 as "T$", then in Java you'd need to write "T$.C1" + * because the java compiler looks at the InnerClass attribute to find if an inner class exists. + * However, the Java compiler would then translate the '.' to '$' and you'd get the class name + * "T$$C1". This class file obviously does not exist. + * + * Directly using the encoded class name "T$C1" in Java does not work: since the classfile + * describes a nested class, the Java compiler hides it from the classpath and will report + * "cannot find symbol T$C1". This means that the class T.N.C2 cannot be referenced from a + * Java source file in any way. + * + * + * STATIC flag + * ----------- + * + * Java: static nested classes have the "static" flag in the InnerClass attribute. This is not the + * case for local classes defined within a static method, even though such classes, as they are + * defined in a static context, don't take an "outer" instance. + * Non-static nested classes (inner classes, including local classes defined in a non-static + * method) take an "outer" instance on construction. + * + * Scala: Explicitouter adds an "outer" parameter to nested classes, except for classes defined + * in a static context, i.e. when all outer classes are module classes. + * package p + * object O1 { + * class C1 // static + * object O2 { + * def f = { + * class C2 { // static + * class C3 // non-static, needs outer + * } + * } + * } + * } + * + * Int the InnerClass attribute, the `static` flag is added for all classes defined in a static + * context, i.e. also for C2. This is different than in Java. + * + * + * Mirror Classes + * -------------- + * + * TODO: innerclass attributes on mirror class, bean info class + */ + /** * Class or Interface type. * -- cgit v1.2.3 From 57de87e655820df9d016613badc13e3e86059417 Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Tue, 10 Jun 2014 11:00:25 +0200 Subject: Remove unnessecary check when computing InnerClass attribute flags The final flag is computed correctly by javaFlags. --- src/compiler/scala/tools/nsc/backend/jvm/BCodeTypes.scala | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeTypes.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeTypes.scala index 22cb189c70..9b131cb123 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeTypes.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeTypes.scala @@ -755,12 +755,23 @@ abstract class BCodeTypes extends BCodeIdiomatic { innerSym.rawname + innerSym.moduleSuffix } - val flagsWithFinal: Int = mkFlags( + // TODO @lry compare with table in spec: for example, deprecated should not be there it seems. + // http://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.7.6-300-D.1-D.1 + // including "deprecated" was added in the initial commit of GenASM, but it was never in GenJVM. + val flags: Int = mkFlags( + // TODO @lry adding "static" whenever the class is owned by a module seems wrong. + // class C { object O { class I } } + // here, I is marked static in the InnerClass attribute. But the I constructor takes an outer instance. + // was added in 0469d41 + // what should it be? check what would make sense for java reflection. + // member of top-level object should be static? how about anonymous / local class that has + // been lifted to a top-level object? + // member that is only nested in objects should be static? + // verify: will ICodeReader still work after that? the code was introduced because of icode reader. if (innerSym.rawowner.hasModuleFlag) asm.Opcodes.ACC_STATIC else 0, javaFlags(innerSym), if (isDeprecated(innerSym)) asm.Opcodes.ACC_DEPRECATED else 0 // ASM pseudo-access flag ) & (INNER_CLASSES_FLAGS | asm.Opcodes.ACC_DEPRECATED) - val flags = if (innerSym.isModuleClass) flagsWithFinal & ~asm.Opcodes.ACC_FINAL else flagsWithFinal // For SI-5676, object overriding. val jname = innerSym.javaBinaryName.toString // never null val oname = { // null when method-enclosed -- cgit v1.2.3 From 1bed39a1cb2be13cb26a038dfd1649964063c498 Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Tue, 10 Jun 2014 11:17:47 +0200 Subject: Pattern matching on ClassBType extracts the inernalName --- .../scala/tools/nsc/backend/jvm/BTypes.scala | 49 ++++++++++++---------- 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala b/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala index 508c585393..15bc068533 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala @@ -43,9 +43,9 @@ abstract class BTypes[G <: Global](val __global_dont_use: G) { case FLOAT => "F" case LONG => "J" case DOUBLE => "D" - case c @ ClassBType(_, _) => "L" + c.internalName + ";" - case ArrayBType(component) => "[" + component - case MethodBType(args, res) => "(" + args.mkString + ")" + res + case ClassBType(internalName) => "L" + internalName + ";" + case ArrayBType(component) => "[" + component + case MethodBType(args, res) => "(" + args.mkString + ")" + res } /** @@ -160,9 +160,9 @@ abstract class BTypes[G <: Global](val __global_dont_use: G) { case FLOAT => asm.Type.FLOAT_TYPE case LONG => asm.Type.LONG_TYPE case DOUBLE => asm.Type.DOUBLE_TYPE - case c @ ClassBType(_, _) => asm.Type.getObjectType(c.internalName) // (*) - case a @ ArrayBType(_) => asm.Type.getObjectType(a.descriptor) - case m @ MethodBType(_, _) => asm.Type.getMethodType(m.descriptor) + case ClassBType(internalName) => asm.Type.getObjectType(internalName) // see (*) above + case a: ArrayBType => asm.Type.getObjectType(a.descriptor) + case m: MethodBType => asm.Type.getMethodType(m.descriptor) } def asRefBType : RefBType = this.asInstanceOf[RefBType] @@ -227,8 +227,8 @@ abstract class BTypes[G <: Global](val __global_dont_use: G) { * This can be verified for example using javap or ASMifier. */ def classOrArrayType: String = this match { - case c: ClassBType => c.internalName - case a: ArrayBType => a.descriptor + case ClassBType(internalName) => internalName + case a: ArrayBType => a.descriptor } } @@ -458,21 +458,23 @@ abstract class BTypes[G <: Global](val __global_dont_use: G) { */ class ClassBType private(val offset: Int, val length: Int) extends RefBType { /** - * Construct a ClassBType for a given (intenred) class name. + * Construct a ClassBType from the (intenred) internal name of a class. * - * @param n The class name as a slice of the `chrs` array, without the surrounding 'L' and ';'. - * Note that `classSymbol.javaBinaryName` returns exactly such a name. + * @param internalName The internal name as a slice of the `chrs` array. The internal name does + * not have the surrounding 'L' and ';'. Note that + * `classSymbol.javaBinaryName` returns exactly such a name. */ - def this(n: BTypeName) = this(n.start, n.length) + def this(internalName: BTypeName) = this(internalName.start, internalName.length) /** - * Construct a ClassBType for a given java class name. + * Construct a ClassBType from the internal name of a class. * - * @param s A class name of the form "java/lang/String", without the surrounding 'L' and ';'. + * @param internalName The internal name of a class has the form "java/lang/String", without the + * surrounding 'L' and ';'. */ - def this(s: String) = this({ - assert(!(s.head == 'L' && s.last == ';'), s"Descriptor instead of internal name: $s") - createNewName(s) + def this(internalName: String) = this({ + assert(!(internalName.head == 'L' && internalName.last == ';'), s"Descriptor instead of internal name: $internalName") + createNewName(internalName) }) /** @@ -490,7 +492,7 @@ abstract class BTypes[G <: Global](val __global_dont_use: G) { * Custom equals / hashCode are needed because this is not a case class. */ override def equals(o: Any): Boolean = (this eq o.asInstanceOf[Object]) || (o match { - case ClassBType(`offset`, `length`) => true + case c: ClassBType => c.offset == this.offset && c.length == this.length case _ => false }) @@ -504,12 +506,15 @@ abstract class BTypes[G <: Global](val __global_dont_use: G) { } object ClassBType { - def apply(n: BTypeName): ClassBType = new ClassBType(n) - def apply(s: String): ClassBType = new ClassBType(s) + def apply(internalName: BTypeName): ClassBType = new ClassBType(internalName) + def apply(internalName: String): ClassBType = new ClassBType(internalName) - def unapply(c: ClassBType): Option[(Int, Int)] = + /** + * Pattern matching on a ClassBType extracts the `internalName` of the class. + */ + def unapply(c: ClassBType): Option[String] = if (c == null) None - else Some((c.offset, c.length)) + else Some(c.internalName) } case class ArrayBType(componentType: BType) extends RefBType { -- cgit v1.2.3 From 0ccdb151ffe9caa9eae7d01a4f2eacc87fa8f5ff Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Tue, 10 Jun 2014 11:21:13 +0200 Subject: Clean up and document some usages of flags in the backend --- .../tools/nsc/backend/jvm/BCodeBodyBuilder.scala | 2 +- .../tools/nsc/backend/jvm/BCodeSkelBuilder.scala | 2 +- .../scala/tools/nsc/backend/jvm/BCodeTypes.scala | 50 ++++++++++++--- .../scala/tools/nsc/backend/jvm/GenBCode.scala | 2 +- .../scala/tools/nsc/symtab/SymbolLoaders.scala | 41 ++++++++---- .../scala/tools/nsc/transform/Flatten.scala | 11 +++- .../scala/tools/nsc/transform/LazyVals.scala | 4 +- src/reflect/scala/reflect/internal/Symbols.scala | 75 +++++++++++++++++----- .../scala/reflect/runtime/SymbolLoaders.scala | 11 +++- 9 files changed, 154 insertions(+), 44 deletions(-) diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala index bffa4bc51d..b79cf01f41 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala @@ -977,7 +977,7 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder { if (!isModuleInitialized && jMethodName == INSTANCE_CONSTRUCTOR_NAME && jname == INSTANCE_CONSTRUCTOR_NAME && - isStaticModule(siteSymbol)) { + isStaticModuleClass(siteSymbol)) { isModuleInitialized = true mnode.visitVarInsn(asm.Opcodes.ALOAD, 0) mnode.visitFieldInsn( diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala index effc68c5e3..f8ecb8e643 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala @@ -95,7 +95,7 @@ abstract class BCodeSkelBuilder extends BCodeHelpers { claszSymbol = cd.symbol isCZParcelable = isAndroidParcelableClass(claszSymbol) - isCZStaticModule = isStaticModule(claszSymbol) + isCZStaticModule = isStaticModuleClass(claszSymbol) isCZRemote = isRemote(claszSymbol) thisName = internalName(claszSymbol) diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeTypes.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeTypes.scala index 9b131cb123..b373f8d74d 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeTypes.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeTypes.scala @@ -627,20 +627,52 @@ abstract class BCodeTypes extends BCodeIdiomatic { false } - /* + /** * must-single-thread + * + * True for module classes of package level objects. The backend will generate a mirror class for + * such objects. */ - def isTopLevelModule(sym: Symbol): Boolean = { - exitingPickler { sym.isModuleClass && !sym.isImplClass && !sym.isNestedClass } + def isTopLevelModuleClass(sym: Symbol): Boolean = exitingPickler { + // phase travel to pickler required for isNestedClass (looks at owner) + val r = sym.isModuleClass && !sym.isNestedClass + // The mixin phase adds the `lateMODULE` flag to trait implementation classes. Since the flag + // is late, it should not be visible here inside the time travel. We check this. + if (r) assert(!sym.isImplClass, s"isModuleClass should be false for impl class $sym") + r } - /* + /** * must-single-thread + * + * True for module classes of modules that are top-level or owned only by objects. Module classes + * for such objects will get a MODULE$ flag and a corresponding static initializer. */ - def isStaticModule(sym: Symbol): Boolean = { - sym.isModuleClass && !sym.isImplClass && !sym.isLifted + def isStaticModuleClass(sym: Symbol): Boolean = { + /* The implementation of this method is tricky because it is a source-level property. Various + * phases changed the symbol's properties in the meantime. + * + * (1) Phase travel to to pickler is required to exclude implementation classes; they have the + * lateMODULEs after mixin, so isModuleClass would be true. + * + * (2) We cannot use `sym.isStatic` because lambdalift modified (destructively) the owner. For + * example, in + * object T { def f { object U } } + * the owner of U is T, so UModuleClass.isStatic is true. Phase travel does not help here. + * So we basically re-implement `sym.isStaticOwner`, but using the original owner chain. + */ + + def isOriginallyStaticOwner(sym: Symbol): Boolean = { + sym.isPackageClass || sym.isModuleClass && isOriginallyStaticOwner(sym.originalOwner) + } + + exitingPickler { // (1) + sym.isModuleClass && + isOriginallyStaticOwner(sym.originalOwner) // (2) + } } + // --------------------------------------------------------------------- // ---------------- InnerClasses attribute (JVMS 4.7.6) ---------------- // --------------------------------------------------------------------- @@ -743,7 +775,7 @@ abstract class BCodeTypes extends BCodeIdiomatic { null else { val outerName = innerSym.rawowner.javaBinaryName - if (isTopLevelModule(innerSym.rawowner)) nme.stripModuleSuffix(outerName) + if (isTopLevelModuleClass(innerSym.rawowner)) nme.stripModuleSuffix(outerName) else outerName } } @@ -825,7 +857,7 @@ abstract class BCodeTypes extends BCodeIdiomatic { // new instances via outerClassInstance.new InnerModuleClass$(). // TODO: do this early, mark the symbol private. val privateFlag = - sym.isPrivate || (sym.isPrimaryConstructor && isTopLevelModule(sym.owner)) + sym.isPrivate || (sym.isPrimaryConstructor && isTopLevelModuleClass(sym.owner)) // Symbols marked in source as `final` have the FINAL flag. (In the past, the flag was also // added to modules and module classes, not anymore since 296b706). @@ -854,7 +886,7 @@ abstract class BCodeTypes extends BCodeIdiomatic { // emit ACC_FINAL. val finalFlag = ( - (((sym.rawflags & symtab.Flags.FINAL) != 0) || isTopLevelModule(sym)) + (((sym.rawflags & symtab.Flags.FINAL) != 0) || isTopLevelModuleClass(sym)) && !sym.enclClass.isInterface && !sym.isClassConstructor && !sym.isMutable // lazy vals and vars both diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala index 6b192556d9..af5d01458a 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala @@ -165,7 +165,7 @@ abstract class GenBCode extends BCodeSyncAndTry { // -------------- mirror class, if needed -------------- val mirrorC = - if (isStaticModule(claszSymbol) && isTopLevelModule(claszSymbol)) { + if (isTopLevelModuleClass(claszSymbol)) { if (claszSymbol.companionClass == NoSymbol) { mirrorCodeGen.genMirrorClass(claszSymbol, cunit) } else { diff --git a/src/compiler/scala/tools/nsc/symtab/SymbolLoaders.scala b/src/compiler/scala/tools/nsc/symtab/SymbolLoaders.scala index 447fa66ae4..82c2a4d6ed 100644 --- a/src/compiler/scala/tools/nsc/symtab/SymbolLoaders.scala +++ b/src/compiler/scala/tools/nsc/symtab/SymbolLoaders.scala @@ -240,6 +240,12 @@ abstract class SymbolLoaders { } } + private def phaseBeforeRefchecks: Phase = { + var resPhase = phase + while (resPhase.refChecked) resPhase = resPhase.prev + resPhase + } + /** * Load contents of a package */ @@ -248,19 +254,24 @@ abstract class SymbolLoaders { protected def doComplete(root: Symbol) { assert(root.isPackageClass, root) - root.setInfo(new PackageClassInfoType(newScope, root)) - - if (!root.isRoot) { - for (classRep <- classpath.classes) { - initializeFromClassPath(root, classRep) - } - } - if (!root.isEmptyPackageClass) { - for (pkg <- classpath.packages) { - enterPackage(root, pkg.name, new PackageLoader(pkg)) + // Time travel to a phase before refchecks avoids an initialization issue. `openPackageModule` + // creates a module symbol and invokes invokes `companionModule` while the `infos` field is + // still null. This calls `isModuleNotMethod`, which forces the `info` if run after refchecks. + enteringPhase(phaseBeforeRefchecks) { + root.setInfo(new PackageClassInfoType(newScope, root)) + + if (!root.isRoot) { + for (classRep <- classpath.classes) { + initializeFromClassPath(root, classRep) + } } + if (!root.isEmptyPackageClass) { + for (pkg <- classpath.packages) { + enterPackage(root, pkg.name, new PackageLoader(pkg)) + } - openPackageModule(root) + openPackageModule(root) + } } } } @@ -290,7 +301,13 @@ abstract class SymbolLoaders { protected def doComplete(root: Symbol) { val start = if (Statistics.canEnable) Statistics.startTimer(classReadNanos) else null - classfileParser.parse(classfile, root) + + // Running the classfile parser after refchecks can lead to "illegal class file dependency" + // errors. More concretely, the classfile parser calls "sym.companionModule", which calls + // "isModuleNotMethod" on the companion. After refchecks, this method forces the info, which + // may run the classfile parser. This produces the error. + enteringPhase(phaseBeforeRefchecks)(classfileParser.parse(classfile, root)) + if (root.associatedFile eq NoAbstractFile) { root match { // In fact, the ModuleSymbol forwards its setter to the module class diff --git a/src/compiler/scala/tools/nsc/transform/Flatten.scala b/src/compiler/scala/tools/nsc/transform/Flatten.scala index c3fbfae322..fa53ef48b5 100644 --- a/src/compiler/scala/tools/nsc/transform/Flatten.scala +++ b/src/compiler/scala/tools/nsc/transform/Flatten.scala @@ -76,8 +76,17 @@ abstract class Flatten extends InfoTransform { for (sym <- decls) { if (sym.isTerm && !sym.isStaticModule) { decls1 enter sym - if (sym.isModule) + if (sym.isModule) { + // Nested, non-static moduls are transformed to methods. + assert(sym.isMethod, s"Non-static $sym should have the lateMETHOD flag from RefChecks") + // Note that module classes are not entered into the 'decls' of the ClassInfoType + // of the outer class, only the module symbols are. So the current loop does + // not visit module classes. Therefore we set the LIFTED flag here for module + // classes. + // TODO: should we also set the LIFTED flag for static, nested module classes? + // currently they don't get the flag, even though they are lifted to the package sym.moduleClass setFlag LIFTED + } } else if (sym.isClass) liftSymbol(sym) } diff --git a/src/compiler/scala/tools/nsc/transform/LazyVals.scala b/src/compiler/scala/tools/nsc/transform/LazyVals.scala index b71d14a04f..38671ebaae 100644 --- a/src/compiler/scala/tools/nsc/transform/LazyVals.scala +++ b/src/compiler/scala/tools/nsc/transform/LazyVals.scala @@ -192,13 +192,15 @@ abstract class LazyVals extends Transform with TypingTransformers with ast.TreeD def mkSlowPathDef(clazz: Symbol, lzyVal: Symbol, cond: Tree, syncBody: List[Tree], stats: List[Tree], retVal: Tree): Tree = { + // Q: is there a reason to first set owner to `clazz` (by using clazz.newMethod), and then + // changing it to lzyVal.owner very soon after? Could we just do lzyVal.owner.newMethod? val defSym = clazz.newMethod(nme.newLazyValSlowComputeName(lzyVal.name.toTermName), lzyVal.pos, STABLE | PRIVATE) defSym setInfo MethodType(List(), lzyVal.tpe.resultType) defSym.owner = lzyVal.owner debuglog(s"crete slow compute path $defSym with owner ${defSym.owner} for lazy val $lzyVal") if (bitmaps.contains(lzyVal)) bitmaps(lzyVal).map(_.owner = defSym) - val rhs: Tree = (gen.mkSynchronizedCheck(clazz, cond, syncBody, stats)).changeOwner(currentOwner -> defSym) + val rhs: Tree = gen.mkSynchronizedCheck(clazz, cond, syncBody, stats).changeOwner(currentOwner -> defSym) DefDef(defSym, addBitmapDefs(lzyVal, BLOCK(rhs, retVal))) } diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala index cc750fb88a..b6cce4524b 100644 --- a/src/reflect/scala/reflect/internal/Symbols.scala +++ b/src/reflect/scala/reflect/internal/Symbols.scala @@ -55,23 +55,29 @@ trait Symbols extends api.Symbols { self: SymbolTable => def newFreeTypeSymbol(name: TypeName, flags: Long = 0L, origin: String): FreeTypeSymbol = new FreeTypeSymbol(name, origin) initFlags flags - /** The original owner of a class. Used by the backend to generate - * EnclosingMethod attributes. + /** + * This map stores the original owner the the first time the owner of a symbol is re-assigned. + * The original owner of a symbol is needed in some places in the backend. Ideally, owners should + * be versioned like the type history. */ - val originalOwner = perRunCaches.newMap[Symbol, Symbol]() + private val originalOwnerMap = perRunCaches.newMap[Symbol, Symbol]() // TODO - don't allow the owner to be changed without checking invariants, at least // when under some flag. Define per-phase invariants for owner/owned relationships, // e.g. after flatten all classes are owned by package classes, there are lots and // lots of these to be declared (or more realistically, discovered.) - protected def saveOriginalOwner(sym: Symbol) { - if (originalOwner contains sym) () - else originalOwner(sym) = sym.rawowner + protected def saveOriginalOwner(sym: Symbol): Unit = { + // some synthetic symbols have NoSymbol as owner initially + if (sym.owner != NoSymbol) { + if (originalOwnerMap contains sym) () + else originalOwnerMap(sym) = sym.rawowner + } } + protected def originalEnclosingMethod(sym: Symbol): Symbol = { if (sym.isMethod || sym == NoSymbol) sym else { - val owner = originalOwner.getOrElse(sym, sym.rawowner) + val owner = sym.originalOwner if (sym.isLocalDummy) owner.enclClass.primaryConstructor else originalEnclosingMethod(owner) } @@ -757,8 +763,22 @@ trait Symbols extends api.Symbols { self: SymbolTable => * So "isModuleNotMethod" exists not for its achievement in * brevity, but to encapsulate the relevant condition. */ - def isModuleNotMethod = isModule && !isMethod - def isStaticModule = isModuleNotMethod && isStatic + def isModuleNotMethod = { + if (isModule) { + if (phase.refChecked) this.info // force completion to make sure lateMETHOD is there. + !isMethod + } else false + } + + // After RefChecks, the `isStatic` check is mostly redundant: all non-static modules should + // be methods (and vice versa). There's a corner case on the vice-versa with mixed-in module + // symbols: + // trait T { object A } + // object O extends T + // The module symbol A is cloned into T$impl (addInterfaces), and then cloned into O (mixin). + // Since the original A is not static, it's turned into a method. The clone in O however is + // static (owned by a module), but it's also a method. + def isStaticModule = isModuleNotMethod && isStatic final def isInitializedToDefault = !isType && hasAllFlags(DEFAULTINIT | ACCESSOR) final def isThisSym = isTerm && owner.thisSym == this @@ -909,10 +929,31 @@ trait Symbols extends api.Symbols { self: SymbolTable => ) final def isModuleVar = hasFlag(MODULEVAR) - /** Is this symbol static (i.e. with no outer instance)? - * Q: When exactly is a sym marked as STATIC? - * A: If it's a member of a toplevel object, or of an object contained in a toplevel object, or any number of levels deep. - * http://groups.google.com/group/scala-internals/browse_thread/thread/d385bcd60b08faf6 + /** + * Is this symbol static (i.e. with no outer instance)? + * Q: When exactly is a sym marked as STATIC? + * A: If it's a member of a toplevel object, or of an object contained in a toplevel object, or + * any number of levels deep. + * http://groups.google.com/group/scala-internals/browse_thread/thread/d385bcd60b08faf6 + * + * TODO: should this only be invoked on class / module symbols? because there's also `isStaticMember`. + * + * Note: the result of `isStatic` changes over time. + * - Lambdalift local definitions to the class level, the `owner` field is modified. + * object T { def foo { object O } } + * After lambdalift, the OModule.isStatic is true. + * + * - After flatten, nested classes are moved to the package level. Invoking `owner` on a + * class returns a package class, for which `isStaticOwner` is true. For example, + * class C { object O } + * OModuleClass.isStatic is true after flatten. Using phase travel to get before flatten, + * method `owner` returns the class C. + * + * Why not make a stable version of `isStatic`? Maybe some parts of the compiler depend on the + * current implementation. For example + * trait T { def foo = 1 } + * The method `foo` in the implementation class T$impl will be `isStatic`, because trait + * impl classes get the `lateMODULE` flag (T$impl.isStaticOwner is true). */ def isStatic = (this hasFlag STATIC) || owner.isStaticOwner @@ -1106,7 +1147,7 @@ trait Symbols extends api.Symbols { self: SymbolTable => * - After lambdalift, all local method and class definitions (those not owned by a class * or package class) change their owner to the enclosing class. This is done through * a destructive "sym.owner = sym.owner.enclClass". The old owner is saved by - * saveOriginalOwner for the backend (needed to generate the EnclosingMethod attribute). + * saveOriginalOwner. * - After flatten, all classes are owned by a PackageClass. This is done through a * phase check (if after flatten) in the (overridden) method "def owner" in * ModuleSymbol / ClassSymbol. The `rawowner` field is not modified. @@ -1129,6 +1170,11 @@ trait Symbols extends api.Symbols { self: SymbolTable => final def safeOwner: Symbol = if (this eq NoSymbol) NoSymbol else owner final def assertOwner: Symbol = if (this eq NoSymbol) abort("no-symbol does not have an owner") else owner + /** + * The initial owner of this symbol. + */ + def originalOwner: Symbol = originalOwnerMap.getOrElse(this, rawowner) + // TODO - don't allow the owner to be changed without checking invariants, at least // when under some flag. Define per-phase invariants for owner/owned relationships, // e.g. after flatten all classes are owned by package classes, there are lots and @@ -1142,7 +1188,6 @@ trait Symbols extends api.Symbols { self: SymbolTable => } def ownerChain: List[Symbol] = this :: owner.ownerChain - def originalOwnerChain: List[Symbol] = this :: originalOwner.getOrElse(this, rawowner).originalOwnerChain // Non-classes skip self and return rest of owner chain; overridden in ClassSymbol. def enclClassChain: List[Symbol] = owner.enclClassChain diff --git a/src/reflect/scala/reflect/runtime/SymbolLoaders.scala b/src/reflect/scala/reflect/runtime/SymbolLoaders.scala index 7ba68b8733..50ea8d9868 100644 --- a/src/reflect/scala/reflect/runtime/SymbolLoaders.scala +++ b/src/reflect/scala/reflect/runtime/SymbolLoaders.scala @@ -65,10 +65,15 @@ private[reflect] trait SymbolLoaders { self: SymbolTable => class LazyPackageType extends LazyType with FlagAgnosticCompleter { override def complete(sym: Symbol) { assert(sym.isPackageClass) - sym setInfo new ClassInfoType(List(), new PackageScope(sym), sym) + // Time travel to a phase before refchecks avoids an initialization issue. `openPackageModule` + // creates a module symbol and invokes invokes `companionModule` while the `infos` field is + // still null. This calls `isModuleNotMethod`, which forces the `info` if run after refchecks. + slowButSafeEnteringPhaseNotLaterThan(picklerPhase) { + sym setInfo new ClassInfoType(List(), new PackageScope(sym), sym) // override def safeToString = pkgClass.toString - openPackageModule(sym) - markAllCompleted(sym) + openPackageModule(sym) + markAllCompleted(sym) + } } } -- cgit v1.2.3 From ee706b873a288deaba55df0b55768e86af0b0167 Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Tue, 10 Jun 2014 11:37:26 +0200 Subject: Support writing classfile of version 52 --- src/compiler/scala/tools/nsc/backend/jvm/BCodeIdiomatic.scala | 1 + src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala | 1 + src/compiler/scala/tools/nsc/settings/StandardScalaSettings.scala | 2 +- 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeIdiomatic.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeIdiomatic.scala index 9b7c975960..2343d378db 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeIdiomatic.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeIdiomatic.scala @@ -34,6 +34,7 @@ abstract class BCodeIdiomatic extends SubComponent { case "jvm-1.5" => asm.Opcodes.V1_5 case "jvm-1.6" => asm.Opcodes.V1_6 case "jvm-1.7" => asm.Opcodes.V1_7 + case "jvm-1.8" => asm.Opcodes.V1_8 } val majorVersion: Int = (classfileVersion & 0xFF) diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala index ce7e7c49ff..d04b111d58 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala @@ -381,6 +381,7 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM { case "jvm-1.5" => asm.Opcodes.V1_5 case "jvm-1.6" => asm.Opcodes.V1_6 case "jvm-1.7" => asm.Opcodes.V1_7 + case "jvm-1.8" => asm.Opcodes.V1_8 } private val majorVersion: Int = (classfileVersion & 0xFF) diff --git a/src/compiler/scala/tools/nsc/settings/StandardScalaSettings.scala b/src/compiler/scala/tools/nsc/settings/StandardScalaSettings.scala index 37dfafb01c..d42c0dd730 100644 --- a/src/compiler/scala/tools/nsc/settings/StandardScalaSettings.scala +++ b/src/compiler/scala/tools/nsc/settings/StandardScalaSettings.scala @@ -39,7 +39,7 @@ trait StandardScalaSettings { val optimise: BooleanSetting // depends on post hook which mutates other settings val print = BooleanSetting ("-print", "Print program with Scala-specific features removed.") val target = ChoiceSetting ("-target", "target", "Target platform for object files. All JVM 1.5 targets are deprecated.", - List("jvm-1.5", "jvm-1.6", "jvm-1.7"), "jvm-1.6") + List("jvm-1.5", "jvm-1.6", "jvm-1.7", "jvm-1.8"), "jvm-1.6") val unchecked = BooleanSetting ("-unchecked", "Enable additional warnings where generated code depends on assumptions.") val uniqid = BooleanSetting ("-uniqid", "Uniquely tag all identifiers in debugging output.") val usejavacp = BooleanSetting ("-usejavacp", "Utilize the java.class.path in classpath resolution.") -- cgit v1.2.3 From a8c88b194eefd5d4d55361b934faa0ebd954ef08 Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Tue, 10 Jun 2014 11:38:24 +0200 Subject: Documentation for isModuleClass --- src/reflect/scala/reflect/api/Symbols.scala | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/reflect/scala/reflect/api/Symbols.scala b/src/reflect/scala/reflect/api/Symbols.scala index dddd3c0e61..42cf600c85 100644 --- a/src/reflect/scala/reflect/api/Symbols.scala +++ b/src/reflect/scala/reflect/api/Symbols.scala @@ -260,6 +260,9 @@ trait Symbols { self: Universe => * with an object definition (module class in scala compiler parlance). * If yes, `isType` is also guaranteed to be true. * + * Note to compiler developers: During the "mixin" phase, trait implementation class symbols + * receive the `lateMODULE` flag, hence `isImplClass && isModuleClass` becomes true. + * * @group Tests */ def isModuleClass: Boolean = false -- cgit v1.2.3 From 2103dbc5230ddf2a369389f179f4ef70eae344f2 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Thu, 22 May 2014 09:02:18 -0700 Subject: SI-8610 -Xlint is multichoice option Make -Xlint a "multichoice" option for purposes of option parsing. This allows turning on "lint with these warnings" instead of only "turn off these warnings but enable other lint warnings". ``` $ scalac -Xlint:warn-adapted-args linty.scala # lint plus a warning $ scalac -Xlint warn-adapted-args linty.scala # same $ scalac -Xlint linty.scala # same as now $ scalac -Xlint -- linty.scala # ok, not necessary $ scalac -Xlint _ -- linty.scala # another funky underscore ``` This would also enable Xlint options that are not standalone options, although that is not implemented in this commit. For example, `-Xlint:no-missing-interpolator` could be used to disable that warning. (There is no `-Xoption:flavor=off` syntax.) (`no-` switches would not be enabled by `_`.) --- .../tools/nsc/settings/AbsScalaSettings.scala | 2 +- .../scala/tools/nsc/settings/MutableSettings.scala | 48 ++++++++++++++-------- .../scala/tools/nsc/settings/Warnings.scala | 22 +++++++++- test/files/neg/t8610-arg.check | 9 ++++ test/files/neg/t8610-arg.flags | 1 + test/files/neg/t8610-arg.scala | 7 ++++ test/files/neg/t8610.check | 15 +++++++ test/files/neg/t8610.flags | 1 + test/files/neg/t8610.scala | 7 ++++ test/files/run/t8610.check | 10 +++++ test/files/run/t8610.flags | 1 + test/files/run/t8610.scala | 13 ++++++ 12 files changed, 116 insertions(+), 20 deletions(-) create mode 100644 test/files/neg/t8610-arg.check create mode 100644 test/files/neg/t8610-arg.flags create mode 100644 test/files/neg/t8610-arg.scala create mode 100644 test/files/neg/t8610.check create mode 100644 test/files/neg/t8610.flags create mode 100644 test/files/neg/t8610.scala create mode 100644 test/files/run/t8610.check create mode 100644 test/files/run/t8610.flags create mode 100644 test/files/run/t8610.scala diff --git a/src/compiler/scala/tools/nsc/settings/AbsScalaSettings.scala b/src/compiler/scala/tools/nsc/settings/AbsScalaSettings.scala index 38a7525862..ef9695a594 100644 --- a/src/compiler/scala/tools/nsc/settings/AbsScalaSettings.scala +++ b/src/compiler/scala/tools/nsc/settings/AbsScalaSettings.scala @@ -29,7 +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 MultiChoiceSetting(name: String, helpArg: String, descr: String, choices: List[String], default: () => Unit): 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 54e444decf..5b5c80c514 100644 --- a/src/compiler/scala/tools/nsc/settings/MutableSettings.scala +++ b/src/compiler/scala/tools/nsc/settings/MutableSettings.scala @@ -211,10 +211,10 @@ 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 = { + def MultiChoiceSetting(name: String, helpArg: String, descr: String, choices: List[String], default: () => Unit = () => ()): MultiChoiceSetting = { val fullChoix = choices.mkString(": ", ",", ".") val fullDescr = s"$descr$fullChoix" - add(new MultiChoiceSetting(name, helpArg, fullDescr, choices)) + add(new MultiChoiceSetting(name, helpArg, fullDescr, choices, default)) } 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)) @@ -553,12 +553,18 @@ class MutableSettings(val errorFn: String => Unit) } } + /** A setting that receives any combination of enumerated values, + * including "_" to mean all values. + * In non-colonated mode, stops consuming args at the first + * non-value, instead of at the next option, as for a multi-string. + */ class MultiChoiceSetting private[nsc]( name: String, arg: String, descr: String, - override val choices: List[String]) - extends MultiStringSetting(name, arg, descr) + override val choices: List[String], + override val default: () => Unit + ) extends MultiStringSetting(name, arg, descr) /** A setting that accumulates all strings supplied to it, * until it encounters one starting with a '-'. @@ -570,24 +576,34 @@ class MutableSettings(val errorFn: String => Unit) extends Setting(name, descr) with Clearable { type T = List[String] protected var v: T = Nil + val default: () => Unit = () => () // no natural default 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 { - case "_" if choices.nonEmpty => choices foreach appendToValue + private def tts(args: List[String], verify: Boolean) = { + def tryArg(arg: String) = arg match { + case "_" if choices.nonEmpty => choices foreach appendToValue case s if choices.isEmpty || (choices contains s) => appendToValue(s) - case s => badChoice(s, name) + case s => badChoice(s, name) } + def loop(args: List[String]): List[String] = args match { + case Nil => Nil + case arg :: _ + if (arg startsWith "-") || (!verify && choices.nonEmpty && !(choices contains arg)) + => args + case arg :: rest => tryArg(arg) ; loop(rest) + } + val rest = loop(args) + if (rest.size == args.size) default() // if no arg consumed, trigger default action Some(rest) } - override def tryToSetColon(args: List[String]) = tryToSet(args) + def tryToSet(args: List[String]) = tts(args, verify = false) + override def tryToSetColon(args: List[String]) = tts(args, verify = true) override def tryToSetFromPropertyValue(s: String) = tryToSet(s.trim.split(',').toList) // used from ide - def clear(): Unit = (v = Nil) - def unparse: List[String] = value map (name + ":" + _) - def contains(s: String) = value contains s + def clear(): Unit = (v = Nil) + def unparse: List[String] = value map (name + ":" + _) + def contains(s: String) = value contains s withHelpSyntax(name + ":<" + arg + ">") } @@ -606,10 +622,8 @@ class MutableSettings(val errorFn: String => Unit) protected var v: T = default def indexOfChoice: Int = choices indexOf value - private def usageErrorMessage = { - "Usage: %s:<%s>\n where <%s> choices are %s (default: %s)\n".format( - name, helpArg, helpArg, choices mkString ", ", default) - } + private def usageErrorMessage = f"Usage: $name:<$helpArg>%n where <$helpArg> choices are ${choices mkString ", "} (default: $default)%n" + def tryToSet(args: List[String]) = errorAndValue(usageErrorMessage, None) override def tryToSetColon(args: List[String]) = args match { diff --git a/src/compiler/scala/tools/nsc/settings/Warnings.scala b/src/compiler/scala/tools/nsc/settings/Warnings.scala index 1509ad13b8..ae2696accf 100644 --- a/src/compiler/scala/tools/nsc/settings/Warnings.scala +++ b/src/compiler/scala/tools/nsc/settings/Warnings.scala @@ -50,8 +50,26 @@ trait Warnings { val warnUnused = BooleanSetting ("-Ywarn-unused", "Warn when local and private vals, vars, defs, and types are are unused") val warnUnusedImport = BooleanSetting ("-Ywarn-unused-import", "Warn when imports are unused") - // Warning groups. - val lint = BooleanSetting("-Xlint", "Enable recommended additional warnings.") enablingIfNotSetByUser lintWarnings + // Warning groups. + val lint = { + // Boolean setting for testing if lint is on; not "added" to option processing + val xlint = new BooleanSetting("-Xlint", "Enable recommended additional warnings.") + val lintables = (lintWarnings map (_.name drop 2)).sorted + def propagate(ss: List[String]): Unit = ss match { + case w :: rest => lintWarnings find (_.name == s"-Y$w") foreach (_.value = true) ; propagate(rest) + case Nil => () + } + def enableAll(): Unit = { // enable lint and the group + xlint.value = true + for (s <- lintWarnings if !s.isSetByUser) s.value = true + } + // The command option + MultiChoiceSetting("-Xlint", "warning", "Enable recommended additional warnings", lintables, enableAll) withPostSetHook { x => + propagate(x.value) // enabling the selections (on each append to value) + xlint.value = true // only enables lint, not the group + } + xlint + } // Backward compatibility. @deprecated("Use fatalWarnings", "2.11.0") def Xwarnfatal = fatalWarnings // used by sbt diff --git a/test/files/neg/t8610-arg.check b/test/files/neg/t8610-arg.check new file mode 100644 index 0000000000..f2879b0d45 --- /dev/null +++ b/test/files/neg/t8610-arg.check @@ -0,0 +1,9 @@ +t8610-arg.scala:3: warning: `$name` looks like an interpolated identifier! Did you forget the interpolator? + def x = "Hi, $name" // missing interp + ^ +t8610-arg.scala:6: warning: side-effecting nullary methods are discouraged: suggest defining as `def u()` instead + def u: Unit = () // unitarian universalist + ^ +error: No warnings can be incurred under -Xfatal-warnings. +two warnings found +one error found diff --git a/test/files/neg/t8610-arg.flags b/test/files/neg/t8610-arg.flags new file mode 100644 index 0000000000..f8867a7b4e --- /dev/null +++ b/test/files/neg/t8610-arg.flags @@ -0,0 +1 @@ +-Xfatal-warnings -Xlint warn-nullary-unit diff --git a/test/files/neg/t8610-arg.scala b/test/files/neg/t8610-arg.scala new file mode 100644 index 0000000000..494a3354da --- /dev/null +++ b/test/files/neg/t8610-arg.scala @@ -0,0 +1,7 @@ + +case class X(name: String) { + def x = "Hi, $name" // missing interp + def f(p: (Int, Int)): Int = p._1 * p._2 + def g = f(3, 4) // adapted + def u: Unit = () // unitarian universalist +} diff --git a/test/files/neg/t8610.check b/test/files/neg/t8610.check new file mode 100644 index 0000000000..7a18e55127 --- /dev/null +++ b/test/files/neg/t8610.check @@ -0,0 +1,15 @@ +t8610.scala:3: warning: `$name` looks like an interpolated identifier! Did you forget the interpolator? + def x = "Hi, $name" // missing interp + ^ +t8610.scala:5: warning: Adapting argument list by creating a 2-tuple: this may not be what you want. + signature: X.f(p: (Int, Int)): Int + given arguments: 3, 4 + after adaptation: X.f((3, 4): (Int, Int)) + def g = f(3, 4) // adapted + ^ +t8610.scala:6: warning: side-effecting nullary methods are discouraged: suggest defining as `def u()` instead + def u: Unit = () // unitarian universalist + ^ +error: No warnings can be incurred under -Xfatal-warnings. +three warnings found +one error found diff --git a/test/files/neg/t8610.flags b/test/files/neg/t8610.flags new file mode 100644 index 0000000000..954eaba352 --- /dev/null +++ b/test/files/neg/t8610.flags @@ -0,0 +1 @@ +-Xfatal-warnings -Xlint diff --git a/test/files/neg/t8610.scala b/test/files/neg/t8610.scala new file mode 100644 index 0000000000..494a3354da --- /dev/null +++ b/test/files/neg/t8610.scala @@ -0,0 +1,7 @@ + +case class X(name: String) { + def x = "Hi, $name" // missing interp + def f(p: (Int, Int)): Int = p._1 * p._2 + def g = f(3, 4) // adapted + def u: Unit = () // unitarian universalist +} diff --git a/test/files/run/t8610.check b/test/files/run/t8610.check new file mode 100644 index 0000000000..077382fb1c --- /dev/null +++ b/test/files/run/t8610.check @@ -0,0 +1,10 @@ +t8610.scala:4: warning: `$name` looks like an interpolated identifier! Did you forget the interpolator? + def x = "Hi, $name" // missing interp + ^ +t8610.scala:6: warning: Adapting argument list by creating a 2-tuple: this may not be what you want. + signature: X.f(p: (Int, Int)): Int + given arguments: 3, 4 + after adaptation: X.f((3, 4): (Int, Int)) + def g = f(3, 4) // adapted + ^ +Hi, $name diff --git a/test/files/run/t8610.flags b/test/files/run/t8610.flags new file mode 100644 index 0000000000..edcb5f4d0c --- /dev/null +++ b/test/files/run/t8610.flags @@ -0,0 +1 @@ +-Xlint:warn-adapted-args diff --git a/test/files/run/t8610.scala b/test/files/run/t8610.scala new file mode 100644 index 0000000000..dd9e8e861e --- /dev/null +++ b/test/files/run/t8610.scala @@ -0,0 +1,13 @@ + +// flags don't warn on u +case class X(name: String) { + def x = "Hi, $name" // missing interp + def f(p: (Int, Int)): Int = p._1 * p._2 + def g = f(3, 4) // adapted + def u: Unit = () // unitarian universalist +} + +object Test extends App { + // poignant demonstration + Console println X("Bob").x +} -- cgit v1.2.3 From 91670d91824493e8bbbd88e9861e04e710311cbb Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Sat, 24 May 2014 01:31:20 -0700 Subject: SI-8616 Error on -deprecation:true,false This is an error, not a crash. Alternatively, one could define the multivalued colon case as equivalent to specifying the option multiple times. That would be very regular. But sometimes it's nicer just to error out. --- src/compiler/scala/tools/nsc/settings/MutableSettings.scala | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/compiler/scala/tools/nsc/settings/MutableSettings.scala b/src/compiler/scala/tools/nsc/settings/MutableSettings.scala index 5b5c80c514..b399c0e024 100644 --- a/src/compiler/scala/tools/nsc/settings/MutableSettings.scala +++ b/src/compiler/scala/tools/nsc/settings/MutableSettings.scala @@ -444,7 +444,7 @@ class MutableSettings(val errorFn: String => Unit) value = s.equalsIgnoreCase("true") } override def tryToSetColon(args: List[String]) = args match { - case Nil => tryToSet(Nil) + case Nil => tryToSet(Nil) case List(x) => if (x.equalsIgnoreCase("true")) { value = true @@ -452,7 +452,8 @@ class MutableSettings(val errorFn: String => Unit) } else if (x.equalsIgnoreCase("false")) { value = false Some(Nil) - } else errorAndValue("'" + x + "' is not a valid choice for '" + name + "'", None) + } else errorAndValue(s"'$x' is not a valid choice for '$name'", None) + case _ => errorAndValue(s"'$name' accepts only one boolean value", None) } } -- cgit v1.2.3 From 44855dcd3c2e19d5dbaf01b2165ea8dc9fb287d3 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Sat, 24 May 2014 06:08:17 -0700 Subject: SI-8525 Add -Xlint:-warn-missing-interpolator Turn off lint warnings with negating prefix, and add a lint-only warning for the infamously nagging "Did you forget the interpolator?" That message is made more dignified. Without `-Xlint:false`, there is no mechanism to turn off anonymous linters once `-Xlint` is selected. --- .../scala/tools/nsc/settings/MutableSettings.scala | 61 +++++++++++++--------- .../scala/tools/nsc/settings/Warnings.scala | 24 ++++++--- .../scala/tools/nsc/typechecker/Typers.scala | 8 +-- test/files/neg/forgot-interpolator.check | 16 +++--- test/files/neg/t7848-interp-warn.check | 6 +-- test/files/neg/t8525.check | 6 +++ test/files/neg/t8525.flags | 1 + test/files/neg/t8525.scala | 10 ++++ test/files/neg/t8610-arg.check | 9 ++-- test/files/neg/t8610-arg.scala | 5 +- test/files/neg/t8610.check | 11 ++-- test/files/neg/t8610.scala | 5 +- test/files/run/t8610.check | 2 +- .../scala/tools/nsc/settings/SettingsTest.scala | 18 ++++++- 14 files changed, 123 insertions(+), 59 deletions(-) create mode 100644 test/files/neg/t8525.check create mode 100644 test/files/neg/t8525.flags create mode 100644 test/files/neg/t8525.scala diff --git a/src/compiler/scala/tools/nsc/settings/MutableSettings.scala b/src/compiler/scala/tools/nsc/settings/MutableSettings.scala index b399c0e024..dc49e8b822 100644 --- a/src/compiler/scala/tools/nsc/settings/MutableSettings.scala +++ b/src/compiler/scala/tools/nsc/settings/MutableSettings.scala @@ -211,11 +211,8 @@ 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], default: () => Unit = () => ()): MultiChoiceSetting = { - val fullChoix = choices.mkString(": ", ",", ".") - val fullDescr = s"$descr$fullChoix" - add(new MultiChoiceSetting(name, helpArg, fullDescr, choices, default)) - } + def MultiChoiceSetting(name: String, helpArg: String, descr: String, choices: List[String], default: () => Unit = () => ()) = + add(new MultiChoiceSetting(name, helpArg, descr, choices, default)) 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)) @@ -564,8 +561,33 @@ class MutableSettings(val errorFn: String => Unit) arg: String, descr: String, override val choices: List[String], - override val default: () => Unit - ) extends MultiStringSetting(name, arg, descr) + val default: () => Unit + ) extends MultiStringSetting(name, arg, s"$descr${ choices.mkString(": ", ",", ".") }") { + + def badChoice(s: String, n: String) = errorFn(s"'$s' is not a valid choice for '$name'") + def choosing = choices.nonEmpty + def isChoice(s: String) = (s == "_") || (choices contains (s stripPrefix "-")) + def wildcards = choices // filter (!_.isSetByUser) + + override protected def tts(args: List[String], halting: Boolean) = { + val total = collection.mutable.ListBuffer.empty[String] ++ value + def tryArg(arg: String) = arg match { + case "_" if choosing => wildcards foreach (total += _) + case s if !choosing || isChoice(s) => total += s + case s => badChoice(s, name) + } + def stoppingAt(arg: String) = (arg startsWith "-") || (choosing && !isChoice(arg)) + def loop(args: List[String]): List[String] = args match { + case arg :: _ if halting && stoppingAt(arg) => args + case arg :: rest => tryArg(arg) ; loop(rest) + case Nil => Nil + } + val rest = loop(args) + if (rest.size == args.size) default() // if no arg consumed, trigger default action + else value = total.toList // update once + Some(rest) + } + } /** A setting that accumulates all strings supplied to it, * until it encounters one starting with a '-'. @@ -577,29 +599,18 @@ class MutableSettings(val errorFn: String => Unit) extends Setting(name, descr) with Clearable { type T = List[String] protected var v: T = Nil - val default: () => Unit = () => () // no natural default - def appendToValue(str: String) { value ++= List(str) } - def badChoice(s: String, n: String) = errorFn(s"'$s' is not a valid choice for '$name'") + def appendToValue(str: String) = value ++= List(str) - private def tts(args: List[String], verify: Boolean) = { - def tryArg(arg: String) = arg match { - case "_" if choices.nonEmpty => choices foreach appendToValue - case s if choices.isEmpty || (choices contains s) => appendToValue(s) - case s => badChoice(s, name) - } + // try to set. halting means halt at first non-arg + protected def tts(args: List[String], halting: Boolean) = { def loop(args: List[String]): List[String] = args match { + case arg :: rest => if (halting && (arg startsWith "-")) args else { appendToValue(arg) ; loop(rest) } case Nil => Nil - case arg :: _ - if (arg startsWith "-") || (!verify && choices.nonEmpty && !(choices contains arg)) - => args - case arg :: rest => tryArg(arg) ; loop(rest) } - val rest = loop(args) - if (rest.size == args.size) default() // if no arg consumed, trigger default action - Some(rest) + Some(loop(args)) } - def tryToSet(args: List[String]) = tts(args, verify = false) - override def tryToSetColon(args: List[String]) = tts(args, verify = true) + def tryToSet(args: List[String]) = tts(args, halting = true) + override def tryToSetColon(args: List[String]) = tts(args, halting = false) override def tryToSetFromPropertyValue(s: String) = tryToSet(s.trim.split(',').toList) // used from ide def clear(): Unit = (v = Nil) diff --git a/src/compiler/scala/tools/nsc/settings/Warnings.scala b/src/compiler/scala/tools/nsc/settings/Warnings.scala index ae2696accf..f1ed45a2a0 100644 --- a/src/compiler/scala/tools/nsc/settings/Warnings.scala +++ b/src/compiler/scala/tools/nsc/settings/Warnings.scala @@ -31,9 +31,10 @@ trait Warnings { warnNullaryOverride, warnNullaryUnit, warnAdaptedArgs, - warnInferAny + warnInferAny, // warnUnused SI-7712, SI-7707 warnUnused not quite ready for prime-time // warnUnusedImport currently considered too noisy for general use + warnMissingInterpolator ) private lazy val warnSelectNullable = BooleanSetting("-Xcheck-null", "This option is obsolete and does nothing.") @@ -50,23 +51,32 @@ trait Warnings { val warnUnused = BooleanSetting ("-Ywarn-unused", "Warn when local and private vals, vars, defs, and types are are unused") val warnUnusedImport = BooleanSetting ("-Ywarn-unused-import", "Warn when imports are unused") + // Lint warnings that are not -Y + val warnMissingInterpolator = new BooleanSetting("warn-missing-interpolator", "Warn when a string literal appears to be missing an interpolator id.") + // Warning groups. val lint = { // Boolean setting for testing if lint is on; not "added" to option processing val xlint = new BooleanSetting("-Xlint", "Enable recommended additional warnings.") - val lintables = (lintWarnings map (_.name drop 2)).sorted + def lintables = (lintWarnings map (_.name stripPrefix "-Y")).sorted + def isAnon(b: BooleanSetting) = !(b.name startsWith "-") + def setPolitely(b: BooleanSetting, v: Boolean) = if (!b.isSetByUser) b.value = v + def set(w: String, v: Boolean) = lintWarnings find (_.name.stripPrefix("-Y") == w) foreach (b => setPolitely(b, v)) + val Neg = "-" def propagate(ss: List[String]): Unit = ss match { - case w :: rest => lintWarnings find (_.name == s"-Y$w") foreach (_.value = true) ; propagate(rest) - case Nil => () + case w :: rest => if (w startsWith Neg) set(w stripPrefix Neg, false) else set(w, true) ; propagate(rest) + case Nil => () } - def enableAll(): Unit = { // enable lint and the group + // enable lint and the group, honoring previous -Y settings + def enableAll(): Unit = { xlint.value = true - for (s <- lintWarnings if !s.isSetByUser) s.value = true + for (s <- lintWarnings) setPolitely(s, true) } // The command option - MultiChoiceSetting("-Xlint", "warning", "Enable recommended additional warnings", lintables, enableAll) withPostSetHook { x => + MultiChoiceSetting("-Xlint", "warning", "Enable recommended additional warnings", choices = lintables, default = enableAll) withPostSetHook { x => propagate(x.value) // enabling the selections (on each append to value) xlint.value = true // only enables lint, not the group + for (b <- lintWarnings if isAnon(b) && !b.isSetByUser) b.value = true // init anonymous settings (but not if disabled) } xlint } diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index 2c31ef2e8e..efc121d479 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -5099,7 +5099,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper def isPlausible(m: Symbol) = m.alternatives exists (m => requiresNoArgs(m.info)) def maybeWarn(s: String): Unit = { - def warn(message: String) = context.warning(lit.pos, s"$message Did you forget the interpolator?") + def warn(message: String) = context.warning(lit.pos, s"possible missing interpolator: $message") def suspiciousSym(name: TermName) = context.lookupSymbol(name, _ => true).symbol def suspiciousExpr = InterpolatorCodeRegex findFirstIn s def suspiciousIdents = InterpolatorIdentRegex findAllIn s map (s => suspiciousSym(s drop 1)) @@ -5107,9 +5107,9 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper // heuristics - no warning on e.g. a string with only "$asInstanceOf" if (s contains ' ') ( if (suspiciousExpr.nonEmpty) - warn("That looks like an interpolated expression!") // "${...}" + warn("detected an interpolated expression") // "${...}" else - suspiciousIdents find isPlausible foreach (sym => warn(s"`$$${sym.name}` looks like an interpolated identifier!")) // "$id" + suspiciousIdents find isPlausible foreach (sym => warn(s"detected interpolated identifier `$$${sym.name}`")) // "$id" ) } lit match { @@ -5119,7 +5119,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper } def typedLiteral(tree: Literal) = { - if (settings.lint) warnMissingInterpolator(tree) + if (settings.warnMissingInterpolator) warnMissingInterpolator(tree) tree setType (if (tree.value.tag == UnitTag) UnitTpe else ConstantType(tree.value)) } diff --git a/test/files/neg/forgot-interpolator.check b/test/files/neg/forgot-interpolator.check index 8988458982..8e75350518 100644 --- a/test/files/neg/forgot-interpolator.check +++ b/test/files/neg/forgot-interpolator.check @@ -1,25 +1,25 @@ -forgot-interpolator.scala:4: warning: `$bippy` looks like an interpolated identifier! Did you forget the interpolator? +forgot-interpolator.scala:4: warning: possible missing interpolator: detected interpolated identifier `$bippy` def f = "Put the $bippy in the $bippy!" // warn 1 ^ -forgot-interpolator.scala:14: warning: That looks like an interpolated expression! Did you forget the interpolator? +forgot-interpolator.scala:14: warning: possible missing interpolator: detected an interpolated expression def f = """Put the ${println("bippy")} in the bippy!""" // warn 2 ^ -forgot-interpolator.scala:30: warning: `$beppo` looks like an interpolated identifier! Did you forget the interpolator? +forgot-interpolator.scala:30: warning: possible missing interpolator: detected interpolated identifier `$beppo` def f = "$beppo was a marx bros who saw dollars." // warn 3 ^ -forgot-interpolator.scala:34: warning: `$aleppo` looks like an interpolated identifier! Did you forget the interpolator? +forgot-interpolator.scala:34: warning: possible missing interpolator: detected interpolated identifier `$aleppo` def f = "$aleppo is a pepper and a city." // warn 4 ^ -forgot-interpolator.scala:47: warning: `$hippo` looks like an interpolated identifier! Did you forget the interpolator? +forgot-interpolator.scala:47: warning: possible missing interpolator: detected interpolated identifier `$hippo` def h = "$hippo takes an implicit" // warn 6 ^ -forgot-interpolator.scala:88: warning: `$groucho` looks like an interpolated identifier! Did you forget the interpolator? +forgot-interpolator.scala:88: warning: possible missing interpolator: detected interpolated identifier `$groucho` def f2 = "I salute $groucho" // warn 7 ^ -forgot-interpolator.scala:89: warning: `$dingo` looks like an interpolated identifier! Did you forget the interpolator? +forgot-interpolator.scala:89: warning: possible missing interpolator: detected interpolated identifier `$dingo` def f3 = "I even salute $dingo" // warn 8 ^ -forgot-interpolator.scala:90: warning: `$calico` looks like an interpolated identifier! Did you forget the interpolator? +forgot-interpolator.scala:90: warning: possible missing interpolator: detected interpolated identifier `$calico` def f4 = "I also salute $calico" // warn 9 ^ error: No warnings can be incurred under -Xfatal-warnings. diff --git a/test/files/neg/t7848-interp-warn.check b/test/files/neg/t7848-interp-warn.check index b7df6d8ce2..4cf9d55ffd 100644 --- a/test/files/neg/t7848-interp-warn.check +++ b/test/files/neg/t7848-interp-warn.check @@ -1,10 +1,10 @@ -t7848-interp-warn.scala:8: warning: `$foo` looks like an interpolated identifier! Did you forget the interpolator? +t7848-interp-warn.scala:8: warning: possible missing interpolator: detected interpolated identifier `$foo` "An important $foo message!" ^ -t7848-interp-warn.scala:12: warning: That looks like an interpolated expression! Did you forget the interpolator? +t7848-interp-warn.scala:12: warning: possible missing interpolator: detected an interpolated expression "A doubly important ${foo * 2} message!" ^ -t7848-interp-warn.scala:16: warning: `$bar` looks like an interpolated identifier! Did you forget the interpolator? +t7848-interp-warn.scala:16: warning: possible missing interpolator: detected interpolated identifier `$bar` def j = s"Try using '${ "something like $bar" }' instead." // warn ^ error: No warnings can be incurred under -Xfatal-warnings. diff --git a/test/files/neg/t8525.check b/test/files/neg/t8525.check new file mode 100644 index 0000000000..554ab23253 --- /dev/null +++ b/test/files/neg/t8525.check @@ -0,0 +1,6 @@ +t8525.scala:9: warning: private[this] value name in class X shadows mutable name inherited from class Named. Changes to name will not be visible within class X - you may want to give them distinct names. + override def toString = name // shadowing mutable var name + ^ +error: No warnings can be incurred under -Xfatal-warnings. +one warning found +one error found diff --git a/test/files/neg/t8525.flags b/test/files/neg/t8525.flags new file mode 100644 index 0000000000..f19af1f717 --- /dev/null +++ b/test/files/neg/t8525.flags @@ -0,0 +1 @@ +-Xfatal-warnings -Xlint:-warn-missing-interpolator diff --git a/test/files/neg/t8525.scala b/test/files/neg/t8525.scala new file mode 100644 index 0000000000..7bed04904f --- /dev/null +++ b/test/files/neg/t8525.scala @@ -0,0 +1,10 @@ + +class Named(var name: String) + +class X(name: String) extends Named(name) { + def x = "Hi, $name" // missing interp + def f(p: (Int, Int)): Int = p._1 * p._2 + def g = f(3, 4) // adapted + def u: Unit = () // unitarian universalist + override def toString = name // shadowing mutable var name +} diff --git a/test/files/neg/t8610-arg.check b/test/files/neg/t8610-arg.check index f2879b0d45..4f194ce84a 100644 --- a/test/files/neg/t8610-arg.check +++ b/test/files/neg/t8610-arg.check @@ -1,9 +1,12 @@ -t8610-arg.scala:3: warning: `$name` looks like an interpolated identifier! Did you forget the interpolator? +t8610-arg.scala:5: warning: possible missing interpolator: detected interpolated identifier `$name` def x = "Hi, $name" // missing interp ^ -t8610-arg.scala:6: warning: side-effecting nullary methods are discouraged: suggest defining as `def u()` instead +t8610-arg.scala:9: warning: private[this] value name in class X shadows mutable name inherited from class Named. Changes to name will not be visible within class X - you may want to give them distinct names. + override def toString = name // shadowing mutable var name + ^ +t8610-arg.scala:8: warning: side-effecting nullary methods are discouraged: suggest defining as `def u()` instead def u: Unit = () // unitarian universalist ^ error: No warnings can be incurred under -Xfatal-warnings. -two warnings found +three warnings found one error found diff --git a/test/files/neg/t8610-arg.scala b/test/files/neg/t8610-arg.scala index 494a3354da..7bed04904f 100644 --- a/test/files/neg/t8610-arg.scala +++ b/test/files/neg/t8610-arg.scala @@ -1,7 +1,10 @@ -case class X(name: String) { +class Named(var name: String) + +class X(name: String) extends Named(name) { def x = "Hi, $name" // missing interp def f(p: (Int, Int)): Int = p._1 * p._2 def g = f(3, 4) // adapted def u: Unit = () // unitarian universalist + override def toString = name // shadowing mutable var name } diff --git a/test/files/neg/t8610.check b/test/files/neg/t8610.check index 7a18e55127..334a947549 100644 --- a/test/files/neg/t8610.check +++ b/test/files/neg/t8610.check @@ -1,15 +1,18 @@ -t8610.scala:3: warning: `$name` looks like an interpolated identifier! Did you forget the interpolator? +t8610.scala:5: warning: possible missing interpolator: detected interpolated identifier `$name` def x = "Hi, $name" // missing interp ^ -t8610.scala:5: warning: Adapting argument list by creating a 2-tuple: this may not be what you want. +t8610.scala:7: warning: Adapting argument list by creating a 2-tuple: this may not be what you want. signature: X.f(p: (Int, Int)): Int given arguments: 3, 4 after adaptation: X.f((3, 4): (Int, Int)) def g = f(3, 4) // adapted ^ -t8610.scala:6: warning: side-effecting nullary methods are discouraged: suggest defining as `def u()` instead +t8610.scala:9: warning: private[this] value name in class X shadows mutable name inherited from class Named. Changes to name will not be visible within class X - you may want to give them distinct names. + override def toString = name // shadowing mutable var name + ^ +t8610.scala:8: warning: side-effecting nullary methods are discouraged: suggest defining as `def u()` instead def u: Unit = () // unitarian universalist ^ error: No warnings can be incurred under -Xfatal-warnings. -three warnings found +four warnings found one error found diff --git a/test/files/neg/t8610.scala b/test/files/neg/t8610.scala index 494a3354da..7bed04904f 100644 --- a/test/files/neg/t8610.scala +++ b/test/files/neg/t8610.scala @@ -1,7 +1,10 @@ -case class X(name: String) { +class Named(var name: String) + +class X(name: String) extends Named(name) { def x = "Hi, $name" // missing interp def f(p: (Int, Int)): Int = p._1 * p._2 def g = f(3, 4) // adapted def u: Unit = () // unitarian universalist + override def toString = name // shadowing mutable var name } diff --git a/test/files/run/t8610.check b/test/files/run/t8610.check index 077382fb1c..0e0dfc2cd3 100644 --- a/test/files/run/t8610.check +++ b/test/files/run/t8610.check @@ -1,4 +1,4 @@ -t8610.scala:4: warning: `$name` looks like an interpolated identifier! Did you forget the interpolator? +t8610.scala:4: warning: possible missing interpolator: detected interpolated identifier `$name` def x = "Hi, $name" // missing interp ^ t8610.scala:6: warning: Adapting argument list by creating a 2-tuple: this may not be what you want. diff --git a/test/junit/scala/tools/nsc/settings/SettingsTest.scala b/test/junit/scala/tools/nsc/settings/SettingsTest.scala index e4b5ecc7c3..29591727a3 100644 --- a/test/junit/scala/tools/nsc/settings/SettingsTest.scala +++ b/test/junit/scala/tools/nsc/settings/SettingsTest.scala @@ -26,7 +26,7 @@ class SettingsTest { assertThrows[IllegalArgumentException](check("-Ytest-setting:rubbish")) } - @Test def userSettingsHavePredecenceOverOptimize() { + @Test def userSettingsHavePrecedenceOverOptimize() { def check(args: String*): MutableSettings#BooleanSetting = { val s = new MutableSettings(msg => throw new IllegalArgumentException(msg)) val (ok, residual) = s.processArguments(args.toList, processAll = true) @@ -38,7 +38,7 @@ class SettingsTest { assertFalse(check("-Yinline:false", "-optimise").value) } - @Test def userSettingsHavePredecenceOverLint() { + @Test def userSettingsHavePrecedenceOverLint() { def check(args: String*): MutableSettings#BooleanSetting = { val s = new MutableSettings(msg => throw new IllegalArgumentException(msg)) val (ok, residual) = s.processArguments(args.toList, processAll = true) @@ -49,4 +49,18 @@ class SettingsTest { assertFalse(check("-Xlint", "-Ywarn-adapted-args:false").value) assertFalse(check("-Ywarn-adapted-args:false", "-Xlint").value) } + + def check(args: String*)(b: MutableSettings => MutableSettings#BooleanSetting): MutableSettings#BooleanSetting = { + val s = new MutableSettings(msg => throw new IllegalArgumentException(msg)) + val (ok, residual) = s.processArguments(args.toList, processAll = true) + assert(residual.isEmpty) + b(s) + } + @Test def anonymousLintersCanBeNamed() { + assertTrue(check("-Xlint")(_.warnMissingInterpolator)) // among Xlint + assertFalse(check("-Xlint:-warn-missing-interpolator")(_.warnMissingInterpolator)) + assertFalse(check("-Xlint:-warn-missing-interpolator", "-Xlint")(_.warnMissingInterpolator)) + // two lint settings are first come etc; unlike -Y + assertTrue(check("-Xlint", "-Xlint:-warn-missing-interpolator")(_.warnMissingInterpolator)) + } } -- cgit v1.2.3 From 70afd0544e86456515caee62a3eaac41882d892f Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Sat, 28 Jun 2014 19:12:48 +0200 Subject: relaxes attachment-matching rules It came as a surprise recently, but attachments.contains/get/update/remove require the class of the payload to match the provided tag exactly, not taking subclassing into account. This commit fixes the oversight. --- src/reflect/scala/reflect/macros/Attachments.scala | 2 +- test/files/pos/macro-attachments/Macros_1.scala | 19 +++++++++++++++++++ test/files/pos/macro-attachments/Test_2.scala | 3 +++ test/files/run/reflection-attachments.check | 0 4 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 test/files/pos/macro-attachments/Macros_1.scala create mode 100644 test/files/pos/macro-attachments/Test_2.scala create mode 100644 test/files/run/reflection-attachments.check diff --git a/src/reflect/scala/reflect/macros/Attachments.scala b/src/reflect/scala/reflect/macros/Attachments.scala index 5ccdc15a03..b5c340645a 100644 --- a/src/reflect/scala/reflect/macros/Attachments.scala +++ b/src/reflect/scala/reflect/macros/Attachments.scala @@ -35,7 +35,7 @@ abstract class Attachments { self => def all: Set[Any] = Set.empty private def matchesTag[T: ClassTag](datum: Any) = - classTag[T].runtimeClass == datum.getClass + classTag[T].runtimeClass.isInstance(datum) /** An underlying payload of the given class type `T`. */ def get[T: ClassTag]: Option[T] = diff --git a/test/files/pos/macro-attachments/Macros_1.scala b/test/files/pos/macro-attachments/Macros_1.scala new file mode 100644 index 0000000000..38d05d5b85 --- /dev/null +++ b/test/files/pos/macro-attachments/Macros_1.scala @@ -0,0 +1,19 @@ +import scala.language.experimental.macros +import scala.reflect.macros.whitebox.Context + +trait Base +class Att extends Base + +object Macros { + def impl(c: Context) = { + import c.universe._ + import c.internal._ + import decorators._ + val dummy = q"x" + dummy.updateAttachment(new Att) + if (dummy.attachments.get[Base].isEmpty) c.abort(c.enclosingPosition, "that's not good") + q"()" + } + + def foo: Any = macro impl +} \ No newline at end of file diff --git a/test/files/pos/macro-attachments/Test_2.scala b/test/files/pos/macro-attachments/Test_2.scala new file mode 100644 index 0000000000..acfddae942 --- /dev/null +++ b/test/files/pos/macro-attachments/Test_2.scala @@ -0,0 +1,3 @@ +object Test extends App { + Macros.foo +} \ No newline at end of file diff --git a/test/files/run/reflection-attachments.check b/test/files/run/reflection-attachments.check new file mode 100644 index 0000000000..e69de29bb2 -- cgit v1.2.3 From 5ba2be24f82631f10d9163513204c5e44f445555 Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Sat, 28 Jun 2014 18:46:54 +0200 Subject: prevents some reflection APIs from destroying rangeposes This commit continues the work started in fcb3932b32. As we've figured out the hard way, exposing internally maintained trees (e.g. macro application) to the user is dangerous, because they can mutate the trees in place using one of the public APIs, potentially corrupting our internal state. Therefore, at some point we started duplicating everything that comes from the user and goes back to the user. This was generally a good idea due to the reason described above, but there was a problem that we didn't foresee - the problem of corrupted positions. It turns out that Tree.duplicate focuses positions in the tree being processed, turning range positions into offset ones, and that makes it impossible for macro users to make use of precise position information. I also went through the calls to Tree.duplicate to see what can be done to them. In cases when corruptions could happen, I tried to replace duplicate with duplicateAndKeepPositions. Some notes: 1) Tree rehashing performed in TreeGen uses duplicates here and there (e.g. in mkTemplate or in mkFor), which means that if one deconstructs a macro argument and then constructs it back, some of the positions in synthetic trees might become inaccurate. That's a general problem with synthetic trees though, so I don't think it should be addressed here. 2) TypeTree.copyAttrs does duplication of originals, which means that even duplicateAndKeepPositions will adversely affect positions of certain publicly accessible parts of type trees. I'm really scared to change this though, because who knows who can use this invariant. 3) Some methods that can be reached from the public API (Tree.substituteXXX, c.reifyXXX, c.untypecheck, ...) do duplicate internally, but that shouldn't be a big problem for us, because nothing is irreversibly corrupted here. It's the user's choice to call those methods (unlike with TypeTree.copyAttrs) and, if necessary, they can fixup the positions themselves afterwards. 4) Macro engine internals (macro impl binding creation, exploratory typechecking in typedMacroBody) use duplicate, but these aren't supposed to be seen by the user, so this shouldn't be a problem. 5) Certain parser functions, member syntheses and typer desugarings also duplicate, but in those cases we aren't talking about taking user trees and screwing them up, but rather about emitting potentially imprecise positions in the first place. Hence this commit isn't the right place to address these potential issues. --- src/reflect/scala/reflect/internal/Internals.scala | 2 +- test/files/run/macro-rangepos-subpatterns.check | 1 + test/files/run/macro-rangepos-subpatterns.flags | 1 + .../run/macro-rangepos-subpatterns/Macros_1.scala | 18 ++++++++++++++++++ test/files/run/macro-rangepos-subpatterns/Test_2.scala | 5 +++++ 5 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 test/files/run/macro-rangepos-subpatterns.check create mode 100644 test/files/run/macro-rangepos-subpatterns.flags create mode 100644 test/files/run/macro-rangepos-subpatterns/Macros_1.scala create mode 100644 test/files/run/macro-rangepos-subpatterns/Test_2.scala diff --git a/src/reflect/scala/reflect/internal/Internals.scala b/src/reflect/scala/reflect/internal/Internals.scala index e9916cf7d1..26f3bfd9d0 100644 --- a/src/reflect/scala/reflect/internal/Internals.scala +++ b/src/reflect/scala/reflect/internal/Internals.scala @@ -129,7 +129,7 @@ trait Internals extends api.Internals { def typeBounds(lo: Type, hi: Type): TypeBounds = self.TypeBounds(lo, hi) def boundedWildcardType(bounds: TypeBounds): BoundedWildcardType = self.BoundedWildcardType(bounds) - def subpatterns(tree: Tree): Option[List[Tree]] = tree.attachments.get[SubpatternsAttachment].map(_.patterns.map(_.duplicate)) + def subpatterns(tree: Tree): Option[List[Tree]] = tree.attachments.get[SubpatternsAttachment].map(_.patterns.map(duplicateAndKeepPositions)) type Decorators = MacroDecoratorApi lazy val decorators: Decorators = new MacroDecoratorApi { diff --git a/test/files/run/macro-rangepos-subpatterns.check b/test/files/run/macro-rangepos-subpatterns.check new file mode 100644 index 0000000000..760e15d019 --- /dev/null +++ b/test/files/run/macro-rangepos-subpatterns.check @@ -0,0 +1 @@ +The width of the subpattern is: 2 diff --git a/test/files/run/macro-rangepos-subpatterns.flags b/test/files/run/macro-rangepos-subpatterns.flags new file mode 100644 index 0000000000..fcf951d907 --- /dev/null +++ b/test/files/run/macro-rangepos-subpatterns.flags @@ -0,0 +1 @@ +-Yrangepos \ No newline at end of file diff --git a/test/files/run/macro-rangepos-subpatterns/Macros_1.scala b/test/files/run/macro-rangepos-subpatterns/Macros_1.scala new file mode 100644 index 0000000000..0f30862347 --- /dev/null +++ b/test/files/run/macro-rangepos-subpatterns/Macros_1.scala @@ -0,0 +1,18 @@ +import scala.reflect.macros.whitebox.Context +import language.experimental.macros + +object Extractor { + def unapply(x: Any): Any = macro unapplyImpl + def unapplyImpl(c: Context)(x: c.Tree) = { + import c.universe._ + import internal._ + val pos = subpatterns(x).get.head.pos + q""" + new { + def isEmpty = false + def get = ${"The width of the subpattern is: " + (pos.end - pos.start + 1)} + def unapply(x: Any) = this + }.unapply($x) + """ + } +} diff --git a/test/files/run/macro-rangepos-subpatterns/Test_2.scala b/test/files/run/macro-rangepos-subpatterns/Test_2.scala new file mode 100644 index 0000000000..7b076e6632 --- /dev/null +++ b/test/files/run/macro-rangepos-subpatterns/Test_2.scala @@ -0,0 +1,5 @@ +object Test extends App { + 42 match { + case Extractor(a) => println(a) + } +} -- cgit v1.2.3 From 3b89c168b4926139f7295183fdc1903f6f553798 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Tue, 1 Jul 2014 08:21:07 -0700 Subject: SI-8525 No anonymous lint Turn anonymous references to `settings.lint` into named settings. After that, trust to Adriaan to make them filterable. There are a few remaining top-level -Y lint warnings that are deprecated. --- src/compiler/scala/tools/nsc/plugins/Plugins.scala | 2 +- .../scala/tools/nsc/settings/Warnings.scala | 97 ++++++++++++++++++---- .../tools/nsc/transform/patmat/MatchAnalysis.scala | 2 +- .../scala/tools/nsc/typechecker/Namers.scala | 2 +- .../scala/tools/nsc/typechecker/RefChecks.scala | 8 +- .../tools/nsc/typechecker/SuperAccessors.scala | 2 +- .../tools/nsc/typechecker/SyntheticMethods.scala | 11 ++- .../scala/tools/nsc/doc/ScaladocAnalyzer.scala | 2 +- test/files/neg/t4851.flags | 2 +- test/files/neg/t8525.check | 11 ++- test/files/neg/t8525.flags | 2 +- test/files/neg/t8610-arg.check | 8 +- test/files/neg/warn-inferred-any.flags | 2 +- test/files/run/t8610.check | 3 + test/files/run/t8610.flags | 2 +- .../scala/tools/nsc/settings/SettingsTest.scala | 6 +- 16 files changed, 122 insertions(+), 40 deletions(-) diff --git a/src/compiler/scala/tools/nsc/plugins/Plugins.scala b/src/compiler/scala/tools/nsc/plugins/Plugins.scala index 12f9aeba27..6e3d013e52 100644 --- a/src/compiler/scala/tools/nsc/plugins/Plugins.scala +++ b/src/compiler/scala/tools/nsc/plugins/Plugins.scala @@ -33,7 +33,7 @@ trait Plugins { global: Global => } val maybes = Plugin.loadAllFrom(paths, dirs, settings.disable.value) val (goods, errors) = maybes partition (_.isSuccess) - // Explicit parameterization of recover to suppress -Xlint warning about inferred Any + // Explicit parameterization of recover to avoid -Xlint warning about inferred Any errors foreach (_.recover[Any] { // legacy behavior ignores altogether, so at least warn devs case e: MissingPluginException => if (global.isDeveloper) warning(e.getMessage) diff --git a/src/compiler/scala/tools/nsc/settings/Warnings.scala b/src/compiler/scala/tools/nsc/settings/Warnings.scala index f1ed45a2a0..3ff2369f86 100644 --- a/src/compiler/scala/tools/nsc/settings/Warnings.scala +++ b/src/compiler/scala/tools/nsc/settings/Warnings.scala @@ -34,34 +34,76 @@ trait Warnings { warnInferAny, // warnUnused SI-7712, SI-7707 warnUnused not quite ready for prime-time // warnUnusedImport currently considered too noisy for general use - warnMissingInterpolator + // warnValueOverrides + warnMissingInterpolator, + warnDocDetached, + warnPrivateShadow, + warnPolyImplicitOverload, + warnOptionImplicit, + warnDelayedInit, + warnByNameRightAssociative, + warnPackageObjectClasses, + warnUnsoundMatch ) private lazy val warnSelectNullable = BooleanSetting("-Xcheck-null", "This option is obsolete and does nothing.") // Individual warnings. - val warnAdaptedArgs = BooleanSetting ("-Ywarn-adapted-args", "Warn if an argument list is modified to match the receiver.") - val warnDeadCode = BooleanSetting ("-Ywarn-dead-code", "Warn when dead code is identified.") - val warnValueDiscard = BooleanSetting ("-Ywarn-value-discard", "Warn when non-Unit expression results are unused.") - val warnNumericWiden = BooleanSetting ("-Ywarn-numeric-widen", "Warn when numerics are widened.") - val warnNullaryUnit = BooleanSetting ("-Ywarn-nullary-unit", "Warn when nullary methods return Unit.") - val warnInaccessible = BooleanSetting ("-Ywarn-inaccessible", "Warn about inaccessible types in method signatures.") - val warnNullaryOverride = BooleanSetting ("-Ywarn-nullary-override", "Warn when non-nullary overrides nullary, e.g. `def foo()` over `def foo`.") - val warnInferAny = BooleanSetting ("-Ywarn-infer-any", "Warn when a type argument is inferred to be `Any`.") - val warnUnused = BooleanSetting ("-Ywarn-unused", "Warn when local and private vals, vars, defs, and types are are unused") - val warnUnusedImport = BooleanSetting ("-Ywarn-unused-import", "Warn when imports are unused") + val warnDeadCode = BooleanSetting("-Ywarn-dead-code", + "Warn when dead code is identified.") + val warnValueDiscard = BooleanSetting("-Ywarn-value-discard", + "Warn when non-Unit expression results are unused.") + val warnNumericWiden = BooleanSetting("-Ywarn-numeric-widen", + "Warn when numerics are widened.") + val warnUnused = BooleanSetting("-Ywarn-unused", + "Warn when local and private vals, vars, defs, and types are are unused") + val warnUnusedImport = BooleanSetting("-Ywarn-unused-import", + "Warn when imports are unused") - // Lint warnings that are not -Y - val warnMissingInterpolator = new BooleanSetting("warn-missing-interpolator", "Warn when a string literal appears to be missing an interpolator id.") + // Lint warnings that are not -Y, created with new instead of autoregistering factory method + private def lintflag(name: String, text: String) = new BooleanSetting(name, text) + + val warnAdaptedArgs = lintflag("adapted-args", + "Warn if an argument list is modified to match the receiver.") + val warnNullaryUnit = lintflag("nullary-unit", + "Warn when nullary methods return Unit.") + val warnInaccessible = lintflag("inaccessible", + "Warn about inaccessible types in method signatures.") + val warnNullaryOverride = lintflag("nullary-override", + "Warn when non-nullary `def f()' overrides nullary `def f'.") + val warnInferAny = lintflag("infer-any", + "Warn when a type argument is inferred to be `Any`.") + val warnMissingInterpolator = lintflag("missing-interpolator", + "A string literal appears to be missing an interpolator id.") + val warnDocDetached = lintflag("doc-detached", + "A ScalaDoc comment appears to be detached from its element.") + val warnPrivateShadow = lintflag("private-shadow", + "A private field (or class parameter) shadows a superclass field.") + val warnPolyImplicitOverload = lintflag("poly-implicit-overload", + "Parameterized overloaded implicit methods are not visible as view bounds") + val warnOptionImplicit = lintflag("option-implicit", + "Option.apply used implicit view.") + val warnDelayedInit = lintflag("delayedinit-select", + "Selecting member of DelayedInit") + val warnByNameRightAssociative = lintflag("by-name-right-associative", + "By-name parameter of right associative operator") + val warnPackageObjectClasses = lintflag("package-object-classes", + "Class or object defined in package object") + val warnUnsoundMatch = lintflag("unsound-match", + "Pattern match may not be typesafe") + + // Lint warnings that are not enabled yet + val warnValueOverrides = lintflag("value-overrides", "Generated value class method overrides an implementation") // Warning groups. val lint = { // Boolean setting for testing if lint is on; not "added" to option processing - val xlint = new BooleanSetting("-Xlint", "Enable recommended additional warnings.") - def lintables = (lintWarnings map (_.name stripPrefix "-Y")).sorted + val xlint = lintflag("-Xlint", "Enable recommended additional warnings.") + val yprefix = "-Ywarn-" + def lintables = (lintWarnings map (_.name stripPrefix yprefix)).sorted def isAnon(b: BooleanSetting) = !(b.name startsWith "-") def setPolitely(b: BooleanSetting, v: Boolean) = if (!b.isSetByUser) b.value = v - def set(w: String, v: Boolean) = lintWarnings find (_.name.stripPrefix("-Y") == w) foreach (b => setPolitely(b, v)) + def set(w: String, v: Boolean) = lintWarnings find (s => (s.name stripPrefix yprefix) == w) foreach (b => setPolitely(b, v)) val Neg = "-" def propagate(ss: List[String]): Unit = ss match { case w :: rest => if (w startsWith Neg) set(w stripPrefix Neg, false) else set(w, true) ; propagate(rest) @@ -81,6 +123,29 @@ trait Warnings { xlint } + // Lint warnings that are currently -Y, but deprecated in that usage + // Alas, the -Yarg must have a doppelgaenger that is not deprecated + @deprecated("Use the Xlint flag", since="2.11.2") + val YwarnAdaptedArgs = BooleanSetting("-Ywarn-adapted-args", + "Warn if an argument list is modified to match the receiver.") withDeprecationMessage + "Enable -Xlint:adapted-args" enabling List(warnAdaptedArgs) + @deprecated("Use the Xlint flag", since="2.11.2") + val YwarnNullaryUnit = BooleanSetting("-Ywarn-nullary-unit", + "Warn when nullary methods return Unit.") withDeprecationMessage + "Enable -Xlint:nullary-unit" enabling List(warnNullaryUnit) + @deprecated("Use the Xlint flag", since="2.11.2") + val YwarnInaccessible = BooleanSetting("-Ywarn-inaccessible", + "Warn about inaccessible types in method signatures.") withDeprecationMessage + "Enable -Xlint:inaccessible" enabling List(warnInaccessible) + @deprecated("Use the Xlint flag", since="2.11.2") + val YwarnNullaryOverride = BooleanSetting("-Ywarn-nullary-override", + "Warn when non-nullary `def f()' overrides nullary `def f'.") withDeprecationMessage + "Enable -Xlint:nullary-override" enabling List(warnNullaryOverride) + @deprecated("Use the Xlint flag", since="2.11.2") + val YwarnInferAny = BooleanSetting("-Ywarn-infer-any", + "Warn when a type argument is inferred to be `Any`.") withDeprecationMessage + "Enable -Xlint:infer-any" enabling List(warnInferAny) + // Backward compatibility. @deprecated("Use fatalWarnings", "2.11.0") def Xwarnfatal = fatalWarnings // used by sbt @deprecated("This option is being removed", "2.11.0") def Xchecknull = warnSelectNullable // used by ide diff --git a/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala b/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala index 6f81cbe152..b2dc6e4e52 100644 --- a/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala +++ b/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala @@ -50,7 +50,7 @@ trait TreeAndTypeAnalysis extends Debugging { // but the annotation didn't bubble up... // This is a pretty poor approximation. def unsoundAssumptionUsed = binder.name != nme.WILDCARD && !(pt <:< pat.tpe) - if (settings.lint && unsoundAssumptionUsed) + if (settings.warnUnsoundMatch && unsoundAssumptionUsed) reporter.warning(pat.pos, sm"""The value matched by $pat is bound to ${binder.name}, which may be used under the |unsound assumption that it has type ${pat.tpe}, whereas we can only safely diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala index 1328119aac..7bbd81118a 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala @@ -717,7 +717,7 @@ trait Namers extends MethodSynthesis { m.updateAttachment(new ConstructorDefaultsAttachment(tree, null)) } val owner = tree.symbol.owner - if (settings.lint && owner.isPackageObjectClass && !mods.isImplicit) { + if (settings.warnPackageObjectClasses && owner.isPackageObjectClass && !mods.isImplicit) { reporter.warning(tree.pos, "it is not recommended to define classes/objects inside of package objects.\n" + "If possible, define " + tree.symbol + " in " + owner.skipPackageObject + " instead." diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala index 1b3da26bf2..47465875e9 100644 --- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala +++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala @@ -167,7 +167,7 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans } // This has become noisy with implicit classes. - if (settings.lint && settings.developer) { + if (settings.warnPolyImplicitOverload && settings.developer) { clazz.info.decls filter (x => x.isImplicit && x.typeParams.nonEmpty) foreach { sym => // implicit classes leave both a module symbol and a method symbol as residue val alts = clazz.info.decl(sym.name).alternatives filterNot (_.isModule) @@ -954,7 +954,7 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans def apply(tp: Type) = mapOver(tp).normalize } - def checkImplicitViewOptionApply(pos: Position, fn: Tree, args: List[Tree]): Unit = if (settings.lint) (fn, args) match { + def checkImplicitViewOptionApply(pos: Position, fn: Tree, args: List[Tree]): Unit = if (settings.warnOptionImplicit) (fn, args) match { case (tap@TypeApply(fun, targs), List(view: ApplyImplicitView)) if fun.symbol == currentRun.runDefinitions.Option_apply => reporter.warning(pos, s"Suspicious application of an implicit view (${view.fun}) in the argument to Option.apply.") // SI-6567 case _ => @@ -1320,7 +1320,7 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans && !qual.tpe.isInstanceOf[ThisType] && sym.accessedOrSelf.isVal ) - if (settings.lint.value && isLikelyUninitialized) + if (settings.warnDelayedInit && isLikelyUninitialized) reporter.warning(pos, s"Selecting ${sym} from ${sym.owner}, which extends scala.DelayedInit, is likely to yield an uninitialized value") } @@ -1387,7 +1387,7 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans private def checkByNameRightAssociativeDef(tree: DefDef) { tree match { case DefDef(_, name, _, params :: _, _, _) => - if (settings.lint && !treeInfo.isLeftAssoc(name.decodedName) && params.exists(p => isByName(p.symbol))) + if (settings.warnByNameRightAssociative && !treeInfo.isLeftAssoc(name.decodedName) && params.exists(p => isByName(p.symbol))) reporter.warning(tree.pos, "by-name parameters will be evaluated eagerly when called as a right-associative infix operator. For more details, see SI-1980.") case _ => diff --git a/src/compiler/scala/tools/nsc/typechecker/SuperAccessors.scala b/src/compiler/scala/tools/nsc/typechecker/SuperAccessors.scala index d3a41b9570..38b00a015b 100644 --- a/src/compiler/scala/tools/nsc/typechecker/SuperAccessors.scala +++ b/src/compiler/scala/tools/nsc/typechecker/SuperAccessors.scala @@ -247,7 +247,7 @@ abstract class SuperAccessors extends transform.Transform with transform.TypingT // also exists in a superclass, because they may be surprised // to find out that a constructor parameter will shadow a // field. See SI-4762. - if (settings.lint) { + if (settings.warnPrivateShadow) { if (sym.isPrivateLocal && sym.paramss.isEmpty) { qual.symbol.ancestors foreach { parent => parent.info.decls filterNot (x => x.isPrivate || x.isLocalToThis) foreach { m2 => diff --git a/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala b/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala index 9516f94135..d0237fb468 100644 --- a/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala +++ b/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala @@ -339,12 +339,11 @@ trait SyntheticMethods extends ast.TreeDSL { !hasOverridingImplementation(m) || { clazz.isDerivedValueClass && (m == Any_hashCode || m == Any_equals) && { // Without a means to suppress this warning, I've thought better of it. - // - // if (settings.lint) { - // (clazz.info nonPrivateMember m.name) filter (m => (m.owner != AnyClass) && (m.owner != clazz) && !m.isDeferred) andAlso { m => - // currentUnit.warning(clazz.pos, s"Implementation of ${m.name} inherited from ${m.owner} overridden in $clazz to enforce value class semantics") - // } - // } + if (settings.warnValueOverrides) { + (clazz.info nonPrivateMember m.name) filter (m => (m.owner != AnyClass) && (m.owner != clazz) && !m.isDeferred) andAlso { m => + currentUnit.warning(clazz.pos, s"Implementation of ${m.name} inherited from ${m.owner} overridden in $clazz to enforce value class semantics") + } + } true } } diff --git a/src/scaladoc/scala/tools/nsc/doc/ScaladocAnalyzer.scala b/src/scaladoc/scala/tools/nsc/doc/ScaladocAnalyzer.scala index 10c382e169..ccf18b76de 100644 --- a/src/scaladoc/scala/tools/nsc/doc/ScaladocAnalyzer.scala +++ b/src/scaladoc/scala/tools/nsc/doc/ScaladocAnalyzer.scala @@ -190,7 +190,7 @@ abstract class ScaladocSyntaxAnalyzer[G <: Global](val global: G) extends Syntax typeParams.nonEmpty || version.nonEmpty || since.nonEmpty } def isDirty = unclean(unmooredParser parseComment doc) - if ((doc ne null) && (settings.lint || isDirty)) + if ((doc ne null) && (settings.warnDocDetached || isDirty)) reporter.warning(doc.pos, "discarding unmoored doc comment") } diff --git a/test/files/neg/t4851.flags b/test/files/neg/t4851.flags index ca0d0a0ba3..044ce22c84 100644 --- a/test/files/neg/t4851.flags +++ b/test/files/neg/t4851.flags @@ -1 +1 @@ --Ywarn-adapted-args -Xfatal-warnings -deprecation +-Xlint:adapted-args -Xfatal-warnings -deprecation diff --git a/test/files/neg/t8525.check b/test/files/neg/t8525.check index 554ab23253..5287e43b7a 100644 --- a/test/files/neg/t8525.check +++ b/test/files/neg/t8525.check @@ -1,6 +1,15 @@ +t8525.scala:7: warning: Adapting argument list by creating a 2-tuple: this may not be what you want. + signature: X.f(p: (Int, Int)): Int + given arguments: 3, 4 + after adaptation: X.f((3, 4): (Int, Int)) + def g = f(3, 4) // adapted + ^ t8525.scala:9: warning: private[this] value name in class X shadows mutable name inherited from class Named. Changes to name will not be visible within class X - you may want to give them distinct names. override def toString = name // shadowing mutable var name ^ +t8525.scala:8: warning: side-effecting nullary methods are discouraged: suggest defining as `def u()` instead + def u: Unit = () // unitarian universalist + ^ error: No warnings can be incurred under -Xfatal-warnings. -one warning found +three warnings found one error found diff --git a/test/files/neg/t8525.flags b/test/files/neg/t8525.flags index f19af1f717..53b2dfe7ec 100644 --- a/test/files/neg/t8525.flags +++ b/test/files/neg/t8525.flags @@ -1 +1 @@ --Xfatal-warnings -Xlint:-warn-missing-interpolator +-Xfatal-warnings -Xlint:-missing-interpolator -Xlint diff --git a/test/files/neg/t8610-arg.check b/test/files/neg/t8610-arg.check index 4f194ce84a..ea2805508d 100644 --- a/test/files/neg/t8610-arg.check +++ b/test/files/neg/t8610-arg.check @@ -1,6 +1,12 @@ t8610-arg.scala:5: warning: possible missing interpolator: detected interpolated identifier `$name` def x = "Hi, $name" // missing interp ^ +t8610-arg.scala:7: warning: Adapting argument list by creating a 2-tuple: this may not be what you want. + signature: X.f(p: (Int, Int)): Int + given arguments: 3, 4 + after adaptation: X.f((3, 4): (Int, Int)) + def g = f(3, 4) // adapted + ^ t8610-arg.scala:9: warning: private[this] value name in class X shadows mutable name inherited from class Named. Changes to name will not be visible within class X - you may want to give them distinct names. override def toString = name // shadowing mutable var name ^ @@ -8,5 +14,5 @@ t8610-arg.scala:8: warning: side-effecting nullary methods are discouraged: sugg def u: Unit = () // unitarian universalist ^ error: No warnings can be incurred under -Xfatal-warnings. -three warnings found +four warnings found one error found diff --git a/test/files/neg/warn-inferred-any.flags b/test/files/neg/warn-inferred-any.flags index a3127d392a..b580dfbbe3 100644 --- a/test/files/neg/warn-inferred-any.flags +++ b/test/files/neg/warn-inferred-any.flags @@ -1 +1 @@ --Xfatal-warnings -Ywarn-infer-any +-Xfatal-warnings -Xlint:infer-any diff --git a/test/files/run/t8610.check b/test/files/run/t8610.check index 0e0dfc2cd3..fde82d5109 100644 --- a/test/files/run/t8610.check +++ b/test/files/run/t8610.check @@ -7,4 +7,7 @@ t8610.scala:6: warning: Adapting argument list by creating a 2-tuple: this may n after adaptation: X.f((3, 4): (Int, Int)) def g = f(3, 4) // adapted ^ +t8610.scala:7: warning: side-effecting nullary methods are discouraged: suggest defining as `def u()` instead + def u: Unit = () // unitarian universalist + ^ Hi, $name diff --git a/test/files/run/t8610.flags b/test/files/run/t8610.flags index edcb5f4d0c..4195dec383 100644 --- a/test/files/run/t8610.flags +++ b/test/files/run/t8610.flags @@ -1 +1 @@ --Xlint:warn-adapted-args +-Xlint:adapted-args diff --git a/test/junit/scala/tools/nsc/settings/SettingsTest.scala b/test/junit/scala/tools/nsc/settings/SettingsTest.scala index 29591727a3..9203054d9a 100644 --- a/test/junit/scala/tools/nsc/settings/SettingsTest.scala +++ b/test/junit/scala/tools/nsc/settings/SettingsTest.scala @@ -58,9 +58,9 @@ class SettingsTest { } @Test def anonymousLintersCanBeNamed() { assertTrue(check("-Xlint")(_.warnMissingInterpolator)) // among Xlint - assertFalse(check("-Xlint:-warn-missing-interpolator")(_.warnMissingInterpolator)) - assertFalse(check("-Xlint:-warn-missing-interpolator", "-Xlint")(_.warnMissingInterpolator)) + assertFalse(check("-Xlint:-missing-interpolator")(_.warnMissingInterpolator)) + assertFalse(check("-Xlint:-missing-interpolator", "-Xlint")(_.warnMissingInterpolator)) // two lint settings are first come etc; unlike -Y - assertTrue(check("-Xlint", "-Xlint:-warn-missing-interpolator")(_.warnMissingInterpolator)) + assertTrue(check("-Xlint", "-Xlint:-missing-interpolator")(_.warnMissingInterpolator)) } } -- cgit v1.2.3 From c39c693f81cdf4d86a6c13bee48fcbc4006fb3bc Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Thu, 10 Jul 2014 13:03:26 +0200 Subject: SI-8117 Fix bug when mixing well-positioned named and positional args The method `missingParams` which returns undefined parameters of a given invocation expression still assumed that named arguments can only appear after positional ones. --- .../tools/nsc/typechecker/NamesDefaults.scala | 41 ++++++++++++++++------ test/files/run/names-defaults.check | 1 + test/files/run/names-defaults.scala | 4 +++ 3 files changed, 35 insertions(+), 11 deletions(-) diff --git a/src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala b/src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala index 43902d1c65..b6387fd56b 100644 --- a/src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala +++ b/src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala @@ -379,18 +379,37 @@ trait NamesDefaults { self: Analyzer => def makeNamedTypes(syms: List[Symbol]) = syms map (sym => NamedType(sym.name, sym.tpe)) - def missingParams[T](args: List[T], params: List[Symbol], argName: T => Option[Name] = nameOfNamedArg _): (List[Symbol], Boolean) = { - val namedArgs = args.dropWhile(arg => { - val n = argName(arg) - n.isEmpty || params.forall(p => p.name != n.get) - }) - val namedParams = params.drop(args.length - namedArgs.length) - // missing: keep those with a name which doesn't exist in namedArgs - val missingParams = namedParams.filter(p => namedArgs.forall(arg => { + /** + * Returns the parameter symbols of an invocation expression that are not defined by the list + * of arguments. + * + * @param args The list of arguments + * @param params The list of parameter sybols of the invoked method + * @param argName A function that extracts the name of an argument expression, if it is a named argument. + */ + def missingParams[T](args: List[T], params: List[Symbol], argName: T => Option[Name]): (List[Symbol], Boolean) = { + // The argument list contains first a mix of positional args and named args that are on the + // right parameter position, and then a number or named args on different positions. + + // collect all named arguments whose position does not match the parameter they define + val namedArgsOnChangedPosition = args.zip(params) dropWhile { + case (arg, param) => + val n = argName(arg) + // drop the argument if + // - it's not named, or + // - it's named, but defines the parameter on its current position, or + // - it's named, but none of the parameter names matches (treated as a positional argument, an assignment expression) + n.isEmpty || n.get == param.name || params.forall(_.name != n.get) + } map (_._1) + + val paramsWithoutPositionalArg = params.drop(args.length - namedArgsOnChangedPosition.length) + + // missing parameters: those with a name which is not specified in one of the namedArgsOnChangedPosition + val missingParams = paramsWithoutPositionalArg.filter(p => namedArgsOnChangedPosition.forall { arg => val n = argName(arg) n.isEmpty || n.get != p.name - })) - val allPositional = missingParams.length == namedParams.length + }) + val allPositional = missingParams.length == paramsWithoutPositionalArg.length (missingParams, allPositional) } @@ -407,7 +426,7 @@ trait NamesDefaults { self: Analyzer => previousArgss: List[List[Tree]], params: List[Symbol], pos: scala.reflect.internal.util.Position, context: Context): (List[Tree], List[Symbol]) = { if (givenArgs.length < params.length) { - val (missing, positional) = missingParams(givenArgs, params) + val (missing, positional) = missingParams(givenArgs, params, nameOfNamedArg) if (missing forall (_.hasDefault)) { val defaultArgs = missing flatMap (p => { val defGetter = defaultGetter(p, context) diff --git a/test/files/run/names-defaults.check b/test/files/run/names-defaults.check index 25999c488a..c358dc5849 100644 --- a/test/files/run/names-defaults.check +++ b/test/files/run/names-defaults.check @@ -124,3 +124,4 @@ List(1, 2) 3 3 (1,0), (1,2) +1 1 0 diff --git a/test/files/run/names-defaults.scala b/test/files/run/names-defaults.scala index 05cd4a540c..b7ed490cbc 100644 --- a/test/files/run/names-defaults.scala +++ b/test/files/run/names-defaults.scala @@ -401,6 +401,10 @@ object Test extends App { C4441a().copy() C4441b()().copy()() + // SI-8117 + def f8177(a: Int = 0, b: Int = 0, c: Int = 0) = s"$a $b $c" + println(f8177(a = 1, 1)) + // DEFINITIONS def test1(a: Int, b: String) = println(a +": "+ b) def test2(u: Int, v: Int)(k: String, l: Int) = println(l +": "+ k +", "+ (u + v)) -- cgit v1.2.3 From 26b77aca9498775968cc9138c7537dbfbc52e3fd Mon Sep 17 00:00:00 2001 From: Philipp Haller Date: Fri, 11 Jul 2014 17:02:41 +0200 Subject: SI-8590 Expand doc comments for ExecutionContext - link to Java API docs for ForkJoinPool-based default implementation - add example for creating an execution context from a `java.util.concurrent.ExecutorService` - add tags for parameters and return values - expand doc comment for `prepare` --- .../scala/concurrent/ExecutionContext.scala | 72 ++++++++++++++++++---- 1 file changed, 60 insertions(+), 12 deletions(-) diff --git a/src/library/scala/concurrent/ExecutionContext.scala b/src/library/scala/concurrent/ExecutionContext.scala index 4674c9174b..11d3bb8b02 100644 --- a/src/library/scala/concurrent/ExecutionContext.scala +++ b/src/library/scala/concurrent/ExecutionContext.scala @@ -61,28 +61,44 @@ or import scala.concurrent.ExecutionContext.Implicits.global.""") trait ExecutionContext { /** Runs a block of code on this execution context. + * + * @param runnable the task to execute */ def execute(runnable: Runnable): Unit /** Reports that an asynchronous computation failed. + * + * @param cause the cause of the failure */ def reportFailure(@deprecatedName('t) cause: Throwable): Unit - /** Prepares for the execution of a task. Returns the prepared - * execution context. A valid implementation of `prepare` is one - * that simply returns `this`. + /** Prepares for the execution of a task. Returns the prepared execution context. + * + * `prepare` should be called at the site where an `ExecutionContext` is received (for + * example, through an implicit method parameter). The returned execution context may + * then be used to execute tasks. The role of `prepare` is to save any context relevant + * to an execution's ''call site'', so that this context may be restored at the + * ''execution site''. (These are often different: for example, execution may be + * suspended through a `Promise`'s future until the `Promise` is completed, which may + * be done in another thread, on another stack.) + * + * Note: a valid implementation of `prepare` is one that simply returns `this`. + * + * @return the prepared execution context */ def prepare(): ExecutionContext = this } /** - * An [[ExecutionContext]] that is also a Java [[Executor]]. + * An [[ExecutionContext]] that is also a + * Java [[http://docs.oracle.com/javase/8/docs/api/java/util/concurrent/Executor.html Executor]]. */ trait ExecutionContextExecutor extends ExecutionContext with Executor /** - * An [[ExecutionContext]] that is also a Java [[ExecutorService]]. + * An [[ExecutionContext]] that is also a + * Java [[http://docs.oracle.com/javase/8/docs/api/java/util/concurrent/ExecutorService.html ExecutorService]]. */ trait ExecutionContextExecutorService extends ExecutionContextExecutor with ExecutorService @@ -91,38 +107,70 @@ trait ExecutionContextExecutorService extends ExecutionContextExecutor with Exec */ object ExecutionContext { /** - * This is the explicit global ExecutionContext, - * call this when you want to provide the global ExecutionContext explicitly + * The explicit global `ExecutionContext`. Invoke `global` when you want to provide the global + * `ExecutionContext` explicitly. + * + * The default `ExecutionContext` implementation is backed by a port of + * [[http://gee.cs.oswego.edu/dl/jsr166/dist/jsr166-4jdk7docs/java/util/concurrent/ForkJoinPool.html java.util.concurrent.ForkJoinPool]]. + * + * @return the global `ExecutionContext` */ def global: ExecutionContextExecutor = Implicits.global object Implicits { /** - * This is the implicit global ExecutionContext, - * import this when you want to provide the global ExecutionContext implicitly + * The implicit global `ExecutionContext`. Import `global` when you want to provide the global + * `ExecutionContext` implicitly. + * + * The default `ExecutionContext` implementation is backed by a port of + * [[http://gee.cs.oswego.edu/dl/jsr166/dist/jsr166-4jdk7docs/java/util/concurrent/ForkJoinPool.html java.util.concurrent.ForkJoinPool]]. */ implicit lazy val global: ExecutionContextExecutor = impl.ExecutionContextImpl.fromExecutor(null: Executor) } /** Creates an `ExecutionContext` from the given `ExecutorService`. + * + * @param e the `ExecutorService` to use + * @param reporter a function for error reporting + * @return the `ExecutionContext` using the given `ExecutorService` */ def fromExecutorService(e: ExecutorService, reporter: Throwable => Unit): ExecutionContextExecutorService = impl.ExecutionContextImpl.fromExecutorService(e, reporter) - /** Creates an `ExecutionContext` from the given `ExecutorService` with the default Reporter. + /** Creates an `ExecutionContext` from the given `ExecutorService` with the [[scala.concurrent.ExecutionContext$.defaultReporter default reporter]]. + * + * If it is guaranteed that none of the executed tasks are blocking, a single-threaded `ExecutorService` + * can be used to create an `ExecutionContext` as follows: + * + * {{{ + * import java.util.concurrent.Executors + * val ec = ExecutionContext.fromExecutorService(Executors.newSingleThreadExecutor()) + * }}} + * + * @param e the `ExecutorService` to use + * @return the `ExecutionContext` using the given `ExecutorService` */ def fromExecutorService(e: ExecutorService): ExecutionContextExecutorService = fromExecutorService(e, defaultReporter) /** Creates an `ExecutionContext` from the given `Executor`. + * + * @param e the `Executor` to use + * @param reporter a function for error reporting + * @return the `ExecutionContext` using the given `Executor` */ def fromExecutor(e: Executor, reporter: Throwable => Unit): ExecutionContextExecutor = impl.ExecutionContextImpl.fromExecutor(e, reporter) - /** Creates an `ExecutionContext` from the given `Executor` with the default Reporter. + /** Creates an `ExecutionContext` from the given `Executor` with the [[scala.concurrent.ExecutionContext$.defaultReporter default reporter]]. + * + * @param e the `Executor` to use + * @return the `ExecutionContext` using the given `Executor` */ def fromExecutor(e: Executor): ExecutionContextExecutor = fromExecutor(e, defaultReporter) - /** The default reporter simply prints the stack trace of the `Throwable` to System.err. + /** The default reporter simply prints the stack trace of the `Throwable` to [[http://docs.oracle.com/javase/8/docs/api/java/lang/System.html#err System.err]]. + * + * @return the function for error reporting */ def defaultReporter: Throwable => Unit = _.printStackTrace() } -- cgit v1.2.3 From 9d527b6558f9ecf32467b6f414175267bb44c1d7 Mon Sep 17 00:00:00 2001 From: Denys Shabalin Date: Mon, 14 Jul 2014 15:22:59 +0200 Subject: Address pull request feedback --- src/compiler/scala/tools/reflect/quasiquotes/Parsers.scala | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/compiler/scala/tools/reflect/quasiquotes/Parsers.scala b/src/compiler/scala/tools/reflect/quasiquotes/Parsers.scala index 460d02c218..392b7fc881 100644 --- a/src/compiler/scala/tools/reflect/quasiquotes/Parsers.scala +++ b/src/compiler/scala/tools/reflect/quasiquotes/Parsers.scala @@ -72,8 +72,10 @@ trait Parsers { self: Quasiquotes => override def makeBlock(stats: List[Tree]): Tree = method match { case nme.apply => stats match { + // we don't want to eagerly flatten trees with placeholders as they + // might have to be wrapped into a block depending on their value case (head @ Ident(name)) :: Nil if isHole(name) => Block(Nil, head) - case _ => super.makeBlock(stats) + case _ => gen.mkBlock(stats, doFlatten = true) } case nme.unapply => gen.mkBlock(stats, doFlatten = false) case other => global.abort("unreachable") -- cgit v1.2.3 From b0dd01ebb4b0cc41a6629357522217bfe14821fb Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Mon, 14 Jul 2014 07:32:03 -0700 Subject: SI-8608 f interpolator emits constant strings When invoking `format` is obviated by a lack of formatting fields, then just degenerate to an unenhanced constant string. This means it doesn't cost anything to use f"$$ordinary" in place of "$ordinary", which may cause warnings under -Xlint. Note that certain format literals, in particular for line separator %n, are not actually literals and can't be replaced at compile time. --- src/compiler/scala/tools/reflect/FormatInterpolator.scala | 6 ++++-- test/files/run/stringinterpolation_macro-run.check | 4 ++++ test/files/run/stringinterpolation_macro-run.scala | 1 + test/files/run/t8608-no-format.scala | 15 +++++++++++++++ 4 files changed, 24 insertions(+), 2 deletions(-) create mode 100644 test/files/run/t8608-no-format.scala diff --git a/src/compiler/scala/tools/reflect/FormatInterpolator.scala b/src/compiler/scala/tools/reflect/FormatInterpolator.scala index 57be1afdfe..4fa06be60f 100644 --- a/src/compiler/scala/tools/reflect/FormatInterpolator.scala +++ b/src/compiler/scala/tools/reflect/FormatInterpolator.scala @@ -183,11 +183,13 @@ abstract class FormatInterpolator { } //q"{..$evals; ${fstring.toString}.format(..$ids)}" - locally { + val format = fstring.toString + if (ids.isEmpty && !format.contains("%")) Literal(Constant(format)) + else { val expr = Apply( Select( - Literal(Constant(fstring.toString)), + Literal(Constant(format)), newTermName("format")), ids.toList ) diff --git a/test/files/run/stringinterpolation_macro-run.check b/test/files/run/stringinterpolation_macro-run.check index ead61e76ac..c7f46bac87 100644 --- a/test/files/run/stringinterpolation_macro-run.check +++ b/test/files/run/stringinterpolation_macro-run.check @@ -63,5 +63,9 @@ She is 4 feet tall. 05/26/12 05/26/12 % + mind +------ +matter + 7 7 9 7 9 9 diff --git a/test/files/run/stringinterpolation_macro-run.scala b/test/files/run/stringinterpolation_macro-run.scala index a6def98540..e18375d521 100644 --- a/test/files/run/stringinterpolation_macro-run.scala +++ b/test/files/run/stringinterpolation_macro-run.scala @@ -115,6 +115,7 @@ println(f"""${"1234"}%TD""") // literals and arg indexes println(f"%%") +println(f" mind%n------%nmatter%n") println(f"${7}%d % Date: Mon, 14 Jul 2014 08:02:41 -0700 Subject: SI-8608 f-interpolator inlines StringOps Instead of "hi".format(), emit new _root_.s.c.i.StringOps("hi").format(), to clarify intent and avoid picking up some other implicit enhancement. A further optimization would be to use String.format directly when that is possible. The ticket says it is not possible for ``` f"${BigDecimal(3.4)}%e" ``` --- src/compiler/scala/tools/reflect/FormatInterpolator.scala | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/compiler/scala/tools/reflect/FormatInterpolator.scala b/src/compiler/scala/tools/reflect/FormatInterpolator.scala index 4fa06be60f..b445f1e2bb 100644 --- a/src/compiler/scala/tools/reflect/FormatInterpolator.scala +++ b/src/compiler/scala/tools/reflect/FormatInterpolator.scala @@ -182,15 +182,23 @@ abstract class FormatInterpolator { case (part, n) => copyPart(part, n) } - //q"{..$evals; ${fstring.toString}.format(..$ids)}" + //q"{..$evals; new StringOps(${fstring.toString}).format(..$ids)}" val format = fstring.toString if (ids.isEmpty && !format.contains("%")) Literal(Constant(format)) else { + val scalaPackage = Select(Ident(nme.ROOTPKG), TermName("scala")) + val newStringOps = Select( + New(Select(Select(Select(scalaPackage, + TermName("collection")), TermName("immutable")), TypeName("StringOps"))), + termNames.CONSTRUCTOR + ) val expr = Apply( Select( - Literal(Constant(format)), - newTermName("format")), + Apply( + newStringOps, + List(Literal(Constant(format)))), + TermName("format")), ids.toList ) val p = c.macroApplication.pos -- cgit v1.2.3 From adba40519bc68defb89bf668d814d16447614d9b Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Thu, 10 Jul 2014 15:11:04 +0200 Subject: Fix -Yno-adapted-args, it would just warn, not err --- .../scala/tools/nsc/typechecker/Adaptations.scala | 3 ++- .../scala/tools/nsc/typechecker/Typers.scala | 20 ++++++++++---------- test/files/neg/t8035-no-adapted-args.check | 21 +++++++++++++++++++++ test/files/neg/t8035-no-adapted-args.flags | 1 + test/files/neg/t8035-no-adapted-args.scala | 6 ++++++ 5 files changed, 40 insertions(+), 11 deletions(-) create mode 100644 test/files/neg/t8035-no-adapted-args.check create mode 100644 test/files/neg/t8035-no-adapted-args.flags create mode 100644 test/files/neg/t8035-no-adapted-args.scala diff --git a/src/compiler/scala/tools/nsc/typechecker/Adaptations.scala b/src/compiler/scala/tools/nsc/typechecker/Adaptations.scala index 37c39c07be..2f4d228347 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Adaptations.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Adaptations.scala @@ -82,7 +82,8 @@ trait Adaptations { } else if (settings.warnAdaptedArgs) context.warning(t.pos, adaptWarningMessage(s"Adapting argument list by creating a ${args.size}-tuple: this may not be what you want.")) - !settings.noAdaptedArgs || !(args.isEmpty && settings.future) + // return `true` if the adaptation should be kept + !(settings.noAdaptedArgs || (args.isEmpty && settings.future)) } } } diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index 2c31ef2e8e..65f11a360e 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -3248,25 +3248,25 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper * to that. This is the last thing which is tried (after * default arguments) */ - def tryTupleApply: Tree = ( + def tryTupleApply: Tree = { if (eligibleForTupleConversion(paramTypes, argslen) && !phase.erasedTypes) { val tupleArgs = List(atPos(tree.pos.makeTransparent)(gen.mkTuple(args))) // expected one argument, but got 0 or >1 ==> try applying to tuple // the inner "doTypedApply" does "extractUndetparams" => restore when it fails val savedUndetparams = context.undetparams silent(_.doTypedApply(tree, fun, tupleArgs, mode, pt)) map { t => - // Depending on user options, may warn or error here if - // a Unit or tuple was inserted. - val keepTree = ( - !mode.typingExprNotFun - || t.symbol == null - || checkValidAdaptation(t, args) - ) - if (keepTree) t else EmptyTree + // Depending on user options, may warn or error here if + // a Unit or tuple was inserted. + val keepTree = ( + !mode.typingExprNotFun // why? introduced in 4e488a60, doc welcome + || t.symbol == null // ditto + || checkValidAdaptation(t, args) + ) + if (keepTree) t else EmptyTree } orElse { _ => context.undetparams = savedUndetparams ; EmptyTree } } else EmptyTree - ) + } /* Treats an application which uses named or default arguments. * Also works if names + a vararg used: when names are used, the vararg diff --git a/test/files/neg/t8035-no-adapted-args.check b/test/files/neg/t8035-no-adapted-args.check new file mode 100644 index 0000000000..43637b2c1f --- /dev/null +++ b/test/files/neg/t8035-no-adapted-args.check @@ -0,0 +1,21 @@ +t8035-no-adapted-args.scala:4: warning: No automatic adaptation here: use explicit parentheses. + signature: Test.f[T](x: T): Int + given arguments: 1, 2, 3 + after adaptation: Test.f((1, 2, 3): (Int, Int, Int)) + f(1, 2, 3) + ^ +t8035-no-adapted-args.scala:4: error: too many arguments for method f: (x: (Int, Int, Int))Int + f(1, 2, 3) + ^ +t8035-no-adapted-args.scala:5: warning: No automatic adaptation here: use explicit parentheses. + signature: Test.f[T](x: T): Int + given arguments: + after adaptation: Test.f((): Unit) + f() + ^ +t8035-no-adapted-args.scala:5: error: not enough arguments for method f: (x: Unit)Int. +Unspecified value parameter x. + f() + ^ +two warnings found +two errors found diff --git a/test/files/neg/t8035-no-adapted-args.flags b/test/files/neg/t8035-no-adapted-args.flags new file mode 100644 index 0000000000..b3e8c505e2 --- /dev/null +++ b/test/files/neg/t8035-no-adapted-args.flags @@ -0,0 +1 @@ +-Yno-adapted-args \ No newline at end of file diff --git a/test/files/neg/t8035-no-adapted-args.scala b/test/files/neg/t8035-no-adapted-args.scala new file mode 100644 index 0000000000..82690ebe94 --- /dev/null +++ b/test/files/neg/t8035-no-adapted-args.scala @@ -0,0 +1,6 @@ +object Test { + def f[T](x: T) = 0 + + f(1, 2, 3) + f() +} -- cgit v1.2.3 From 997647eb0dcf1d4028b50f1e30cbef7ead88784a Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Tue, 15 Jul 2014 11:45:07 +0200 Subject: Bump jline version to 2.12 (Re: SI-8535) Move version info where it belongs: versions.properties --- build.xml | 2 -- versions.properties | 1 + 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/build.xml b/build.xml index c407c8d797..a89e694ac2 100755 --- a/build.xml +++ b/build.xml @@ -187,8 +187,6 @@ TODO: - - diff --git a/versions.properties b/versions.properties index 1212734d22..c334629d20 100644 --- a/versions.properties +++ b/versions.properties @@ -24,6 +24,7 @@ scala-continuations-library.version.number=1.0.2 scala-swing.version.number=1.0.1 akka-actor.version.number=2.3.3 actors-migration.version.number=1.1.0 +jline.version=2.12 # external modules, used internally (not shipped) partest.version.number=1.0.0 -- cgit v1.2.3 From 5762110d0dc2cb492e34d5595c473aa0f9ca786a Mon Sep 17 00:00:00 2001 From: Philipp Haller Date: Wed, 9 Jul 2014 14:43:12 +0200 Subject: SI-5919 TypeTags and Exprs should be serializable - Make TypeCreator and TreeCreator extend Serializable. - When replacing a SerializedTypeTag with a TypeTag or WeakTypeTag, do not use scala.reflect.runtime.universe.rootMirror, since it is unlikely to find user classes; instead, create a runtime mirror using the context ClassLoader of the current thread. Use the same logic for SerializedExpr. - Remove writeObject/readObject methods from SerializedTypeTag and SerializedExpr since they are unused. - Add @throws annotation on writeReplace and readResolve methods. - Handle SecurityException if the current thread cannot access the context ClassLoader. - To make type tags of primitive value classes serializable, make PredefTypeCreator a top-level class. Otherwise, it would retain a reference to the enclosing Universe, rendering the TypeCreator non-serializable. Binary compatibility: - Keep nested PredefTypeCreator class to avoid backward binary incompatible change. - Keep `var` modifiers on the class parameters of SerializedTypeTag for backward binary compatibility. - Adds filter rules to forward binary compatibility whitelist: - `TypeCreator`, `PredefTypeCreator`, and `TreeCreator` must now extend from `Serializable`. - Must have new class `scala.reflect.api.PredefTypeCreator` to avoid problematic outer reference. --- bincompat-forward.whitelist.conf | 17 +++++++++++ src/reflect/scala/reflect/api/Exprs.scala | 22 +++++++------- src/reflect/scala/reflect/api/TreeCreator.scala | 6 ++-- src/reflect/scala/reflect/api/TypeCreator.scala | 2 +- src/reflect/scala/reflect/api/TypeTags.scala | 40 +++++++++++++++++-------- test/files/run/abstypetags_serialize.check | 4 +-- test/files/run/exprs_serialize.check | 21 +++++++++++-- test/files/run/exprs_serialize.scala | 12 +++++++- test/files/run/typetags_serialize.check | 5 ++-- test/files/run/typetags_serialize.scala | 5 ++++ 10 files changed, 99 insertions(+), 35 deletions(-) diff --git a/bincompat-forward.whitelist.conf b/bincompat-forward.whitelist.conf index 30dac79974..0b90cf4c8b 100644 --- a/bincompat-forward.whitelist.conf +++ b/bincompat-forward.whitelist.conf @@ -254,6 +254,23 @@ filter { { matchName="scala.reflect.runtime.JavaUniverse.PerRunReporting" problemName=MissingMethodProblem + }, + // see SI-5919 + { + matchName="scala.reflect.api.TypeTags$PredefTypeCreator" + problemName=MissingTypesProblem + }, + { + matchName="scala.reflect.api.TreeCreator" + problemName=MissingTypesProblem + }, + { + matchName="scala.reflect.api.TypeCreator" + problemName=MissingTypesProblem + }, + { + matchName="scala.reflect.api.PredefTypeCreator" + problemName=MissingClassProblem } ] } diff --git a/src/reflect/scala/reflect/api/Exprs.scala b/src/reflect/scala/reflect/api/Exprs.scala index 5b6ff2325c..6d401b5a79 100644 --- a/src/reflect/scala/reflect/api/Exprs.scala +++ b/src/reflect/scala/reflect/api/Exprs.scala @@ -9,6 +9,7 @@ package api import scala.reflect.runtime.{universe => ru} import scala.annotation.compileTimeOnly +import java.io.ObjectStreamException /** * EXPERIMENTAL @@ -157,23 +158,22 @@ trait Exprs { self: Universe => |if you want to get a value of the underlying expression, add scala-compiler.jar to the classpath, |import `scala.tools.reflect.Eval` and call `.eval` instead.""".trim.stripMargin) + @throws(classOf[ObjectStreamException]) private def writeReplace(): AnyRef = new SerializedExpr(treec, implicitly[WeakTypeTag[T]].in(ru.rootMirror)) } } private[scala] class SerializedExpr(var treec: TreeCreator, var tag: ru.WeakTypeTag[_]) extends Serializable { - private def writeObject(out: java.io.ObjectOutputStream): Unit = { - out.writeObject(treec) - out.writeObject(tag) - } - - private def readObject(in: java.io.ObjectInputStream): Unit = { - treec = in.readObject().asInstanceOf[TreeCreator] - tag = in.readObject().asInstanceOf[ru.WeakTypeTag[_]] - } + import scala.reflect.runtime.universe.{Expr, runtimeMirror} + @throws(classOf[ObjectStreamException]) private def readResolve(): AnyRef = { - import ru._ - Expr(rootMirror, treec)(tag) + val loader: ClassLoader = try { + Thread.currentThread().getContextClassLoader() + } catch { + case se: SecurityException => null + } + val m = runtimeMirror(loader) + Expr(m, treec)(tag.in(m)) } } diff --git a/src/reflect/scala/reflect/api/TreeCreator.scala b/src/reflect/scala/reflect/api/TreeCreator.scala index 027c840955..000eaa1aa6 100644 --- a/src/reflect/scala/reflect/api/TreeCreator.scala +++ b/src/reflect/scala/reflect/api/TreeCreator.scala @@ -2,12 +2,12 @@ package scala package reflect package api -/** This is an internal implementation class. +/** A mirror-aware factory for trees. * * This class is used internally by Scala Reflection, and is not recommended for use in client code. * - * @group ReflectionAPI + * @group ReflectionAPI */ -abstract class TreeCreator { +abstract class TreeCreator extends Serializable { def apply[U <: Universe with Singleton](m: scala.reflect.api.Mirror[U]): U # Tree } diff --git a/src/reflect/scala/reflect/api/TypeCreator.scala b/src/reflect/scala/reflect/api/TypeCreator.scala index 37fff90b43..cbd55b9428 100644 --- a/src/reflect/scala/reflect/api/TypeCreator.scala +++ b/src/reflect/scala/reflect/api/TypeCreator.scala @@ -8,6 +8,6 @@ package api * * @group ReflectionAPI */ -abstract class TypeCreator { +abstract class TypeCreator extends Serializable { def apply[U <: Universe with Singleton](m: scala.reflect.api.Mirror[U]): U # Type } diff --git a/src/reflect/scala/reflect/api/TypeTags.scala b/src/reflect/scala/reflect/api/TypeTags.scala index 1dfc84be69..1d53453bde 100644 --- a/src/reflect/scala/reflect/api/TypeTags.scala +++ b/src/reflect/scala/reflect/api/TypeTags.scala @@ -9,6 +9,7 @@ package api import java.lang.{ Class => jClass } import scala.language.implicitConversions +import java.io.ObjectStreamException /** * A `TypeTag[T]` encapsulates the runtime type representation of some type `T`. @@ -233,6 +234,7 @@ trait TypeTags { self: Universe => val otherMirror1 = otherMirror.asInstanceOf[scala.reflect.api.Mirror[otherMirror.universe.type]] otherMirror.universe.WeakTypeTag[T](otherMirror1, tpec) } + @throws(classOf[ObjectStreamException]) private def writeReplace(): AnyRef = new SerializedTypeTag(tpec, concrete = false) } @@ -293,10 +295,13 @@ trait TypeTags { self: Universe => val otherMirror1 = otherMirror.asInstanceOf[scala.reflect.api.Mirror[otherMirror.universe.type]] otherMirror.universe.TypeTag[T](otherMirror1, tpec) } + @throws(classOf[ObjectStreamException]) private def writeReplace(): AnyRef = new SerializedTypeTag(tpec, concrete = true) } /* @group TypeTags */ + // This class only exists to silence MIMA complaining about a binary incompatibility. + // Only the top-level class (api.PredefTypeCreator) should be used. private class PredefTypeCreator[T](copyIn: Universe => Universe#TypeTag[T]) extends TypeCreator { def apply[U <: Universe with Singleton](m: scala.reflect.api.Mirror[U]): U # Type = { copyIn(m.universe).asInstanceOf[U # TypeTag[T]].tpe @@ -304,8 +309,9 @@ trait TypeTags { self: Universe => } /* @group TypeTags */ - private class PredefTypeTag[T](_tpe: Type, copyIn: Universe => Universe#TypeTag[T]) extends TypeTagImpl[T](rootMirror, new PredefTypeCreator(copyIn)) { + private class PredefTypeTag[T](_tpe: Type, copyIn: Universe => Universe#TypeTag[T]) extends TypeTagImpl[T](rootMirror, new api.PredefTypeCreator(copyIn)) { override lazy val tpe: Type = _tpe + @throws(classOf[ObjectStreamException]) private def writeReplace(): AnyRef = new SerializedTypeTag(tpec, concrete = true) } @@ -341,20 +347,28 @@ trait TypeTags { self: Universe => def symbolOf[T: WeakTypeTag]: TypeSymbol } +// This class should be final, but we can't do that in Scala 2.11.x without breaking +// binary incompatibility. +// Since instances of this class are serialized, this class should have a +// SerialVersionUID annotation. private[scala] class SerializedTypeTag(var tpec: TypeCreator, var concrete: Boolean) extends Serializable { - private def writeObject(out: java.io.ObjectOutputStream): Unit = { - out.writeObject(tpec) - out.writeBoolean(concrete) - } - - private def readObject(in: java.io.ObjectInputStream): Unit = { - tpec = in.readObject().asInstanceOf[TypeCreator] - concrete = in.readBoolean() + import scala.reflect.runtime.universe.{TypeTag, WeakTypeTag, runtimeMirror} + @throws(classOf[ObjectStreamException]) + private def readResolve(): AnyRef = { + val loader: ClassLoader = try { + Thread.currentThread().getContextClassLoader() + } catch { + case se: SecurityException => null + } + val m = runtimeMirror(loader) + if (concrete) TypeTag(m, tpec) + else WeakTypeTag(m, tpec) } +} - private def readResolve(): AnyRef = { - import scala.reflect.runtime.universe._ - if (concrete) TypeTag(rootMirror, tpec) - else WeakTypeTag(rootMirror, tpec) +/* @group TypeTags */ +private class PredefTypeCreator[T](copyIn: Universe => Universe#TypeTag[T]) extends TypeCreator { + def apply[U <: Universe with Singleton](m: scala.reflect.api.Mirror[U]): U # Type = { + copyIn(m.universe).asInstanceOf[U # TypeTag[T]].tpe } } diff --git a/test/files/run/abstypetags_serialize.check b/test/files/run/abstypetags_serialize.check index bddc4523e6..1b5e2ebddf 100644 --- a/test/files/run/abstypetags_serialize.check +++ b/test/files/run/abstypetags_serialize.check @@ -1,2 +1,2 @@ -java.io.NotSerializableException: Test$$typecreator1$1 -java.io.NotSerializableException: Test$$typecreator2$1 +WeakTypeTag[T] +WeakTypeTag[U[String]] diff --git a/test/files/run/exprs_serialize.check b/test/files/run/exprs_serialize.check index 20ad6c110c..551823ccdc 100644 --- a/test/files/run/exprs_serialize.check +++ b/test/files/run/exprs_serialize.check @@ -1,2 +1,19 @@ -java.io.NotSerializableException: Test$$treecreator1$1 -java.io.NotSerializableException: Test$$treecreator2$1 +Expr[Int(2)](2) +Expr[java.lang.String]({ + def foo = "hello"; + foo.$plus("world!") +}) +Expr[Boolean]({ + def foo(x: Int) = { + class Local extends AnyRef { + def () = { + super.(); + () + }; + val f = 2 + }; + val obj = new Local(); + x.$percent(obj.f).$eq$eq(0) + }; + foo(5) +}) diff --git a/test/files/run/exprs_serialize.scala b/test/files/run/exprs_serialize.scala index c4310b0fe1..91027803b4 100644 --- a/test/files/run/exprs_serialize.scala +++ b/test/files/run/exprs_serialize.scala @@ -26,4 +26,14 @@ object Test extends App { test(reify(2)) test(reify{def foo = "hello"; foo + "world!"}) -} \ No newline at end of file + test(reify { + def foo(x: Int) = { + class Local { + val f = 2 + } + val obj = new Local + x % obj.f == 0 + } + foo(5) + }) +} diff --git a/test/files/run/typetags_serialize.check b/test/files/run/typetags_serialize.check index f79436ea5d..22928a2e94 100644 --- a/test/files/run/typetags_serialize.check +++ b/test/files/run/typetags_serialize.check @@ -1,2 +1,3 @@ -java.io.NotSerializableException: scala.reflect.api.TypeTags$PredefTypeCreator -java.io.NotSerializableException: Test$$typecreator1$1 +TypeTag[Int] +TypeTag[String] +TypeTag[Test.C[Double]] diff --git a/test/files/run/typetags_serialize.scala b/test/files/run/typetags_serialize.scala index 3c842e6cc9..a7a7845232 100644 --- a/test/files/run/typetags_serialize.scala +++ b/test/files/run/typetags_serialize.scala @@ -4,6 +4,10 @@ import scala.reflect.runtime.{universe => ru} import scala.reflect.runtime.{currentMirror => cm} object Test extends App { + class C[A] { + def m(a: A): Int = 5 + } + def test(tag: TypeTag[_]) = try { val fout = new ByteArrayOutputStream() @@ -26,4 +30,5 @@ object Test extends App { test(implicitly[TypeTag[Int]]) test(implicitly[TypeTag[String]]) + test(implicitly[TypeTag[C[Double]]]) } \ No newline at end of file -- cgit v1.2.3 From f15a289ec5c192ced30970bd9ab23cbf38ead92e Mon Sep 17 00:00:00 2001 From: Philipp Haller Date: Mon, 14 Jul 2014 19:12:47 +0200 Subject: Add SerialVersionUID to SerializedTypeTag and SerializedExpr The reason for adding the SerialVersionUID annotations is to be able to provide serialization stability throughout the 2.11.x series. And since type tags (and exprs) have not been serializable before, this does not break serialization for existing code. --- src/reflect/scala/reflect/api/Exprs.scala | 1 + src/reflect/scala/reflect/api/TypeTags.scala | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/reflect/scala/reflect/api/Exprs.scala b/src/reflect/scala/reflect/api/Exprs.scala index 6d401b5a79..3230fdbc67 100644 --- a/src/reflect/scala/reflect/api/Exprs.scala +++ b/src/reflect/scala/reflect/api/Exprs.scala @@ -163,6 +163,7 @@ trait Exprs { self: Universe => } } +@SerialVersionUID(1L) private[scala] class SerializedExpr(var treec: TreeCreator, var tag: ru.WeakTypeTag[_]) extends Serializable { import scala.reflect.runtime.universe.{Expr, runtimeMirror} diff --git a/src/reflect/scala/reflect/api/TypeTags.scala b/src/reflect/scala/reflect/api/TypeTags.scala index 1d53453bde..7db375ca61 100644 --- a/src/reflect/scala/reflect/api/TypeTags.scala +++ b/src/reflect/scala/reflect/api/TypeTags.scala @@ -349,8 +349,7 @@ trait TypeTags { self: Universe => // This class should be final, but we can't do that in Scala 2.11.x without breaking // binary incompatibility. -// Since instances of this class are serialized, this class should have a -// SerialVersionUID annotation. +@SerialVersionUID(1L) private[scala] class SerializedTypeTag(var tpec: TypeCreator, var concrete: Boolean) extends Serializable { import scala.reflect.runtime.universe.{TypeTag, WeakTypeTag, runtimeMirror} @throws(classOf[ObjectStreamException]) -- cgit v1.2.3 From ec46a1fba7395714e65e0f6644b7ad429c985ec7 Mon Sep 17 00:00:00 2001 From: Antoine Gourlay Date: Tue, 15 Jul 2014 19:05:52 +0200 Subject: SI-8557 make scaladoc normalize paths of external jars. Scaladoc compares (string representations of) the paths from -doc-external-doc and the paths form `sym.underlyingSource`. We now normalize on both ends before comparing them. --- src/scaladoc/scala/tools/nsc/doc/Settings.scala | 2 +- .../scala/tools/nsc/doc/model/MemberLookup.scala | 2 +- test/scaladoc/run/t8557.check | 1 + test/scaladoc/run/t8557.scala | 32 ++++++++++++++++++++++ 4 files changed, 35 insertions(+), 2 deletions(-) create mode 100644 test/scaladoc/run/t8557.check create mode 100644 test/scaladoc/run/t8557.scala diff --git a/src/scaladoc/scala/tools/nsc/doc/Settings.scala b/src/scaladoc/scala/tools/nsc/doc/Settings.scala index 67529f4178..a8e1dee4a0 100644 --- a/src/scaladoc/scala/tools/nsc/doc/Settings.scala +++ b/src/scaladoc/scala/tools/nsc/doc/Settings.scala @@ -249,7 +249,7 @@ class Settings(error: String => Unit, val printMsg: String => Unit = println(_)) val idx = s.indexOf("#") if (idx > 0) { val (first, last) = s.splitAt(idx) - Some(new File(first).getAbsolutePath -> appendIndex(last.substring(1))) + Some(new File(first).getCanonicalPath -> appendIndex(last.substring(1))) } else { error(s"Illegal -doc-external-doc option; expected a pair with '#' separator, found: '$s'") None diff --git a/src/scaladoc/scala/tools/nsc/doc/model/MemberLookup.scala b/src/scaladoc/scala/tools/nsc/doc/model/MemberLookup.scala index 339129bdbc..64eb1adbea 100644 --- a/src/scaladoc/scala/tools/nsc/doc/model/MemberLookup.scala +++ b/src/scaladoc/scala/tools/nsc/doc/model/MemberLookup.scala @@ -45,7 +45,7 @@ trait MemberLookup extends base.MemberLookupBase { sym.info.member(newTermName("package")) else sym Option(sym1.associatedFile) flatMap (_.underlyingSource) flatMap { src => - val path = src.path + val path = src.canonicalPath settings.extUrlMapping get path map { url => LinkToExternal(name, url + "#" + name) } diff --git a/test/scaladoc/run/t8557.check b/test/scaladoc/run/t8557.check new file mode 100644 index 0000000000..619c56180b --- /dev/null +++ b/test/scaladoc/run/t8557.check @@ -0,0 +1 @@ +Done. diff --git a/test/scaladoc/run/t8557.scala b/test/scaladoc/run/t8557.scala new file mode 100644 index 0000000000..451f004d7d --- /dev/null +++ b/test/scaladoc/run/t8557.scala @@ -0,0 +1,32 @@ +import scala.tools.nsc.doc.base._ +import scala.tools.nsc.doc.model._ +import scala.tools.partest.ScaladocModelTest + +object Test extends ScaladocModelTest { + + override def code = """ + package scala.test.scaladoc.T8857 + + /** + * A link: + * + * [[scala.Option$ object Option]]. + */ + class A + """ + + // a non-canonical path to scala-library.jar should still work + // this is a bit fragile (depends on the current directory being the root of the repo ; + // ant & partest seem to do that properly) + def scaladocSettings = "-doc-external-doc build/pack/bin/../lib/scala-library.jar#http://www.scala-lang.org/api/current/" + + def testModel(rootPackage: Package) = { + // get the quick access implicit defs in scope (_package(s), _class(es), _trait(s), object(s) _method(s), _value(s)) + import access._ + + val a = rootPackage._package("scala")._package("test")._package("scaladoc")._package("T8857")._class("A") + + val links = countLinks(a.comment.get, _.link.isInstanceOf[LinkToExternal]) + assert(links == 1, links + " == 1 (the links to external in class A)") + } +} -- cgit v1.2.3 From f81ec8d1f6481ddacfb27e743c6c58961e765f0e Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Wed, 16 Jul 2014 11:01:31 -0700 Subject: SI-8525 Clarify usage of -Xlint:_,flag Also clarify usage of -Xlint flag. Align more with javac -Xlint:all,-flag,flag where once a flag is explicitly enabled it cannot be disabled, but where the wildcard is a backstop only. (There is no all option yet here.) -Xlint and -Xlint:_ just set a flag which is consulted by any unset lint warning. Xlint warnings consult the state of Xlint when they are unset. Individual -Ywarn-ings do not. Other warnings are explicitly set to false. They can only be enabled programmatically. Some tests are corrected. Also, option order is no longer significant, see the unit test. --- .../scala/tools/nsc/settings/MutableSettings.scala | 9 +- .../scala/tools/nsc/settings/Warnings.scala | 120 +++++++++++---------- test/files/neg/t8610-arg.check | 14 +-- test/files/neg/t8610-arg.flags | 2 +- test/files/run/t8610.check | 6 -- .../scala/tools/nsc/settings/SettingsTest.scala | 24 ++--- 6 files changed, 77 insertions(+), 98 deletions(-) diff --git a/src/compiler/scala/tools/nsc/settings/MutableSettings.scala b/src/compiler/scala/tools/nsc/settings/MutableSettings.scala index dc49e8b822..0dd4ae0b3b 100644 --- a/src/compiler/scala/tools/nsc/settings/MutableSettings.scala +++ b/src/compiler/scala/tools/nsc/settings/MutableSettings.scala @@ -567,13 +567,12 @@ class MutableSettings(val errorFn: String => Unit) def badChoice(s: String, n: String) = errorFn(s"'$s' is not a valid choice for '$name'") def choosing = choices.nonEmpty def isChoice(s: String) = (s == "_") || (choices contains (s stripPrefix "-")) - def wildcards = choices // filter (!_.isSetByUser) override protected def tts(args: List[String], halting: Boolean) = { - val total = collection.mutable.ListBuffer.empty[String] ++ value + val added = collection.mutable.ListBuffer.empty[String] def tryArg(arg: String) = arg match { - case "_" if choosing => wildcards foreach (total += _) - case s if !choosing || isChoice(s) => total += s + case "_" if choosing => default() + case s if !choosing || isChoice(s) => added += s case s => badChoice(s, name) } def stoppingAt(arg: String) = (arg startsWith "-") || (choosing && !isChoice(arg)) @@ -584,7 +583,7 @@ class MutableSettings(val errorFn: String => Unit) } val rest = loop(args) if (rest.size == args.size) default() // if no arg consumed, trigger default action - else value = total.toList // update once + else value = added.toList // update all new settings at once Some(rest) } } diff --git a/src/compiler/scala/tools/nsc/settings/Warnings.scala b/src/compiler/scala/tools/nsc/settings/Warnings.scala index 3ff2369f86..0b9ad80041 100644 --- a/src/compiler/scala/tools/nsc/settings/Warnings.scala +++ b/src/compiler/scala/tools/nsc/settings/Warnings.scala @@ -17,12 +17,15 @@ trait Warnings { // Warning semantics. val fatalWarnings = BooleanSetting("-Xfatal-warnings", "Fail the compilation if there are any warnings.") - // These warnings are all so noisy as to be useless in their + // These additional warnings are all so noisy as to be useless in their // present form, but have the potential to offer useful info. protected def allWarnings = lintWarnings ++ List( warnDeadCode, warnValueDiscard, - warnNumericWiden + warnNumericWiden, + warnUnused, // SI-7712, SI-7707 warnUnused not quite ready for prime-time + warnUnusedImport, // currently considered too noisy for general use + warnValueOverrides // currently turned off as experimental ) // These warnings should be pretty quiet unless you're doing // something inadvisable. @@ -32,9 +35,6 @@ trait Warnings { warnNullaryUnit, warnAdaptedArgs, warnInferAny, - // warnUnused SI-7712, SI-7707 warnUnused not quite ready for prime-time - // warnUnusedImport currently considered too noisy for general use - // warnValueOverrides warnMissingInterpolator, warnDocDetached, warnPrivateShadow, @@ -46,22 +46,26 @@ trait Warnings { warnUnsoundMatch ) - private lazy val warnSelectNullable = BooleanSetting("-Xcheck-null", "This option is obsolete and does nothing.") + // Individual warnings. They can be set with -Ywarn. + private def nonlintflag(name: String, text: String): BooleanSetting = BooleanSetting(name, text) - // Individual warnings. - val warnDeadCode = BooleanSetting("-Ywarn-dead-code", + val warnDeadCode = nonlintflag("-Ywarn-dead-code", "Warn when dead code is identified.") - val warnValueDiscard = BooleanSetting("-Ywarn-value-discard", + val warnValueDiscard = nonlintflag("-Ywarn-value-discard", "Warn when non-Unit expression results are unused.") - val warnNumericWiden = BooleanSetting("-Ywarn-numeric-widen", + val warnNumericWiden = nonlintflag("-Ywarn-numeric-widen", "Warn when numerics are widened.") - val warnUnused = BooleanSetting("-Ywarn-unused", + val warnUnused = nonlintflag("-Ywarn-unused", "Warn when local and private vals, vars, defs, and types are are unused") - val warnUnusedImport = BooleanSetting("-Ywarn-unused-import", + val warnUnusedImport = nonlintflag("-Ywarn-unused-import", "Warn when imports are unused") - // Lint warnings that are not -Y, created with new instead of autoregistering factory method - private def lintflag(name: String, text: String) = new BooleanSetting(name, text) + // Lint warnings that have no -Y avatar, created with new instead of the autoregistering factory method. + // They evaluate true if set to true or else are unset but -Xlint is true + private def lintflag(name: String, text: String): BooleanSetting = + new BooleanSetting(name, text) { + override def value = if (isSetByUser) super.value else xlint + } val warnAdaptedArgs = lintflag("adapted-args", "Warn if an argument list is modified to match the receiver.") @@ -92,59 +96,59 @@ trait Warnings { val warnUnsoundMatch = lintflag("unsound-match", "Pattern match may not be typesafe") - // Lint warnings that are not enabled yet - val warnValueOverrides = lintflag("value-overrides", "Generated value class method overrides an implementation") + // Experimental lint warnings that are turned off, but which could be turned on programmatically. + // These warnings are said to blind those who dare enable them. + // They are not activated by -Xlint and can't be enabled on the command line. + val warnValueOverrides = { + val flag = lintflag("value-overrides", "Generated value class method overrides an implementation") + flag.value = false + flag + } - // Warning groups. - val lint = { - // Boolean setting for testing if lint is on; not "added" to option processing - val xlint = lintflag("-Xlint", "Enable recommended additional warnings.") - val yprefix = "-Ywarn-" - def lintables = (lintWarnings map (_.name stripPrefix yprefix)).sorted - def isAnon(b: BooleanSetting) = !(b.name startsWith "-") - def setPolitely(b: BooleanSetting, v: Boolean) = if (!b.isSetByUser) b.value = v - def set(w: String, v: Boolean) = lintWarnings find (s => (s.name stripPrefix yprefix) == w) foreach (b => setPolitely(b, v)) - val Neg = "-" - def propagate(ss: List[String]): Unit = ss match { - case w :: rest => if (w startsWith Neg) set(w stripPrefix Neg, false) else set(w, true) ; propagate(rest) - case Nil => () - } - // enable lint and the group, honoring previous -Y settings - def enableAll(): Unit = { - xlint.value = true - for (s <- lintWarnings) setPolitely(s, true) + // The Xlint warning group. + private val xlint = new BooleanSetting("-Zunused", "True if -Xlint or -Xlint:_") + // On -Xlint or -Xlint:_, set xlint, otherwise set the lint warning unless already set true + val lint = + MultiChoiceSetting( + name = "-Xlint", + helpArg = "warning", + descr = "Enable recommended additional warnings", + choices = (lintWarnings map (_.name)).sorted, + default = () => xlint.value = true + ) withPostSetHook { x => + val Neg = "-" + def setPolitely(b: BooleanSetting, v: Boolean) = if (!b.isSetByUser || !b) b.value = v + def set(w: String, v: Boolean) = lintWarnings find (_.name == w) foreach (setPolitely(_, v)) + def propagate(ss: List[String]): Unit = ss match { + case w :: rest => if (w startsWith Neg) set(w stripPrefix Neg, false) else set(w, true) ; propagate(rest) + case Nil => () + } + propagate(x.value) } - // The command option - MultiChoiceSetting("-Xlint", "warning", "Enable recommended additional warnings", choices = lintables, default = enableAll) withPostSetHook { x => - propagate(x.value) // enabling the selections (on each append to value) - xlint.value = true // only enables lint, not the group - for (b <- lintWarnings if isAnon(b) && !b.isSetByUser) b.value = true // init anonymous settings (but not if disabled) - } - xlint - } // Lint warnings that are currently -Y, but deprecated in that usage - // Alas, the -Yarg must have a doppelgaenger that is not deprecated - @deprecated("Use the Xlint flag", since="2.11.2") + @deprecated("Use warnAdaptedArgs", since="2.11.2") val YwarnAdaptedArgs = BooleanSetting("-Ywarn-adapted-args", - "Warn if an argument list is modified to match the receiver.") withDeprecationMessage - "Enable -Xlint:adapted-args" enabling List(warnAdaptedArgs) - @deprecated("Use the Xlint flag", since="2.11.2") + "Warn if an argument list is modified to match the receiver.") enabling List(warnAdaptedArgs) + //withDeprecationMessage "Enable -Xlint:adapted-args" + @deprecated("Use warnNullaryUnit", since="2.11.2") val YwarnNullaryUnit = BooleanSetting("-Ywarn-nullary-unit", - "Warn when nullary methods return Unit.") withDeprecationMessage - "Enable -Xlint:nullary-unit" enabling List(warnNullaryUnit) - @deprecated("Use the Xlint flag", since="2.11.2") + "Warn when nullary methods return Unit.") enabling List(warnNullaryUnit) + //withDeprecationMessage "Enable -Xlint:nullary-unit" + @deprecated("Use warnInaccessible", since="2.11.2") val YwarnInaccessible = BooleanSetting("-Ywarn-inaccessible", - "Warn about inaccessible types in method signatures.") withDeprecationMessage - "Enable -Xlint:inaccessible" enabling List(warnInaccessible) - @deprecated("Use the Xlint flag", since="2.11.2") + "Warn about inaccessible types in method signatures.") enabling List(warnInaccessible) + //withDeprecationMessage "Enable -Xlint:inaccessible" + @deprecated("Use warnNullaryOverride", since="2.11.2") val YwarnNullaryOverride = BooleanSetting("-Ywarn-nullary-override", - "Warn when non-nullary `def f()' overrides nullary `def f'.") withDeprecationMessage - "Enable -Xlint:nullary-override" enabling List(warnNullaryOverride) - @deprecated("Use the Xlint flag", since="2.11.2") + "Warn when non-nullary `def f()' overrides nullary `def f'.") enabling List(warnNullaryOverride) + //withDeprecationMessage "Enable -Xlint:nullary-override" + @deprecated("Use warnInferAny", since="2.11.2") val YwarnInferAny = BooleanSetting("-Ywarn-infer-any", - "Warn when a type argument is inferred to be `Any`.") withDeprecationMessage - "Enable -Xlint:infer-any" enabling List(warnInferAny) + "Warn when a type argument is inferred to be `Any`.") enabling List(warnInferAny) + //withDeprecationMessage "Enable -Xlint:infer-any" + + private lazy val warnSelectNullable = BooleanSetting("-Xcheck-null", "This option is obsolete and does nothing.") // Backward compatibility. @deprecated("Use fatalWarnings", "2.11.0") def Xwarnfatal = fatalWarnings // used by sbt diff --git a/test/files/neg/t8610-arg.check b/test/files/neg/t8610-arg.check index ea2805508d..d6fe207119 100644 --- a/test/files/neg/t8610-arg.check +++ b/test/files/neg/t8610-arg.check @@ -1,18 +1,6 @@ -t8610-arg.scala:5: warning: possible missing interpolator: detected interpolated identifier `$name` - def x = "Hi, $name" // missing interp - ^ -t8610-arg.scala:7: warning: Adapting argument list by creating a 2-tuple: this may not be what you want. - signature: X.f(p: (Int, Int)): Int - given arguments: 3, 4 - after adaptation: X.f((3, 4): (Int, Int)) - def g = f(3, 4) // adapted - ^ -t8610-arg.scala:9: warning: private[this] value name in class X shadows mutable name inherited from class Named. Changes to name will not be visible within class X - you may want to give them distinct names. - override def toString = name // shadowing mutable var name - ^ t8610-arg.scala:8: warning: side-effecting nullary methods are discouraged: suggest defining as `def u()` instead def u: Unit = () // unitarian universalist ^ error: No warnings can be incurred under -Xfatal-warnings. -four warnings found +one warning found one error found diff --git a/test/files/neg/t8610-arg.flags b/test/files/neg/t8610-arg.flags index f8867a7b4e..f331ba9383 100644 --- a/test/files/neg/t8610-arg.flags +++ b/test/files/neg/t8610-arg.flags @@ -1 +1 @@ --Xfatal-warnings -Xlint warn-nullary-unit +-Xfatal-warnings -Xlint nullary-unit diff --git a/test/files/run/t8610.check b/test/files/run/t8610.check index fde82d5109..b3ab7a9cef 100644 --- a/test/files/run/t8610.check +++ b/test/files/run/t8610.check @@ -1,13 +1,7 @@ -t8610.scala:4: warning: possible missing interpolator: detected interpolated identifier `$name` - def x = "Hi, $name" // missing interp - ^ t8610.scala:6: warning: Adapting argument list by creating a 2-tuple: this may not be what you want. signature: X.f(p: (Int, Int)): Int given arguments: 3, 4 after adaptation: X.f((3, 4): (Int, Int)) def g = f(3, 4) // adapted ^ -t8610.scala:7: warning: side-effecting nullary methods are discouraged: suggest defining as `def u()` instead - def u: Unit = () // unitarian universalist - ^ Hi, $name diff --git a/test/junit/scala/tools/nsc/settings/SettingsTest.scala b/test/junit/scala/tools/nsc/settings/SettingsTest.scala index 9203054d9a..960d7f8ac1 100644 --- a/test/junit/scala/tools/nsc/settings/SettingsTest.scala +++ b/test/junit/scala/tools/nsc/settings/SettingsTest.scala @@ -38,29 +38,23 @@ class SettingsTest { assertFalse(check("-Yinline:false", "-optimise").value) } - @Test def userSettingsHavePrecedenceOverLint() { - def check(args: String*): MutableSettings#BooleanSetting = { - val s = new MutableSettings(msg => throw new IllegalArgumentException(msg)) - val (ok, residual) = s.processArguments(args.toList, processAll = true) - assert(residual.isEmpty) - s.warnAdaptedArgs // among Xlint - } - assertTrue(check("-Xlint").value) - assertFalse(check("-Xlint", "-Ywarn-adapted-args:false").value) - assertFalse(check("-Ywarn-adapted-args:false", "-Xlint").value) - } - - def check(args: String*)(b: MutableSettings => MutableSettings#BooleanSetting): MutableSettings#BooleanSetting = { + // for the given args, select the desired setting + private def check(args: String*)(b: MutableSettings => MutableSettings#BooleanSetting): MutableSettings#BooleanSetting = { val s = new MutableSettings(msg => throw new IllegalArgumentException(msg)) val (ok, residual) = s.processArguments(args.toList, processAll = true) assert(residual.isEmpty) b(s) } + @Test def userSettingsHavePrecedenceOverLint() { + assertTrue(check("-Xlint")(_.warnAdaptedArgs)) + assertFalse(check("-Xlint", "-Ywarn-adapted-args:false")(_.warnAdaptedArgs)) + assertFalse(check("-Ywarn-adapted-args:false", "-Xlint")(_.warnAdaptedArgs)) + } + @Test def anonymousLintersCanBeNamed() { assertTrue(check("-Xlint")(_.warnMissingInterpolator)) // among Xlint assertFalse(check("-Xlint:-missing-interpolator")(_.warnMissingInterpolator)) assertFalse(check("-Xlint:-missing-interpolator", "-Xlint")(_.warnMissingInterpolator)) - // two lint settings are first come etc; unlike -Y - assertTrue(check("-Xlint", "-Xlint:-missing-interpolator")(_.warnMissingInterpolator)) + assertFalse(check("-Xlint", "-Xlint:-missing-interpolator")(_.warnMissingInterpolator)) } } -- cgit v1.2.3 From 41507ba71b7a8a5fbd5dffb4378a963ecd08be2a Mon Sep 17 00:00:00 2001 From: mpociecha Date: Tue, 15 Jul 2014 14:38:50 +0200 Subject: Remove invalidation from Global.scala The invalidation has been introduced in these commits: https://github.com/scala/scala/commit/167309afd10f9b65b35e6874a30ea6340a1ddc44 https://github.com/scala/scala/commit/ace051ff0abe112b767c3912f846eb4d50e52cf5 https://github.com/scala/scala/commit/e156d4a7cf4afdab91b7c281a0e8ae6e4743cc4a It's safe to remove this functionality. It was added originally to support an experiment with resident compilation. The experiment was performed in sbt and dropped in https://github.com/sbt/sbt/commit/6def08e029e474dc35af04b7403a2aeaddd0dec6 Since then Scala team concluded to not work on resident compilation so it's safe to delete unused code. --- src/compiler/scala/tools/nsc/Global.scala | 237 --------------------- .../scala/tools/nsc/settings/ScalaSettings.scala | 1 - 2 files changed, 238 deletions(-) diff --git a/src/compiler/scala/tools/nsc/Global.scala b/src/compiler/scala/tools/nsc/Global.scala index 572e579aca..5bf0d8d9f7 100644 --- a/src/compiler/scala/tools/nsc/Global.scala +++ b/src/compiler/scala/tools/nsc/Global.scala @@ -831,198 +831,11 @@ class Global(var currentSettings: Settings, var reporter: Reporter) } reverse } - // ------------ Invalidations --------------------------------- - - /** Is given package class a system package class that cannot be invalidated? - */ - private def isSystemPackageClass(pkg: Symbol) = - pkg == RootClass || - pkg == definitions.ScalaPackageClass || { - val pkgname = pkg.fullName - (pkgname startsWith "scala.") && !(pkgname startsWith "scala.tools") - } - - /** Invalidates packages that contain classes defined in a classpath entry, and - * rescans that entry. - * @param paths Fully qualified names that refer to directories or jar files that are - * a entries on the classpath. - * First, causes the classpath entry referred to by `path` to be rescanned, so that - * any new files or deleted files or changes in subpackages are picked up. - * Second, invalidates any packages for which one of the following considitions is met: - - * - the classpath entry contained during the last compilation run classfiles - * that represent a member in the package - * - the classpath entry now contains classfiles - * that represent a member in the package - * - the set of subpackages has changed. - * - * The invalidated packages are reset in their entirety; all member classes and member packages - * are re-accessed using the new classpath. - * Not invalidated are system packages that the compiler needs to access as parts - * of standard definitions. The criterion what is a system package is currently: - * any package rooted in "scala", with the exception of packages rooted in "scala.tools". - * This can be refined later. - * @return A pair consisting of - * - a list of invalidated packages - * - a list of of packages that should have been invalidated but were not because - * they are system packages. - */ - def invalidateClassPathEntries(paths: String*): (List[ClassSymbol], List[ClassSymbol]) = { - val invalidated, failed = new mutable.ListBuffer[ClassSymbol] - classPath match { - case cp: MergedClassPath[_] => - def assoc(path: String): List[(PlatformClassPath, PlatformClassPath)] = { - val dir = AbstractFile getDirectory path - val canonical = dir.canonicalPath - def matchesCanonical(e: ClassPath[_]) = e.origin match { - case Some(opath) => - (AbstractFile getDirectory opath).canonicalPath == canonical - case None => - false - } - cp.entries find matchesCanonical match { - case Some(oldEntry) => - List(oldEntry -> cp.context.newClassPath(dir)) - case None => - println(s"canonical = $canonical, origins = ${cp.entries map (_.origin)}") - error(s"cannot invalidate: no entry named $path in classpath $classPath") - List() - } - } - val subst = Map(paths flatMap assoc: _*) - if (subst.nonEmpty) { - platform updateClassPath subst - informProgress(s"classpath updated on entries [${subst.keys mkString ","}]") - def mkClassPath(elems: Iterable[PlatformClassPath]): PlatformClassPath = - if (elems.size == 1) elems.head - else new MergedClassPath(elems, classPath.context) - val oldEntries = mkClassPath(subst.keys) - val newEntries = mkClassPath(subst.values) - reSync(RootClass, Some(classPath), Some(oldEntries), Some(newEntries), invalidated, failed) - } - } - def show(msg: String, syms: scala.collection.Traversable[Symbol]) = - if (syms.nonEmpty) - informProgress(s"$msg: ${syms map (_.fullName) mkString ","}") - show("invalidated packages", invalidated) - show("could not invalidate system packages", failed) - (invalidated.toList, failed.toList) - } - - /** Re-syncs symbol table with classpath - * @param root The root symbol to be resynced (a package class) - * @param allEntries Optionally, the corresponding package in the complete current classPath - * @param oldEntries Optionally, the corresponding package in the old classPath entries - * @param newEntries Optionally, the corresponding package in the new classPath entries - * @param invalidated A listbuffer collecting the invalidated package classes - * @param failed A listbuffer collecting system package classes which could not be invalidated - * The resyncing strategy is determined by the absence or presence of classes and packages. - * If either oldEntries or newEntries contains classes, root is invalidated, provided a corresponding package - * exists in allEntries, or otherwise is removed. - * Otherwise, the action is determined by the following matrix, with columns: - * - * old new all sym action - * + + + + recurse into all child packages of old ++ new - * + - + + invalidate root - * + - - + remove root from its scope - * - + + + invalidate root - * - + + - create and enter root - * - - * * no action - * - * Here, old, new, all mean classpaths and sym means symboltable. + is presence of an - * entry in its column, - is absence, * is don't care. - * - * Note that new <= all and old <= sym, so the matrix above covers all possibilities. - */ - private def reSync(root: ClassSymbol, - allEntries: OptClassPath, oldEntries: OptClassPath, newEntries: OptClassPath, - invalidated: mutable.ListBuffer[ClassSymbol], failed: mutable.ListBuffer[ClassSymbol]) { - ifDebug(informProgress(s"syncing $root, $oldEntries -> $newEntries")) - - val getName: ClassPath[AbstractFile] => String = (_.name) - def hasClasses(cp: OptClassPath) = cp.isDefined && cp.get.classes.nonEmpty - def invalidateOrRemove(root: ClassSymbol) = { - allEntries match { - case Some(cp) => root setInfo new loaders.PackageLoader(cp) - case None => root.owner.info.decls unlink root.sourceModule - } - invalidated += root - } - def packageNames(cp: PlatformClassPath): Set[String] = cp.packages.toSet map getName - def subPackage(cp: PlatformClassPath, name: String): OptClassPath = - cp.packages find (cp1 => getName(cp1) == name) - - val classesFound = hasClasses(oldEntries) || hasClasses(newEntries) - if (classesFound && !isSystemPackageClass(root)) { - invalidateOrRemove(root) - } else { - if (classesFound) { - if (root.isRoot) invalidateOrRemove(EmptyPackageClass) - else failed += root - } - (oldEntries, newEntries) match { - case (Some(oldcp) , Some(newcp)) => - for (pstr <- packageNames(oldcp) ++ packageNames(newcp)) { - val pname = newTermName(pstr) - val pkg = (root.info decl pname) orElse { - // package was created by external agent, create symbol to track it - assert(!subPackage(oldcp, pstr).isDefined) - loaders.enterPackage(root, pstr, new loaders.PackageLoader(allEntries.get)) - } - reSync( - pkg.moduleClass.asInstanceOf[ClassSymbol], - subPackage(allEntries.get, pstr), subPackage(oldcp, pstr), subPackage(newcp, pstr), - invalidated, failed) - } - case (Some(oldcp), None) => - invalidateOrRemove(root) - case (None, Some(newcp)) => - invalidateOrRemove(root) - case (None, None) => - } - } - } - - /** Invalidate contents of setting -Yinvalidate */ - def doInvalidation() = settings.Yinvalidate.value match { - case "" => - case entry => invalidateClassPathEntries(entry) - } - // ----------- Runs --------------------------------------- private var curRun: Run = null private var curRunId = 0 - /** A hook that lets subclasses of `Global` define whether a package or class should be kept loaded for the - * next compiler run. If the parameter `sym` is a class or object, and `clearOnNextRun(sym)` returns `true`, - * then the symbol is unloaded and reset to its state before the last compiler run. If the parameter `sym` is - * a package, and clearOnNextRun(sym)` returns `true`, the package is recursively searched for - * classes to drop. - * - * Example: Let's say I want a compiler that drops all classes corresponding to the current project - * between runs. Then `keepForNextRun` of a toplevel class or object should return `true` if the - * class or object does not form part of the current project, `false` otherwise. For a package, - * clearOnNextRun should return `true` if no class in that package forms part of the current project, - * `false` otherwise. - * - * @param sym A class symbol, object symbol, package, or package class. - */ - @deprecated("use invalidateClassPathEntries instead", "2.10.0") - def clearOnNextRun(sym: Symbol) = false - /* To try out clearOnNext run on the scala.tools.nsc project itself - * replace `false` above with the following code - - settings.Xexperimental.value && { sym.isRoot || { - sym.fullName match { - case "scala" | "scala.tools" | "scala.tools.nsc" => true - case _ => sym.owner.fullName.startsWith("scala.tools.nsc") - } - }} - - * Then, fsc -Xexperimental clears the nsc project between successive runs of `fsc`. - */ - object typeDeconstruct extends { val global: Global.this.type = Global.this } with typechecker.StructuredTypeStrings @@ -1292,47 +1105,6 @@ class Global(var currentSettings: Settings, var reporter: Reporter) first } - /** Reset all classes contained in current project, as determined by - * the clearOnNextRun hook - */ - @deprecated("use invalidateClassPathEntries instead", "2.10.0") - def resetProjectClasses(root: Symbol): Unit = try { - def unlink(sym: Symbol) = - if (sym != NoSymbol) root.info.decls.unlink(sym) - if (settings.verbose) inform("[reset] recursing in "+root) - val toReload = mutable.Set[String]() - for (sym <- root.info.decls) { - if (sym.isInitialized && clearOnNextRun(sym)) - if (sym.hasPackageFlag) { - resetProjectClasses(sym.moduleClass) - openPackageModule(sym.moduleClass) - } else { - unlink(sym) - unlink(root.info.decls.lookup( - if (sym.isTerm) sym.name.toTypeName else sym.name.toTermName)) - toReload += sym.fullName - // note: toReload could be set twice with the same name - // but reinit must happen only once per name. That's why - // the following classPath.findClass { ... } code cannot be moved here. - } - } - for (fullname <- toReload) - classPath.findClass(fullname) match { - case Some(classRep) => - if (settings.verbose) inform("[reset] reinit "+fullname) - loaders.initializeFromClassPath(root, classRep) - case _ => - } - } catch { - case ex: Throwable => - // this handler should not be nessasary, but it seems that `fsc` - // eats exceptions if they appear here. Need to find out the cause for - // this and fix it. - inform("[reset] exception happened: "+ex) - ex.printStackTrace() - throw ex - } - // --------------- Miscellania ------------------------------- /** Progress tracking. Measured in "progress units" which are 1 per @@ -1542,8 +1314,6 @@ class Global(var currentSettings: Settings, var reporter: Reporter) compileUnitsInternal(units, fromPhase) private def compileUnitsInternal(units: List[CompilationUnit], fromPhase: Phase) { - doInvalidation() - units foreach addUnit val startTime = currentTime @@ -1619,13 +1389,6 @@ class Global(var currentSettings: Settings, var reporter: Reporter) // Clear any sets or maps created via perRunCaches. perRunCaches.clearAll() - - // Reset project - if (!stopPhase("namer")) { - enteringPhase(namerPhase) { - resetProjectClasses(RootClass) - } - } } /** Compile list of abstract files. */ diff --git a/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala b/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala index d22dcacad6..87cd05bdb4 100644 --- a/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala +++ b/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala @@ -185,7 +185,6 @@ trait ScalaSettings extends AbsScalaSettings val YmethodInfer = BooleanSetting ("-Yinfer-argument-types", "Infer types for arguments of overriden methods.") val etaExpandKeepsStar = BooleanSetting ("-Yeta-expand-keeps-star", "Eta-expand varargs methods to T* rather than Seq[T]. This is a temporary option to ease transition.").withDeprecationMessage(removalIn212) val inferByName = BooleanSetting ("-Yinfer-by-name", "Allow inference of by-name types. This is a temporary option to ease transition. See SI-7899.").withDeprecationMessage(removalIn212) - val Yinvalidate = StringSetting ("-Yinvalidate", "classpath-entry", "Invalidate classpath entry before run", "") val YvirtClasses = false // too embryonic to even expose as a -Y //BooleanSetting ("-Yvirtual-classes", "Support virtual classes") val YdisableUnreachablePrevention = BooleanSetting("-Ydisable-unreachable-prevention", "Disable the prevention of unreachable blocks in code generation.") val YnoLoadImplClass = BooleanSetting ("-Yno-load-impl-class", "Do not load $class.class files.") -- cgit v1.2.3 From bde623925d011841f222891050c5fdb08f3bb251 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Thu, 17 Jul 2014 10:04:20 -0700 Subject: SI-8525 Multichoice help Enables -Xlint:help and -language:help. The Settings API makes it difficult to innovate. --- src/compiler/scala/tools/nsc/CompilerCommand.scala | 10 +++++++++- .../scala/tools/nsc/settings/MutableSettings.scala | 17 +++++++++++------ .../scala/tools/nsc/settings/ScalaSettings.scala | 7 +++++-- src/compiler/scala/tools/nsc/settings/Warnings.scala | 18 +++++++++++++----- 4 files changed, 38 insertions(+), 14 deletions(-) diff --git a/src/compiler/scala/tools/nsc/CompilerCommand.scala b/src/compiler/scala/tools/nsc/CompilerCommand.scala index a1d0d52dcf..95f7a70980 100644 --- a/src/compiler/scala/tools/nsc/CompilerCommand.scala +++ b/src/compiler/scala/tools/nsc/CompilerCommand.scala @@ -103,7 +103,15 @@ class CompilerCommand(arguments: List[String], val settings: Settings) { val components = global.phaseNames // global.phaseDescriptors // one initializes s"Phase graph of ${components.size} components output to ${genPhaseGraph.value}*.dot." } - else "" + // would be nicer if we could ask all the options for their helpful messages + else { + val sb = new StringBuilder + allSettings foreach { + case s: MultiChoiceSetting if s.isHelping => sb append s.help append "\n" + case _ => + } + sb.toString + } } /** diff --git a/src/compiler/scala/tools/nsc/settings/MutableSettings.scala b/src/compiler/scala/tools/nsc/settings/MutableSettings.scala index 0dd4ae0b3b..8c69b49b98 100644 --- a/src/compiler/scala/tools/nsc/settings/MutableSettings.scala +++ b/src/compiler/scala/tools/nsc/settings/MutableSettings.scala @@ -201,7 +201,7 @@ class MutableSettings(val errorFn: String => Unit) } // a wrapper for all Setting creators to keep our list up to date - private def add[T <: Setting](s: T): T = { + private[nsc] def add[T <: Setting](s: T): T = { allSettings += s s } @@ -552,7 +552,7 @@ class MutableSettings(val errorFn: String => Unit) } /** A setting that receives any combination of enumerated values, - * including "_" to mean all values. + * including "_" to mean all values and "help" for verbose info. * In non-colonated mode, stops consuming args at the first * non-value, instead of at the next option, as for a multi-string. */ @@ -562,16 +562,18 @@ class MutableSettings(val errorFn: String => Unit) descr: String, override val choices: List[String], val default: () => Unit - ) extends MultiStringSetting(name, arg, s"$descr${ choices.mkString(": ", ",", ".") }") { + ) extends MultiStringSetting(name, s"_,$arg,-$arg", s"$descr: `_' for all, `$name:help' to list") { - def badChoice(s: String, n: String) = errorFn(s"'$s' is not a valid choice for '$name'") - def choosing = choices.nonEmpty - def isChoice(s: String) = (s == "_") || (choices contains (s stripPrefix "-")) + private def badChoice(s: String, n: String) = errorFn(s"'$s' is not a valid choice for '$name'") + private def choosing = choices.nonEmpty + private def isChoice(s: String) = (s == "_") || (choices contains (s stripPrefix "-")) + private var sawHelp = false override protected def tts(args: List[String], halting: Boolean) = { val added = collection.mutable.ListBuffer.empty[String] def tryArg(arg: String) = arg match { case "_" if choosing => default() + case "help" if choosing => sawHelp = true case s if !choosing || isChoice(s) => added += s case s => badChoice(s, name) } @@ -586,6 +588,9 @@ class MutableSettings(val errorFn: String => Unit) else value = added.toList // update all new settings at once Some(rest) } + + def isHelping: Boolean = sawHelp + def help: String = s"$descr${ choices.mkString(":\n", " \n", "\n") }" } /** A setting that accumulates all strings supplied to it, diff --git a/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala b/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala index d22dcacad6..6675ad96e6 100644 --- a/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala +++ b/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala @@ -44,8 +44,11 @@ trait ScalaSettings extends AbsScalaSettings /** If any of these settings is enabled, the compiler should print a message and exit. */ def infoSettings = List[Setting](version, help, Xhelp, Yhelp, showPlugins, showPhases, genPhaseGraph) + /** Any -multichoice:help? Nicer if any option could report that it had help to offer. */ + private def multihelp = allSettings exists { case s: MultiChoiceSetting => s.isHelping case _ => false } + /** Is an info setting set? */ - def isInfo = infoSettings exists (_.isSetByUser) + def isInfo = (infoSettings exists (_.isSetByUser)) || multihelp /** Disable a setting */ def disable(s: Setting) = allSettings -= s @@ -68,7 +71,7 @@ trait ScalaSettings extends AbsScalaSettings // 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) + MultiChoiceSetting("-language", "feature", "Enable or disable language features", features) } /* diff --git a/src/compiler/scala/tools/nsc/settings/Warnings.scala b/src/compiler/scala/tools/nsc/settings/Warnings.scala index 0b9ad80041..3accd9fdaa 100644 --- a/src/compiler/scala/tools/nsc/settings/Warnings.scala +++ b/src/compiler/scala/tools/nsc/settings/Warnings.scala @@ -108,14 +108,19 @@ trait Warnings { // The Xlint warning group. private val xlint = new BooleanSetting("-Zunused", "True if -Xlint or -Xlint:_") // On -Xlint or -Xlint:_, set xlint, otherwise set the lint warning unless already set true - val lint = - MultiChoiceSetting( + val lint = { + val d = "Enable or disable specific warnings" + val s = new MultiChoiceSetting( name = "-Xlint", - helpArg = "warning", - descr = "Enable recommended additional warnings", + arg = "warning", + descr = d, choices = (lintWarnings map (_.name)).sorted, default = () => xlint.value = true - ) withPostSetHook { x => + ) { + def helpline(n: String) = lintWarnings.find(_.name == n).map(w => f" ${w.name}%-25s ${w.helpDescription}%n") + override def help = s"$d${ choices flatMap (helpline(_)) mkString (":\n", "", "\n") }" + } + val t = s withPostSetHook { x => val Neg = "-" def setPolitely(b: BooleanSetting, v: Boolean) = if (!b.isSetByUser || !b) b.value = v def set(w: String, v: Boolean) = lintWarnings find (_.name == w) foreach (setPolitely(_, v)) @@ -125,6 +130,9 @@ trait Warnings { } propagate(x.value) } + add(t) + t + } // Lint warnings that are currently -Y, but deprecated in that usage @deprecated("Use warnAdaptedArgs", since="2.11.2") -- cgit v1.2.3 From 68560dd80d99f031bdf419f08463abeb83e47b3c Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Fri, 18 Jul 2014 00:19:01 -0700 Subject: SI-8525 MultiChoice takes a helper function Clean-up code review comments. MultiChoice takes a helper function for outputting help text. --- src/compiler/scala/tools/nsc/CompilerCommand.scala | 2 +- .../tools/nsc/settings/AbsScalaSettings.scala | 2 +- .../scala/tools/nsc/settings/MutableSettings.scala | 13 +++++++----- .../scala/tools/nsc/settings/ScalaSettings.scala | 23 ++++++++++++++++++++-- .../scala/tools/nsc/settings/Warnings.scala | 20 +++++++++---------- 5 files changed, 40 insertions(+), 20 deletions(-) diff --git a/src/compiler/scala/tools/nsc/CompilerCommand.scala b/src/compiler/scala/tools/nsc/CompilerCommand.scala index 95f7a70980..3ded456378 100644 --- a/src/compiler/scala/tools/nsc/CompilerCommand.scala +++ b/src/compiler/scala/tools/nsc/CompilerCommand.scala @@ -107,7 +107,7 @@ class CompilerCommand(arguments: List[String], val settings: Settings) { else { val sb = new StringBuilder allSettings foreach { - case s: MultiChoiceSetting if s.isHelping => sb append s.help append "\n" + case s: MultiChoiceSetting if s.isHelping => sb append s.help case _ => } sb.toString diff --git a/src/compiler/scala/tools/nsc/settings/AbsScalaSettings.scala b/src/compiler/scala/tools/nsc/settings/AbsScalaSettings.scala index ef9695a594..c73e7ce00e 100644 --- a/src/compiler/scala/tools/nsc/settings/AbsScalaSettings.scala +++ b/src/compiler/scala/tools/nsc/settings/AbsScalaSettings.scala @@ -29,7 +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], default: () => Unit): MultiChoiceSetting + def MultiChoiceSetting(name: String, helpArg: String, descr: String, choices: List[String], default: () => Unit)(helper: MultiChoiceSetting => 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 8c69b49b98..3f41ede3ad 100644 --- a/src/compiler/scala/tools/nsc/settings/MutableSettings.scala +++ b/src/compiler/scala/tools/nsc/settings/MutableSettings.scala @@ -201,7 +201,7 @@ class MutableSettings(val errorFn: String => Unit) } // a wrapper for all Setting creators to keep our list up to date - private[nsc] def add[T <: Setting](s: T): T = { + private def add[T <: Setting](s: T): T = { allSettings += s s } @@ -211,8 +211,10 @@ 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], default: () => Unit = () => ()) = - add(new MultiChoiceSetting(name, helpArg, descr, choices, default)) + def MultiChoiceSetting(name: String, helpArg: String, descr: String, choices: List[String], default: () => Unit = () => ())( + helper: MultiChoiceSetting => String = _ => choices.mkString(f"$descr:%n", f"%n ", f"%n") + ) = + add(new MultiChoiceSetting(name, helpArg, descr, choices, default, helper)) 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)) @@ -561,7 +563,8 @@ class MutableSettings(val errorFn: String => Unit) arg: String, descr: String, override val choices: List[String], - val default: () => Unit + val default: () => Unit, + helper: MultiChoiceSetting => String ) extends MultiStringSetting(name, s"_,$arg,-$arg", s"$descr: `_' for all, `$name:help' to list") { private def badChoice(s: String, n: String) = errorFn(s"'$s' is not a valid choice for '$name'") @@ -590,7 +593,7 @@ class MutableSettings(val errorFn: String => Unit) } def isHelping: Boolean = sawHelp - def help: String = s"$descr${ choices.mkString(":\n", " \n", "\n") }" + def help: String = helper(this) } /** A setting that accumulates all strings supplied to it, diff --git a/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala b/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala index 6675ad96e6..5c3eadbd60 100644 --- a/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala +++ b/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala @@ -70,8 +70,27 @@ trait ScalaSettings extends AbsScalaSettings // 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 or disable language features", features) + val features = List( + "dynamics" -> "Allow direct or indirect subclasses of scala.Dynamic", + "postfixOps" -> "Allow postfix operator notation, such as `1 to 10 toList'", + "reflectiveCalls" -> "Allow reflective access to members of structural types", + "implicitConversions" -> "Allow definition of implicit functions called views", + "higherKinds" -> "Allow higher-kinded types", // "Ask Adriaan, but if you have to ask..." + "existentials" -> "Existential types (besides wildcard types) can be written and inferred", + "experimental.macros" -> "Allow macro defintion (besides implementation and application)" + ) + val description = "Enable or disable language features" + MultiChoiceSetting( + name = "-language", + helpArg = "feature", + descr = description, + choices = features map (_._1) + ) { s => + val helpline: ((String, String)) => String = { + case (name, text) => f" $name%-25s $text%n" + } + features map helpline mkString (f"$description:%n", "", f"%n") + } } /* diff --git a/src/compiler/scala/tools/nsc/settings/Warnings.scala b/src/compiler/scala/tools/nsc/settings/Warnings.scala index 3accd9fdaa..9989d1d188 100644 --- a/src/compiler/scala/tools/nsc/settings/Warnings.scala +++ b/src/compiler/scala/tools/nsc/settings/Warnings.scala @@ -109,18 +109,18 @@ trait Warnings { private val xlint = new BooleanSetting("-Zunused", "True if -Xlint or -Xlint:_") // On -Xlint or -Xlint:_, set xlint, otherwise set the lint warning unless already set true val lint = { - val d = "Enable or disable specific warnings" - val s = new MultiChoiceSetting( + val description = "Enable or disable specific warnings" + val choices = (lintWarnings map (_.name)).sorted + MultiChoiceSetting( name = "-Xlint", - arg = "warning", - descr = d, - choices = (lintWarnings map (_.name)).sorted, + helpArg = "warning", + descr = description, + choices = choices, default = () => xlint.value = true - ) { + ) { s => def helpline(n: String) = lintWarnings.find(_.name == n).map(w => f" ${w.name}%-25s ${w.helpDescription}%n") - override def help = s"$d${ choices flatMap (helpline(_)) mkString (":\n", "", "\n") }" - } - val t = s withPostSetHook { x => + choices flatMap (helpline(_)) mkString (f"$description:%n", "", f"%n") + } withPostSetHook { x => val Neg = "-" def setPolitely(b: BooleanSetting, v: Boolean) = if (!b.isSetByUser || !b) b.value = v def set(w: String, v: Boolean) = lintWarnings find (_.name == w) foreach (setPolitely(_, v)) @@ -130,8 +130,6 @@ trait Warnings { } propagate(x.value) } - add(t) - t } // Lint warnings that are currently -Y, but deprecated in that usage -- cgit v1.2.3 From c7cc1a803764b4d26dd23d2baf34a27a3a40f682 Mon Sep 17 00:00:00 2001 From: Rex Kerr Date: Sun, 20 Jul 2014 12:36:25 -0700 Subject: SI-8738 Regression in range equality Missed the case of comparing a non-empty range to an empty one. Fixed by checking nonEmpty/isEmpty on other collection. Added a test to verify the behavior. --- src/library/scala/collection/immutable/Range.scala | 19 ++++++++++--------- test/files/run/t8738.scala | 16 ++++++++++++++++ 2 files changed, 26 insertions(+), 9 deletions(-) create mode 100644 test/files/run/t8738.scala diff --git a/src/library/scala/collection/immutable/Range.scala b/src/library/scala/collection/immutable/Range.scala index 26ccd09803..720dfeed59 100644 --- a/src/library/scala/collection/immutable/Range.scala +++ b/src/library/scala/collection/immutable/Range.scala @@ -364,15 +364,16 @@ extends scala.collection.AbstractSeq[Int] override def equals(other: Any) = other match { case x: Range => // Note: this must succeed for overfull ranges (length > Int.MaxValue) - (x canEqual this) && ( - isEmpty || // all empty sequences are equal - (start == x.start && { // Otherwise, must have same start - val l0 = last - (l0 == x.last && ( // And same end - start == l0 || step == x.step // And either the same step, or not take any steps - )) - }) - ) + (x canEqual this) && { + if (isEmpty) x.isEmpty // empty sequences are equal + else // this is non-empty... + x.nonEmpty && start == x.start && { // ...so other must contain something and have same start + val l0 = last + (l0 == x.last && ( // And same end + start == l0 || step == x.step // And either the same step, or not take any steps + )) + } + } case _ => super.equals(other) } diff --git a/test/files/run/t8738.scala b/test/files/run/t8738.scala new file mode 100644 index 0000000000..6898301db7 --- /dev/null +++ b/test/files/run/t8738.scala @@ -0,0 +1,16 @@ +object Test { + def check(a: Range, b: Range) = (a == b) == (a.toList == b.toList) + def main(args: Array[String]) { + val lo = -2 to 2 + val hi = lo + val step = List(-6,-2,-1,1,2,6) + for (i <- lo; j <- hi; n <- step; k <- lo; l <- hi; m <- step) { + assert( + check(i until j by n, k until l by m) && + check(i until j by n, k to l by m) && + check(i to j by n, k until l by m) && + check(i to j by n, k to l by m) + ) + } + } +} -- cgit v1.2.3 From 01da58e6b6d356e7830183806056e27f70099caf Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Sun, 20 Jul 2014 14:48:33 -0700 Subject: SI-8736 Restore -language to former glory MultiChoice allows -language to work like -Xlint. The bug as described was that the setting value was set instead of updated (++=) with additional flags. The unreported bug was that `_` no longer set all settings. The corrected behavior is that "contains" means "it was enabled, or _ was set and it was not disabled explicitly." That is the behavior of `-Xlint` but with a different mechanism, since each lint option is a Setting. A semantic difference is that -Xlint enables all the lint options, but -language does not enable all the language features. `scalac -X` does not explain this additional behavior of the `-Xlint` flag. Also worth noting that `scalac -feature -language unused.scala` failed in 2.11.1 but succeeds silently now. --- .../scala/tools/nsc/settings/AbsScalaSettings.scala | 2 +- .../scala/tools/nsc/settings/MutableSettings.scala | 19 +++++++++++++------ src/compiler/scala/tools/nsc/settings/Warnings.scala | 2 +- test/files/neg/t8736-c.check | 11 +++++++++++ test/files/neg/t8736-c.flags | 1 + test/files/neg/t8736-c.scala | 7 +++++++ test/files/pos/t8736-b.flags | 1 + test/files/pos/t8736-b.scala | 7 +++++++ test/files/pos/t8736.flags | 1 + test/files/pos/t8736.scala | 7 +++++++ 10 files changed, 50 insertions(+), 8 deletions(-) create mode 100644 test/files/neg/t8736-c.check create mode 100644 test/files/neg/t8736-c.flags create mode 100644 test/files/neg/t8736-c.scala create mode 100644 test/files/pos/t8736-b.flags create mode 100644 test/files/pos/t8736-b.scala create mode 100644 test/files/pos/t8736.flags create mode 100644 test/files/pos/t8736.scala diff --git a/src/compiler/scala/tools/nsc/settings/AbsScalaSettings.scala b/src/compiler/scala/tools/nsc/settings/AbsScalaSettings.scala index c73e7ce00e..d0b8fd70ed 100644 --- a/src/compiler/scala/tools/nsc/settings/AbsScalaSettings.scala +++ b/src/compiler/scala/tools/nsc/settings/AbsScalaSettings.scala @@ -29,7 +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], default: () => Unit)(helper: MultiChoiceSetting => String): MultiChoiceSetting + def MultiChoiceSetting(name: String, helpArg: String, descr: String, choices: List[String], default: Option[() => Unit])(helper: MultiChoiceSetting => 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 3f41ede3ad..ed8fb2a630 100644 --- a/src/compiler/scala/tools/nsc/settings/MutableSettings.scala +++ b/src/compiler/scala/tools/nsc/settings/MutableSettings.scala @@ -211,7 +211,7 @@ 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], default: () => Unit = () => ())( + def MultiChoiceSetting(name: String, helpArg: String, descr: String, choices: List[String], default: Option[() => Unit] = None)( helper: MultiChoiceSetting => String = _ => choices.mkString(f"$descr:%n", f"%n ", f"%n") ) = add(new MultiChoiceSetting(name, helpArg, descr, choices, default, helper)) @@ -563,19 +563,21 @@ class MutableSettings(val errorFn: String => Unit) arg: String, descr: String, override val choices: List[String], - val default: () => Unit, + val default: Option[() => Unit], helper: MultiChoiceSetting => String ) extends MultiStringSetting(name, s"_,$arg,-$arg", s"$descr: `_' for all, `$name:help' to list") { private def badChoice(s: String, n: String) = errorFn(s"'$s' is not a valid choice for '$name'") private def choosing = choices.nonEmpty private def isChoice(s: String) = (s == "_") || (choices contains (s stripPrefix "-")) + private var sawHelp = false + private var sawAll = false override protected def tts(args: List[String], halting: Boolean) = { val added = collection.mutable.ListBuffer.empty[String] def tryArg(arg: String) = arg match { - case "_" if choosing => default() + case "_" if choosing => addAll() case "help" if choosing => sawHelp = true case s if !choosing || isChoice(s) => added += s case s => badChoice(s, name) @@ -587,13 +589,18 @@ class MutableSettings(val errorFn: String => Unit) case Nil => Nil } val rest = loop(args) - if (rest.size == args.size) default() // if no arg consumed, trigger default action - else value = added.toList // update all new settings at once + if (rest.size == args.size) default foreach (_()) // if no arg consumed, trigger default action + else value ++= added.toList // update all new settings at once Some(rest) } def isHelping: Boolean = sawHelp - def help: String = helper(this) + def help: String = helper(this) + private val adderAll = Some(() => sawAll = true) + def addAll(): Unit = default orElse adderAll foreach (_()) + + // the semantics is: s is enabled, i.e., either s or (_ but not -s) + override def contains(s: String) = isChoice(s) && (value contains s) || (sawAll && !(value contains s"-$s")) } /** A setting that accumulates all strings supplied to it, diff --git a/src/compiler/scala/tools/nsc/settings/Warnings.scala b/src/compiler/scala/tools/nsc/settings/Warnings.scala index 9989d1d188..bec068b56a 100644 --- a/src/compiler/scala/tools/nsc/settings/Warnings.scala +++ b/src/compiler/scala/tools/nsc/settings/Warnings.scala @@ -116,7 +116,7 @@ trait Warnings { helpArg = "warning", descr = description, choices = choices, - default = () => xlint.value = true + default = Some(() => xlint.value = true) ) { s => def helpline(n: String) = lintWarnings.find(_.name == n).map(w => f" ${w.name}%-25s ${w.helpDescription}%n") choices flatMap (helpline(_)) mkString (f"$description:%n", "", f"%n") diff --git a/test/files/neg/t8736-c.check b/test/files/neg/t8736-c.check new file mode 100644 index 0000000000..06b2228543 --- /dev/null +++ b/test/files/neg/t8736-c.check @@ -0,0 +1,11 @@ +t8736-c.scala:4: warning: higher-kinded type should be enabled +by making the implicit value scala.language.higherKinds visible. +This can be achieved by adding the import clause 'import scala.language.higherKinds' +or by setting the compiler option -language:higherKinds. +See the Scala docs for value scala.language.higherKinds for a discussion +why the feature should be explicitly enabled. + def hk[M[_]] = ??? + ^ +error: No warnings can be incurred under -Xfatal-warnings. +one warning found +one error found diff --git a/test/files/neg/t8736-c.flags b/test/files/neg/t8736-c.flags new file mode 100644 index 0000000000..fde5313c96 --- /dev/null +++ b/test/files/neg/t8736-c.flags @@ -0,0 +1 @@ +-feature -language:-higherKinds,_ -Xfatal-warnings diff --git a/test/files/neg/t8736-c.scala b/test/files/neg/t8736-c.scala new file mode 100644 index 0000000000..8432775ae1 --- /dev/null +++ b/test/files/neg/t8736-c.scala @@ -0,0 +1,7 @@ +// scalac: -feature -language:-higherKinds,_ -Xfatal-warnings +// showing that wildcard doesn't supersede explicit disablement +class X { + def hk[M[_]] = ??? + + implicit def imp(x: X): Int = x.hashCode +} diff --git a/test/files/pos/t8736-b.flags b/test/files/pos/t8736-b.flags new file mode 100644 index 0000000000..1ad4eabe0f --- /dev/null +++ b/test/files/pos/t8736-b.flags @@ -0,0 +1 @@ +-feature -language:_ -Xfatal-warnings diff --git a/test/files/pos/t8736-b.scala b/test/files/pos/t8736-b.scala new file mode 100644 index 0000000000..903292d232 --- /dev/null +++ b/test/files/pos/t8736-b.scala @@ -0,0 +1,7 @@ +// scalac: -feature -language:_ -Xfatal-warnings +// showing that all are set +class X { + def hk[M[_]] = ??? + + implicit def imp(x: X): Int = x.hashCode +} diff --git a/test/files/pos/t8736.flags b/test/files/pos/t8736.flags new file mode 100644 index 0000000000..7fe42f7340 --- /dev/null +++ b/test/files/pos/t8736.flags @@ -0,0 +1 @@ +-feature -language:implicitConversions -language:higherKinds -language:-implicitConversions -Xfatal-warnings diff --git a/test/files/pos/t8736.scala b/test/files/pos/t8736.scala new file mode 100644 index 0000000000..46c0cdfd00 --- /dev/null +++ b/test/files/pos/t8736.scala @@ -0,0 +1,7 @@ +// scalac: -feature -language:implicitConversions -language:higherKinds -language:-implicitConversions -Xfatal-warnings +// showing that multiple settings are respected, and explicit enablement has precedence +class X { + def hk[M[_]] = ??? + + implicit def imp(x: X): Int = x.hashCode +} -- cgit v1.2.3 From 5a30c40d3c9882bf4213124f8921b2be10d5f444 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Sun, 20 Jul 2014 21:11:55 -0700 Subject: SI-8736 Error if no arg and no default Now `-language` with no option will emit an error: ``` $ skalac -feature -language unused.scala scalac error: '-language' requires an option. See '-language:help'. scalac -help gives more information ``` --- .../scala/tools/nsc/settings/MutableSettings.scala | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/compiler/scala/tools/nsc/settings/MutableSettings.scala b/src/compiler/scala/tools/nsc/settings/MutableSettings.scala index ed8fb2a630..f26192f88a 100644 --- a/src/compiler/scala/tools/nsc/settings/MutableSettings.scala +++ b/src/compiler/scala/tools/nsc/settings/MutableSettings.scala @@ -571,8 +571,10 @@ class MutableSettings(val errorFn: String => Unit) private def choosing = choices.nonEmpty private def isChoice(s: String) = (s == "_") || (choices contains (s stripPrefix "-")) - private var sawHelp = false - private var sawAll = false + private var sawHelp = false + private var sawAll = false + private val adderAll = () => sawAll = true + private val noargs = () => errorFn(s"'$name' requires an option. See '$name:help'.") override protected def tts(args: List[String], halting: Boolean) = { val added = collection.mutable.ListBuffer.empty[String] @@ -589,15 +591,16 @@ class MutableSettings(val errorFn: String => Unit) case Nil => Nil } val rest = loop(args) - if (rest.size == args.size) default foreach (_()) // if no arg consumed, trigger default action - else value ++= added.toList // update all new settings at once + if (rest.size == args.size) + (default getOrElse noargs)() // if no arg consumed, trigger default action or error + else + value ++= added.toList // update all new settings at once Some(rest) } def isHelping: Boolean = sawHelp def help: String = helper(this) - private val adderAll = Some(() => sawAll = true) - def addAll(): Unit = default orElse adderAll foreach (_()) + def addAll(): Unit = (default getOrElse adderAll)() // the semantics is: s is enabled, i.e., either s or (_ but not -s) override def contains(s: String) = isChoice(s) && (value contains s) || (sawAll && !(value contains s"-$s")) -- cgit v1.2.3 From 7a1863e7d2a7db5ab0ceecde436ab6d63e03a9f1 Mon Sep 17 00:00:00 2001 From: Dan Garrette Date: Mon, 21 Jul 2014 09:01:43 -0500 Subject: Remove "throws InvalidEscapeException" from StringContext.raw doc Since StringContext.raw doesn't escape its input, it does not call `treatEscapes` and cannot throw the InvalidEscapeException. --- src/library/scala/StringContext.scala | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/library/scala/StringContext.scala b/src/library/scala/StringContext.scala index c0468e8b02..a99b9503ac 100644 --- a/src/library/scala/StringContext.scala +++ b/src/library/scala/StringContext.scala @@ -112,8 +112,6 @@ case class StringContext(parts: String*) { * @throws An `IllegalArgumentException` * if the number of `parts` in the enclosing `StringContext` does not exceed * the number of arguments `arg` by exactly 1. - * @throws A `StringContext.InvalidEscapeException` if a `parts` string contains a backslash (`\`) character - * that does not start a valid escape sequence. */ def raw(args: Any*): String = standardInterpolator(identity, args) -- cgit v1.2.3 From 0f5580efb1d30b95513121bc499ba5187330f601 Mon Sep 17 00:00:00 2001 From: Dan Garrette Date: Mon, 21 Jul 2014 12:36:04 -0500 Subject: Fixed incorrect example in StringContext.raw doc As pointed out by @som-snytt, \u0023 is #, not \u0025. --- src/library/scala/StringContext.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/library/scala/StringContext.scala b/src/library/scala/StringContext.scala index a99b9503ac..fe69c6fbf8 100644 --- a/src/library/scala/StringContext.scala +++ b/src/library/scala/StringContext.scala @@ -104,7 +104,7 @@ case class StringContext(parts: String*) { * ''Note:'' Even when using the raw interpolator, Scala will preprocess unicode escapes. * For example: * {{{ - * scala> raw"\u005cu0025" + * scala> raw"\u005cu0023" * res0: String = # * }}} * -- cgit v1.2.3 From 926585a3f36f35c9afc3f9ed9fe0ded72f6b7014 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 23 Jul 2014 09:56:24 +0200 Subject: SI-8743 Fix crasher with poly-methods annotated with @varargs The code that generated the Java varargs forwarder was basing things on the `ValDef-s` of the parameters of the source method. But, their types refer to a type parameter skolems of the enclosing method, which led to a type mismatch when typechecking the forwarder. Instead, I've reworked the code to simply use the `DefDef`-s symbol's info, which *doesn't* refer to skolems. This actually simplifies the surrounding code somewhat; rather than repeated symbols in a map we can just time travel the pre-uncurry method signatures to figure out which params are releated. --- .../scala/tools/nsc/transform/UnCurry.scala | 41 +++++++++------------- src/reflect/scala/reflect/internal/TreeInfo.scala | 7 ---- test/files/jvm/varargs.check | 3 +- test/files/jvm/varargs/JavaClass.java | 1 + test/files/jvm/varargs/VaClass.scala | 2 +- test/files/pos/t8743.scala | 15 ++++++++ 6 files changed, 35 insertions(+), 34 deletions(-) create mode 100644 test/files/pos/t8743.scala diff --git a/src/compiler/scala/tools/nsc/transform/UnCurry.scala b/src/compiler/scala/tools/nsc/transform/UnCurry.scala index 2209aac00f..9b85f1b36a 100644 --- a/src/compiler/scala/tools/nsc/transform/UnCurry.scala +++ b/src/compiler/scala/tools/nsc/transform/UnCurry.scala @@ -69,7 +69,6 @@ abstract class UnCurry extends InfoTransform private val byNameArgs = mutable.HashSet[Tree]() private val noApply = mutable.HashSet[Tree]() private val newMembers = mutable.Map[Symbol, mutable.Buffer[Tree]]() - private val repeatedParams = mutable.Map[Symbol, List[ValDef]]() /** Add a new synthetic member for `currentOwner` */ private def addNewMember(t: Tree): Unit = @@ -428,7 +427,7 @@ abstract class UnCurry extends InfoTransform treeCopy.ValDef(p, p.mods, p.name, p.tpt, EmptyTree) }) - if (dd.symbol hasAnnotation VarargsClass) saveRepeatedParams(dd) + if (dd.symbol hasAnnotation VarargsClass) validateVarargs(dd) withNeedLift(needLift = false) { if (dd.symbol.isClassConstructor) { @@ -699,19 +698,12 @@ abstract class UnCurry extends InfoTransform } } - - /* Analyzes repeated params if method is annotated as `varargs`. - * If the repeated params exist, it saves them into the `repeatedParams` map, - * which is used later. - */ - private def saveRepeatedParams(dd: DefDef): Unit = + private def validateVarargs(dd: DefDef): Unit = if (dd.symbol.isConstructor) reporter.error(dd.symbol.pos, "A constructor cannot be annotated with a `varargs` annotation.") - else treeInfo.repeatedParams(dd) match { - case Nil => - reporter.error(dd.symbol.pos, "A method without repeated parameters cannot be annotated with the `varargs` annotation.") - case reps => - repeatedParams(dd.symbol) = reps + else { + val hasRepeated = mexists(dd.symbol.paramss)(sym => definitions.isRepeatedParamType(sym.tpe)) + if (!hasRepeated) reporter.error(dd.symbol.pos, "A method without repeated parameters cannot be annotated with the `varargs` annotation.") } /* Called during post transform, after the method argument lists have been flattened. @@ -719,7 +711,7 @@ abstract class UnCurry extends InfoTransform * varargs forwarder. */ private def addJavaVarargsForwarders(dd: DefDef, flatdd: DefDef): DefDef = { - if (!dd.symbol.hasAnnotation(VarargsClass) || !repeatedParams.contains(dd.symbol)) + if (!dd.symbol.hasAnnotation(VarargsClass) || !enteringUncurry(mexists(dd.symbol.paramss)(sym => definitions.isRepeatedParamType(sym.tpe)))) return flatdd def toArrayType(tp: Type): Type = { @@ -735,19 +727,18 @@ abstract class UnCurry extends InfoTransform ) } - val reps = repeatedParams(dd.symbol) - val rpsymbols = reps.map(_.symbol).toSet val theTyper = typer.atOwner(dd, currentClass) - val flatparams = flatdd.vparamss.head + val flatparams = flatdd.symbol.paramss.head + val isRepeated = enteringUncurry(dd.symbol.info.paramss.flatten.map(sym => definitions.isRepeatedParamType(sym.tpe))) // create the type - val forwformals = flatparams map { - case p if rpsymbols(p.symbol) => toArrayType(p.symbol.tpe) - case p => p.symbol.tpe + val forwformals = map2(flatparams, isRepeated) { + case (p, true) => toArrayType(p.tpe) + case (p, false)=> p.tpe } val forwresult = dd.symbol.tpe_*.finalResultType val forwformsyms = map2(forwformals, flatparams)((tp, oldparam) => - currentClass.newValueParameter(oldparam.name, oldparam.symbol.pos).setInfo(tp) + currentClass.newValueParameter(oldparam.name.toTermName, oldparam.pos).setInfo(tp) ) def mono = MethodType(forwformsyms, forwresult) val forwtype = dd.symbol.tpe match { @@ -761,13 +752,13 @@ abstract class UnCurry extends InfoTransform // create the tree val forwtree = theTyper.typedPos(dd.pos) { - val locals = map2(forwParams, flatparams) { - case (_, fp) if !rpsymbols(fp.symbol) => null - case (argsym, fp) => + val locals = map3(forwParams, flatparams, isRepeated) { + case (_, fp, false) => null + case (argsym, fp, true) => Block(Nil, gen.mkCast( gen.mkWrapArray(Ident(argsym), elementType(ArrayClass, argsym.tpe)), - seqType(elementType(SeqClass, fp.symbol.tpe)) + seqType(elementType(SeqClass, fp.tpe)) ) ) } diff --git a/src/reflect/scala/reflect/internal/TreeInfo.scala b/src/reflect/scala/reflect/internal/TreeInfo.scala index b7d7d4df88..c521277f69 100644 --- a/src/reflect/scala/reflect/internal/TreeInfo.scala +++ b/src/reflect/scala/reflect/internal/TreeInfo.scala @@ -509,13 +509,6 @@ abstract class TreeInfo { case _ => false } - /** The parameter ValDefs of a method definition that have vararg types of the form T* - */ - def repeatedParams(tree: Tree): List[ValDef] = tree match { - case DefDef(_, _, _, vparamss, _, _) => vparamss.flatten filter (vd => isRepeatedParamType(vd.tpt)) - case _ => Nil - } - /** Is tpt a by-name parameter type of the form => T? */ def isByNameParamType(tpt: Tree) = tpt match { case TypeTree() => definitions.isByNameParamType(tpt.tpe) diff --git a/test/files/jvm/varargs.check b/test/files/jvm/varargs.check index 8379befe93..986f98896a 100644 --- a/test/files/jvm/varargs.check +++ b/test/files/jvm/varargs.check @@ -1,3 +1,4 @@ 7 10 -19 \ No newline at end of file +19 +a diff --git a/test/files/jvm/varargs/JavaClass.java b/test/files/jvm/varargs/JavaClass.java index 9851e1b78b..6928ee5adc 100644 --- a/test/files/jvm/varargs/JavaClass.java +++ b/test/files/jvm/varargs/JavaClass.java @@ -11,5 +11,6 @@ public class JavaClass { va.vi(1, 2, 3, 4); varargz(5, 1.0, 2.0, 3.0); va.vt(16, "", "", ""); + System.out.println(va.vt1(16, "a", "b", "c")); } } \ No newline at end of file diff --git a/test/files/jvm/varargs/VaClass.scala b/test/files/jvm/varargs/VaClass.scala index 6343f9c6f6..e94e8a625a 100644 --- a/test/files/jvm/varargs/VaClass.scala +++ b/test/files/jvm/varargs/VaClass.scala @@ -9,5 +9,5 @@ class VaClass { @varargs def vs(a: Int, b: String*) = println(a + b.length) @varargs def vi(a: Int, b: Int*) = println(a + b.sum) @varargs def vt[T](a: Int, b: T*) = println(a + b.length) - + @varargs def vt1[T](a: Int, b: T*): T = b.head } diff --git a/test/files/pos/t8743.scala b/test/files/pos/t8743.scala new file mode 100644 index 0000000000..03b0cd7044 --- /dev/null +++ b/test/files/pos/t8743.scala @@ -0,0 +1,15 @@ +import annotation.varargs + +object VarArgs { + @varargs + def foo[A](x: A, xs: String*): A = ??? + + @varargs + def bar[A](x: List[A], xs: String*): A = ??? + + @varargs + def baz[A](x: List[A], xs: String*): A = ??? + + @varargs + def boz[A](x: A, xs: String*): Nothing = ??? +} -- cgit v1.2.3