From f8647ee2540b2c71f9852d4ce661bffe912e4812 Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Mon, 5 Nov 2012 17:23:23 -0800 Subject: show developer guidelines on opening pull request Includes "Improvements and typo fixes to CONTRIBUTING.md" by @heathercmiller as well as feedback from @retronym. --- CONTRIBUTING.md | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000000..551100ae85 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,66 @@ +# Scala Project & Developer Guidelines + +These guidelines are meant to be a living document that should be changed and adapted as needed. We encourage changes that make it easier to achieve our goals in an efficient way. + +## General Workflow + +This is the process for committing code to the Scala project. There are of course exceptions to these rules, for example minor changes to comments and documentation, fixing a broken build etc. + +1. Make sure you have signed the [Scala CLA](http://www.scala-lang.org/sites/default/files/contributor_agreement.pdf), if not, sign it. +2. Before starting to work on a feature or a fix, it's good practice to ensure that: + 1. There is a ticket for your work in the project's issue tracker. If not, create it first (perhaps given a thumbs up from the scala-internals mailing list first). + 2. The ticket has been discussed and prioritized by the team. +3. You should always perform your work in its own Git branch. The branch should be given a descriptive name that explains its intent. Some teams also like adding the ticket number and/or the [GitHub](http://github.com) user ID to the branch name, these details is up to each of the individual teams. (See below for more details on branch naming.) +4. When the feature or fix is completed you should open a [Pull Request](https://help.github.com/articles/using-pull-requests) on GitHub. +5. The Pull Request should be reviewed by other maintainers (as many as feasible/practical). Note that a reviewer can also be an outside contributor-- members of Typesafe and independent contributors are encouraged to participate in the review process. It is not a closed process. Please try to avoid conflict of interest -- the spirit of the review process is to evenly distribute the understanding of our code base across its maintainers as well as to load balance quality assurance. Assigning a review to a "sure win" reviewer is not a good long-term solution. +6. After the review, you should resolve issues brought up by the reviewers as needed (pushing a new commit to address reviewers' comments), iterating until the reviewers give their thumbs up, the "LGTM" (acronym for "Looks Good To Me"). +7. Once the code has passed review the Pull Request can be merged into the distribution. + +## Pull Request Requirements + +First, please have a look at and follow the [Pull Request Policy](https://github.com/scala/scala/wiki/Pull-Request-Policy) for guidelines on submitting a pull request to the Scala project. + +In order for a Pull Request to be considered, it has to meet these requirements: + +1. Live up to the current code standard: + - Not violate [DRY](http://programmer.97things.oreilly.com/wiki/index.php/Don%27t_Repeat_Yourself). + - [Boy Scout Rule](http://programmer.97things.oreilly.com/wiki/index.php/The_Boy_Scout_Rule) should be applied. +2. Tests are of paramount importance. +3. The code must be well documented in the project's standard documentation format (see the ‘Documentation’ section below). + +If *all* of these requirements are not met then the code should **not** be merged into the distribution, and need not even be reviewed. + +## Documentation + +All contributed code should come accompanied with documentation. Pull requests containing undocumented code will not be accepted. Both user-facing Scaladoc comments, as well as committer-facing internal documentation (i.e. important design decisions that other maintainers should know about should be placed inline with line comments `//`) should be accompanying all contributed code where possible. + + +## Work In Progress + +It is ok to work on a public feature branch in the GitHub repository. Something that can sometimes be useful for early feedback etc. If so, then it is preferable to name the branch accordingly. This can be done by either prefixing the name with ``wip-`` as in ‘Work In Progress’, or use hierarchical names like ``wip/..``, ``feature/..`` or ``topic/..``. Either way is fine as long as it is clear that it is work in progress and not ready for merge. This work can temporarily have a lower standard. However, to be merged into master it will have to go through the regular process outlined above, with Pull Request, review etc.. + +Also, to facilitate both well-formed commits and working together, the ``wip`` and ``feature``/``topic`` identifiers also have special meaning. Any branch labelled with ``wip`` is considered “git-unstable” and may be rebased and have its history rewritten. Any branch with ``feature``/``topic`` in the name is considered “stable” enough for others to depend on when a group is working on a feature. + +## Creating Commits And Writing Commit Messages + +Follow these guidelines when creating public commits and writing commit messages. + +1. If your work spans multiple local commits (for example; if you do safe point commits while working in a feature branch or work in a branch for long time doing merges/rebases etc.) then please do not commit it all but rewrite the history by squashing the commits into one large commit which is accompanied by a detailed commit message for (as discussed in the following sections). For more info, see the article: [Git Workflow](http://sandofsky.com/blog/git-workflow.html). Additionally, every commit should be able to be used in isolation-- that is, each commit must build and pass all tests. +2. The first line should be a descriptive sentence about what the commit is doing. It should be possible to fully understand what the commit does by just reading this single line. It is **not ok** to only list the ticket number, type "minor fix" or similar. If the commit has a corresponding ticket, include a reference to the ticket number, prefixed with "SI-", at the beginning of the first line followed by the title of the ticket, assuming that it aptly and concisely summarizes the commit in a single line. If the commit is a small fix, then you are done. If not, go to 3. +3. Following the single line description (ideally no more than 70 characters long) should be a blank line followed by an enumerated list with the details of the commit. +4. Add keywords for your commit (depending on the degree of automation we reach, the list may change over time): + * ``Review by @githubuser`` - will notify the reviewer via GitHub. Everyone is encouraged to give feedback, however. (Remember that @-mentions will result in notifications also when pushing to a WIP branch, so please only include this in your commit message when you're ready for your pull request to be reviewed. Alternatively, you may request a review in the pull request's description.) + * ``Fix/Fixing/Fixes/Close/Closing/Refs #ticket`` - if you want to mark the ticket as fixed in the issue tracker (Assembla understands this). + * ``backport to _branch name_`` - if the fix needs to be cherry-picked to another branch (like 2.9.x, 2.10.x, etc) + +Example: + + SI-4032 Implicit conversion visibility affected by presence of "this" + + - Details 1 + - Details 2 + - Details 3 + +## The Scala Improvement Process +A new language feature requires a SIP (Scala Improvement Process) proposal. Note that significant additions to the standard library are also considered candidates for a SIP proposal. +For more details on submitting SIPs, see (how to submit a SIP)[http://docs.scala-lang.org/sips/sip-submission.html]. -- cgit v1.2.3 From 925c6e34635a9a8621b20c328670cbd23975327f Mon Sep 17 00:00:00 2001 From: Simon Ochsenreither Date: Fri, 9 Nov 2012 03:18:33 +0100 Subject: SI-6632 SI-6633 Fixes issues and data corruption in ListBuffer - Disallow negative positions for ListBuffer#insert/insertAll/update - Fix data corruption issue in ListBuffer#insert --- .../scala/collection/mutable/ListBuffer.scala | 93 ++++++++++------------ test/files/run/t6632.check | 3 + test/files/run/t6632.scala | 29 +++++++ test/files/run/t6633.check | 3 + test/files/run/t6633.scala | 33 ++++++++ 5 files changed, 112 insertions(+), 49 deletions(-) create mode 100644 test/files/run/t6632.check create mode 100644 test/files/run/t6632.scala create mode 100644 test/files/run/t6633.check create mode 100644 test/files/run/t6633.scala diff --git a/src/library/scala/collection/mutable/ListBuffer.scala b/src/library/scala/collection/mutable/ListBuffer.scala index 53b2b57f50..53501a7267 100644 --- a/src/library/scala/collection/mutable/ListBuffer.scala +++ b/src/library/scala/collection/mutable/ListBuffer.scala @@ -129,29 +129,27 @@ final class ListBuffer[A] * @throws Predef.IndexOutOfBoundsException if `n` is out of bounds. */ def update(n: Int, x: A) { - try { - if (exported) copy() - if (n == 0) { - val newElem = new :: (x, start.tail); - if (last0 eq start) { - last0 = newElem - } - start = newElem - } else { - var cursor = start - var i = 1 - while (i < n) { - cursor = cursor.tail - i += 1 - } - val newElem = new :: (x, cursor.tail.tail) - if (last0 eq cursor.tail) { - last0 = newElem - } - cursor.asInstanceOf[::[A]].tl = newElem + // We check the bounds early, so that we don't trigger copying. + if (n < 0 || n >= len) throw new IndexOutOfBoundsException(n.toString) + if (exported) copy() + if (n == 0) { + val newElem = new :: (x, start.tail); + if (last0 eq start) { + last0 = newElem + } + start = newElem + } else { + var cursor = start + var i = 1 + while (i < n) { + cursor = cursor.tail + i += 1 + } + val newElem = new :: (x, cursor.tail.tail) + if (last0 eq cursor.tail) { + last0 = newElem } - } catch { - case ex: Exception => throw new IndexOutOfBoundsException(n.toString()) + cursor.asInstanceOf[::[A]].tl = newElem } } @@ -212,34 +210,31 @@ final class ListBuffer[A] * @throws Predef.IndexOutOfBoundsException if `n` is out of bounds. */ def insertAll(n: Int, seq: Traversable[A]) { - try { - if (exported) copy() - var elems = seq.toList.reverse - len += elems.length - if (n == 0) { - while (!elems.isEmpty) { - val newElem = new :: (elems.head, start) - if (start.isEmpty) last0 = newElem - start = newElem - elems = elems.tail - } - } else { - var cursor = start - var i = 1 - while (i < n) { - cursor = cursor.tail - i += 1 - } - while (!elems.isEmpty) { - val newElem = new :: (elems.head, cursor.tail) - if (cursor.tail.isEmpty) last0 = newElem - cursor.asInstanceOf[::[A]].tl = newElem - elems = elems.tail - } + // We check the bounds early, so that we don't trigger copying. + if (n < 0 || n > len) throw new IndexOutOfBoundsException(n.toString) + if (exported) copy() + var elems = seq.toList.reverse + len += elems.length + if (n == 0) { + while (!elems.isEmpty) { + val newElem = new :: (elems.head, start) + if (start.isEmpty) last0 = newElem + start = newElem + elems = elems.tail + } + } else { + var cursor = start + var i = 1 + while (i < n) { + cursor = cursor.tail + i += 1 + } + while (!elems.isEmpty) { + val newElem = new :: (elems.head, cursor.tail) + if (cursor.tail.isEmpty) last0 = newElem + cursor.asInstanceOf[::[A]].tl = newElem + elems = elems.tail } - } catch { - case ex: Exception => - throw new IndexOutOfBoundsException(n.toString()) } } diff --git a/test/files/run/t6632.check b/test/files/run/t6632.check new file mode 100644 index 0000000000..1f084b1dac --- /dev/null +++ b/test/files/run/t6632.check @@ -0,0 +1,3 @@ +java.lang.IndexOutOfBoundsException: -1 +java.lang.IndexOutOfBoundsException: -2 +java.lang.IndexOutOfBoundsException: -3 diff --git a/test/files/run/t6632.scala b/test/files/run/t6632.scala new file mode 100644 index 0000000000..c1c8d4abe0 --- /dev/null +++ b/test/files/run/t6632.scala @@ -0,0 +1,29 @@ +object Test extends App { + import collection.mutable.ListBuffer + + def newLB = ListBuffer('a, 'b, 'c, 'd, 'e) + + val lb0 = newLB + + try { + lb0.insert(-1, 'x) + } catch { + case ex: IndexOutOfBoundsException => println(ex) + } + + val lb1 = newLB + + try { + lb1.insertAll(-2, Array('x, 'y, 'z)) + } catch { + case ex: IndexOutOfBoundsException => println(ex) + } + + val lb2 = newLB + + try { + lb2.update(-3, 'u) + } catch { + case ex: IndexOutOfBoundsException => println(ex) + } +} \ No newline at end of file diff --git a/test/files/run/t6633.check b/test/files/run/t6633.check new file mode 100644 index 0000000000..1ff8cdbc44 --- /dev/null +++ b/test/files/run/t6633.check @@ -0,0 +1,3 @@ +java.lang.IndexOutOfBoundsException: 9 +replStringOf OK +length OK diff --git a/test/files/run/t6633.scala b/test/files/run/t6633.scala new file mode 100644 index 0000000000..bd993c8d88 --- /dev/null +++ b/test/files/run/t6633.scala @@ -0,0 +1,33 @@ +object Test extends App { + import collection.mutable.ListBuffer + + def newLB = ListBuffer('a, 'b, 'c, 'd, 'e) + + val lb0 = newLB + + try { + lb0.insert(9, 'x) + } catch { + case ex: IndexOutOfBoundsException => println(ex) + } + + val lb1 = newLB + + try { + lb1.insert(9, 'x) + } catch { + case ex: IndexOutOfBoundsException => + } + + val replStr = scala.runtime.ScalaRunTime.replStringOf(lb1, 100) + if (replStr == "ListBuffer('a, 'b, 'c, 'd, 'e)\n") + println("replStringOf OK") + else + println("replStringOf FAILED: " + replStr) + + val len = lb1.length + if (len == 5) + println("length OK") + else + println("length FAILED: " + len) +} \ No newline at end of file -- cgit v1.2.3 From 0b59b4627a76d99531a51c7f17bbfa8b9c8c4bd8 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Sat, 10 Nov 2012 15:31:43 +0100 Subject: SI-6640 Better reporting of deficient classpaths. In a55788e, StubSymbols were introduced to fail-slow when the classpath was deficient. This allowed compilation to succeed in cases when one didn't actually use the part of class A which referred to some missing class B. But a few problems were introduced. Firstly, when the deferred error eventually happened, it was signalled with abort(msg), rather than through a thrown MissingRequirementError. The latter is desirable, as it doesn't lead to printing a stack trace. Second, the actual error message changed, and no longer included the name of the class file that refers to the missing class. Finally, it seems that we can end up with a stub term symbol in a situation where a class symbol is desired. An assertion in the constructor of ThisType throws trips when calling .isClass, before the useful error message from StubSymbol can be emitted. This commit addresses these points, and rewords the error a little to be more accessible. The last point is the most fragile in this arrangement, there might be some whack-a-mole required to find other places that also need this. I don't see a clean solution for this, but am open to suggestions. We should really build a facility in partest to delete specified classfiles between groups in separate compilation tests, in order to have tests for this. I'll work on that as a followup. For now, here's the result of my manual testing: [info] Set current project to default-821d14 (in build file:/Users/jason/code/scratch1/) > compile [info] Compiling 1 Scala source to /Users/jason/code/scratch1/target/scala-2.10/classes... [error] [error] while compiling: /Users/jason/code/scratch1/test.scala [error] during phase: typer [error] library version: version 2.10.0-RC2 [error] compiler version: version 2.10.0-RC2 ... [error] last tree to typer: Ident(SwingWorker) [error] symbol: (flags: ) [error] symbol definition: [error] symbol owners: [error] context owners: object Test -> package ... [error] uncaught exception during compilation: java.lang.AssertionError [trace] Stack trace suppressed: run last compile:compile for the full output. [error] (compile:compile) java.lang.AssertionError: assertion failed: value actors [error] Total time: 2 s, completed Nov 10, 2012 3:18:34 PM > > set scalaHome := Some(file("/Users/jason/code/scala/build/pack")) [info] Defining *:scala-home [info] The new value will be used by no settings or tasks. [info] Reapplying settings... [info] Set current project to default-821d14 (in build file:/Users/jason/code/scratch1/) ^[compile [info] Compiling 1 Scala source to /Users/jason/code/scratch1/target/scala-2.10/classes... [error] /Users/jason/code/scratch1/test.scala:4: A signature in SwingWorker.class refers to term actors in package scala which is missing from the classpath. [error] object Test extends SwingWorker [error] ^ [error] one error found [error] (compile:compile) Compilation failed [error] Total time: 2 s, completed Nov 10, 2012 3:18:45 PM --- src/reflect/scala/reflect/internal/Symbols.scala | 21 ++++++++------------- src/reflect/scala/reflect/internal/Types.scala | 2 +- .../scala/reflect/internal/pickling/UnPickler.scala | 8 ++------ 3 files changed, 11 insertions(+), 20 deletions(-) diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala index 7cb9a0e105..33d99b35d8 100644 --- a/src/reflect/scala/reflect/internal/Symbols.scala +++ b/src/reflect/scala/reflect/internal/Symbols.scala @@ -423,9 +423,9 @@ trait Symbols extends api.Symbols { self: SymbolTable => * failure to the point when that name is used for something, which is * often to the point of never. */ - def newStubSymbol(name: Name): Symbol = name match { - case n: TypeName => new StubClassSymbol(this, n) - case _ => new StubTermSymbol(this, name.toTermName) + def newStubSymbol(name: Name, missingMessage: String): Symbol = name match { + case n: TypeName => new StubClassSymbol(this, n, missingMessage) + case _ => new StubTermSymbol(this, name.toTermName, missingMessage) } @deprecated("Use the other signature", "2.10.0") @@ -3100,14 +3100,11 @@ trait Symbols extends api.Symbols { self: SymbolTable => ) } trait StubSymbol extends Symbol { - protected def stubWarning = { - val from = if (associatedFile == null) "" else s" - referenced from ${associatedFile.canonicalPath}" - s"$kindString $nameString$locationString$from (a classfile may be missing)" - } + protected def missingMessage: String private def fail[T](alt: T): T = { // Avoid issuing lots of redundant errors if (!hasFlag(IS_ERROR)) { - globalError(s"bad symbolic reference to " + stubWarning) + MissingRequirementError.signal(missingMessage) if (settings.debug.value) (new Throwable).printStackTrace @@ -3124,12 +3121,10 @@ trait Symbols extends api.Symbols { self: SymbolTable => override def rawInfo = fail(NoType) override def companionSymbol = fail(NoSymbol) - locally { - debugwarn("creating stub symbol for " + stubWarning) - } + debugwarn("creating stub symbol to defer error: " + missingMessage) } - class StubClassSymbol(owner0: Symbol, name0: TypeName) extends ClassSymbol(owner0, owner0.pos, name0) with StubSymbol - class StubTermSymbol(owner0: Symbol, name0: TermName) extends TermSymbol(owner0, owner0.pos, name0) with StubSymbol + class StubClassSymbol(owner0: Symbol, name0: TypeName, protected val missingMessage: String) extends ClassSymbol(owner0, owner0.pos, name0) with StubSymbol + class StubTermSymbol(owner0: Symbol, name0: TermName, protected val missingMessage: String) extends TermSymbol(owner0, owner0.pos, name0) with StubSymbol trait FreeSymbol extends Symbol { def origin: String diff --git a/src/reflect/scala/reflect/internal/Types.scala b/src/reflect/scala/reflect/internal/Types.scala index e8054fcdf5..86d03d7450 100644 --- a/src/reflect/scala/reflect/internal/Types.scala +++ b/src/reflect/scala/reflect/internal/Types.scala @@ -1385,7 +1385,7 @@ trait Types extends api.Types { self: SymbolTable => /** A class for this-types of the form .this.type */ abstract case class ThisType(sym: Symbol) extends SingletonType with ThisTypeApi { - assert(sym.isClass, sym) + assert(sym.isClass, {sym.info; sym}) // call .info to allow StubSymbols to reveal what's missing from the classpath //assert(sym.isClass && !sym.isModuleClass || sym.isRoot, sym) override def isTrivial: Boolean = sym.isPackageClass override def isNotNull = true diff --git a/src/reflect/scala/reflect/internal/pickling/UnPickler.scala b/src/reflect/scala/reflect/internal/pickling/UnPickler.scala index 43b982a8a4..15465f919a 100644 --- a/src/reflect/scala/reflect/internal/pickling/UnPickler.scala +++ b/src/reflect/scala/reflect/internal/pickling/UnPickler.scala @@ -233,7 +233,8 @@ abstract class UnPickler /*extends scala.reflect.generic.UnPickler*/ { // (4) Call the mirror's "missing" hook. adjust(mirrorThatLoaded(owner).missingHook(owner, name)) orElse { // (5) Create a stub symbol to defer hard failure a little longer. - owner.newStubSymbol(name) + val missingMessage = s"A signature in $filename refers to ${name.longString} in ${owner.fullLocationString} which is missing from the classpath." + owner.newStubSymbol(name, missingMessage) } } } @@ -827,11 +828,6 @@ abstract class UnPickler /*extends scala.reflect.generic.UnPickler*/ { protected def errorBadSignature(msg: String) = throw new RuntimeException("malformed Scala signature of " + classRoot.name + " at " + readIndex + "; " + msg) - protected def errorMissingRequirement(name: Name, owner: Symbol): Symbol = - mirrorThatLoaded(owner).missingHook(owner, name) orElse MissingRequirementError.signal( - s"bad reference while unpickling $filename: ${name.longString} not found in ${owner.tpe.widen}" - ) - def inferMethodAlternative(fun: Tree, argtpes: List[Type], restpe: Type) {} // can't do it; need a compiler for that. def newLazyTypeRef(i: Int): LazyType = new LazyTypeRef(i) -- cgit v1.2.3 From 86e045e2863b04bf4af4abb5c2ce345bcdae2b80 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Sat, 10 Nov 2012 21:30:00 +0100 Subject: Refine the message and triggering of MissingRequirementError. - To force a failure of the stub, call a new method `failIfStub` rather than `info`. - Offer a broader range of potential root causes in the error message. --- src/reflect/scala/reflect/internal/Symbols.scala | 4 ++++ src/reflect/scala/reflect/internal/Types.scala | 6 +++++- src/reflect/scala/reflect/internal/pickling/UnPickler.scala | 8 ++++++-- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala index 33d99b35d8..67b2d05224 100644 --- a/src/reflect/scala/reflect/internal/Symbols.scala +++ b/src/reflect/scala/reflect/internal/Symbols.scala @@ -1344,6 +1344,9 @@ trait Symbols extends api.Symbols { self: SymbolTable => } } + /** Raises a `MissingRequirementError` if this symbol is a `StubSymbol` */ + def failIfStub() {} + /** Initialize the symbol */ final def initialize: this.type = { if (!isInitialized) info @@ -3100,6 +3103,7 @@ trait Symbols extends api.Symbols { self: SymbolTable => ) } trait StubSymbol extends Symbol { + override final def failIfStub() = fail(()) protected def missingMessage: String private def fail[T](alt: T): T = { // Avoid issuing lots of redundant errors diff --git a/src/reflect/scala/reflect/internal/Types.scala b/src/reflect/scala/reflect/internal/Types.scala index 86d03d7450..57212bab55 100644 --- a/src/reflect/scala/reflect/internal/Types.scala +++ b/src/reflect/scala/reflect/internal/Types.scala @@ -1385,7 +1385,11 @@ trait Types extends api.Types { self: SymbolTable => /** A class for this-types of the form .this.type */ abstract case class ThisType(sym: Symbol) extends SingletonType with ThisTypeApi { - assert(sym.isClass, {sym.info; sym}) // call .info to allow StubSymbols to reveal what's missing from the classpath + // SI-6640 allow StubSymbols to reveal what's missing from the classpath + // before we trip the assertion. + sym.failIfStub() + assert(sym.isClass, sym) + //assert(sym.isClass && !sym.isModuleClass || sym.isRoot, sym) override def isTrivial: Boolean = sym.isPackageClass override def isNotNull = true diff --git a/src/reflect/scala/reflect/internal/pickling/UnPickler.scala b/src/reflect/scala/reflect/internal/pickling/UnPickler.scala index 15465f919a..5464a68205 100644 --- a/src/reflect/scala/reflect/internal/pickling/UnPickler.scala +++ b/src/reflect/scala/reflect/internal/pickling/UnPickler.scala @@ -20,7 +20,7 @@ import scala.annotation.switch /** @author Martin Odersky * @version 1.0 */ -abstract class UnPickler /*extends scala.reflect.generic.UnPickler*/ { +abstract class UnPickler { val global: SymbolTable import global._ @@ -233,7 +233,11 @@ abstract class UnPickler /*extends scala.reflect.generic.UnPickler*/ { // (4) Call the mirror's "missing" hook. adjust(mirrorThatLoaded(owner).missingHook(owner, name)) orElse { // (5) Create a stub symbol to defer hard failure a little longer. - val missingMessage = s"A signature in $filename refers to ${name.longString} in ${owner.fullLocationString} which is missing from the classpath." + val missingMessage = + s"""|A signature in $filename refers to ${name.longString} in ${owner.fullLocationString} + |which is not available. It may be completely missing from the current classpath, + |or the version on the classpath might be incompatible with the version used when + |compiling $filename.""".stripMargin owner.newStubSymbol(name, missingMessage) } } -- cgit v1.2.3 From 079296632d8ef5ecc40aafa83757231599c78783 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Tue, 13 Nov 2012 11:02:30 +0100 Subject: SI-6440 Address regressions around MissingRequirementError Go back to using globalError to report when a stub's info is referenced, and only throw the MissingRequirementError when compilation really must abort due to having a StubTermSymbol in a place where a StubClassSymbol would have been a better choice. This situation arises when an entire package is missing from the classpath, as was the case in the reported bug. Adds `StoreReporterDirectTest`, which buffers messages issued during compilation for more structured interrogation. Use this in two test for manifests -- these tests were using a crude means of grepping compiler console output to focus on the relevant output, but this approach was insufficient with the new multi-line error message emitted as part of this change. Also used that base test class to add two new tests: one for the reported error (package missing), and another for a simpler error (class missing). The latter test shows how stub symbols allow code to compile if it doesn't the subset of signatures in some type that refer to a missing class. Gave the INFO/WARNING/ERROR members of Reporter sensible toString implementations; they inherit from Enumeration#Value in an unusual manner (why?) that means the built in toString of Enumeration printed `Severity@0`. --- .../scala/tools/nsc/reporters/Reporter.scala | 12 +++-- src/partest/scala/tools/partest/DirectTest.scala | 8 ++- .../tools/partest/StoreReporterDirectTest.scala | 15 ++++++ src/reflect/scala/reflect/internal/Symbols.scala | 10 +++- src/reflect/scala/reflect/internal/Types.scala | 9 ++-- .../reflect/internal/pickling/UnPickler.scala | 8 +-- test/files/neg/t5148.check | 10 +++- test/files/run/t6440.check | 5 +- test/files/run/t6440.scala | 47 +++++++++++++++-- test/files/run/t6440b.check | 4 ++ test/files/run/t6440b.scala | 61 ++++++++++++++++++++++ ...tags_without_scala_reflect_typetag_lookup.check | 5 +- ...tags_without_scala_reflect_typetag_lookup.scala | 14 +++-- ...ut_scala_reflect_typetag_manifest_interop.check | 5 +- ...ut_scala_reflect_typetag_manifest_interop.scala | 15 +++--- 15 files changed, 186 insertions(+), 42 deletions(-) create mode 100644 src/partest/scala/tools/partest/StoreReporterDirectTest.scala create mode 100644 test/files/run/t6440b.check create mode 100644 test/files/run/t6440b.scala diff --git a/src/compiler/scala/tools/nsc/reporters/Reporter.scala b/src/compiler/scala/tools/nsc/reporters/Reporter.scala index c5321dd728..8871ae6555 100644 --- a/src/compiler/scala/tools/nsc/reporters/Reporter.scala +++ b/src/compiler/scala/tools/nsc/reporters/Reporter.scala @@ -20,9 +20,15 @@ abstract class Reporter { class Severity(val id: Int) extends severity.Value { var count: Int = 0 } - val INFO = new Severity(0) - val WARNING = new Severity(1) - val ERROR = new Severity(2) + val INFO = new Severity(0) { + override def toString: String = "INFO" + } + val WARNING = new Severity(1) { + override def toString: String = "WARNING" + } + val ERROR = new Severity(2) { + override def toString: String = "ERROR" + } /** Whether very long lines can be truncated. This exists so important * debugging information (like printing the classpath) is not rendered diff --git a/src/partest/scala/tools/partest/DirectTest.scala b/src/partest/scala/tools/partest/DirectTest.scala index 554e7848c7..8c18809ad6 100644 --- a/src/partest/scala/tools/partest/DirectTest.scala +++ b/src/partest/scala/tools/partest/DirectTest.scala @@ -8,6 +8,7 @@ package scala.tools.partest import scala.tools.nsc._ import io.Directory import util.{BatchSourceFile, CommandLineParser} +import reporters.{Reporter, ConsoleReporter} /** A class for testing code which is embedded as a string. * It allows for more complete control over settings, compiler @@ -38,9 +39,12 @@ abstract class DirectTest extends App { // new compiler def newCompiler(args: String*): Global = { val settings = newSettings((CommandLineParser tokenize ("-d \"" + testOutput.path + "\" " + extraSettings)) ++ args.toList) - if (settings.Yrangepos.value) new Global(settings) with interactive.RangePositions - else new Global(settings) + if (settings.Yrangepos.value) new Global(settings, reporter(settings)) with interactive.RangePositions + else new Global(settings, reporter(settings)) } + + def reporter(settings: Settings): Reporter = new ConsoleReporter(settings) + def newSources(sourceCodes: String*) = sourceCodes.toList.zipWithIndex map { case (src, idx) => new BatchSourceFile("newSource" + (idx + 1), src) } diff --git a/src/partest/scala/tools/partest/StoreReporterDirectTest.scala b/src/partest/scala/tools/partest/StoreReporterDirectTest.scala new file mode 100644 index 0000000000..7f3604c86c --- /dev/null +++ b/src/partest/scala/tools/partest/StoreReporterDirectTest.scala @@ -0,0 +1,15 @@ +package scala.tools.partest + +import scala.tools.nsc.Settings +import scala.tools.nsc.reporters.StoreReporter +import scala.collection.mutable + +trait StoreReporterDirectTest extends DirectTest { + lazy val storeReporter: StoreReporter = new scala.tools.nsc.reporters.StoreReporter() + + /** Discards all but the first message issued at a given position. */ + def filteredInfos: Seq[storeReporter.Info] = storeReporter.infos.groupBy(_.pos).map(_._2.head).toList + + /** Hook into [[scala.tools.partest.DirectTest]] to install the custom reporter */ + override def reporter(settings: Settings) = storeReporter +} diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala index 67b2d05224..06bf54c1f8 100644 --- a/src/reflect/scala/reflect/internal/Symbols.scala +++ b/src/reflect/scala/reflect/internal/Symbols.scala @@ -3103,12 +3103,18 @@ trait Symbols extends api.Symbols { self: SymbolTable => ) } trait StubSymbol extends Symbol { - override final def failIfStub() = fail(()) protected def missingMessage: String + + /** Fail the stub by throwing a [[scala.reflect.internal.MissingRequirementError]]. */ + override final def failIfStub() = {MissingRequirementError.signal(missingMessage)} // + + /** Fail the stub by reporting an error to the reporter, setting the IS_ERROR flag + * on this symbol, and returning the dummy value `alt`. + */ private def fail[T](alt: T): T = { // Avoid issuing lots of redundant errors if (!hasFlag(IS_ERROR)) { - MissingRequirementError.signal(missingMessage) + globalError(missingMessage) if (settings.debug.value) (new Throwable).printStackTrace diff --git a/src/reflect/scala/reflect/internal/Types.scala b/src/reflect/scala/reflect/internal/Types.scala index 57212bab55..d5347705a6 100644 --- a/src/reflect/scala/reflect/internal/Types.scala +++ b/src/reflect/scala/reflect/internal/Types.scala @@ -1385,10 +1385,11 @@ trait Types extends api.Types { self: SymbolTable => /** A class for this-types of the form .this.type */ abstract case class ThisType(sym: Symbol) extends SingletonType with ThisTypeApi { - // SI-6640 allow StubSymbols to reveal what's missing from the classpath - // before we trip the assertion. - sym.failIfStub() - assert(sym.isClass, sym) + if (!sym.isClass) { + // SI-6640 allow StubSymbols to reveal what's missing from the classpath before we trip the assertion. + sym.failIfStub() + assert(false, sym) + } //assert(sym.isClass && !sym.isModuleClass || sym.isRoot, sym) override def isTrivial: Boolean = sym.isPackageClass diff --git a/src/reflect/scala/reflect/internal/pickling/UnPickler.scala b/src/reflect/scala/reflect/internal/pickling/UnPickler.scala index 5464a68205..f3a5053a91 100644 --- a/src/reflect/scala/reflect/internal/pickling/UnPickler.scala +++ b/src/reflect/scala/reflect/internal/pickling/UnPickler.scala @@ -234,10 +234,10 @@ abstract class UnPickler { adjust(mirrorThatLoaded(owner).missingHook(owner, name)) orElse { // (5) Create a stub symbol to defer hard failure a little longer. val missingMessage = - s"""|A signature in $filename refers to ${name.longString} in ${owner.fullLocationString} - |which is not available. It may be completely missing from the current classpath, - |or the version on the classpath might be incompatible with the version used when - |compiling $filename.""".stripMargin + s"""|bad symbolic reference. A signature in $filename refers to ${name.longString} + |in ${owner.kindString} ${owner.fullName} which is not available. + |It may be completely missing from the current classpath, or the version on + |the classpath might be incompatible with the version used when compiling $filename.""".stripMargin owner.newStubSymbol(name, missingMessage) } } diff --git a/test/files/neg/t5148.check b/test/files/neg/t5148.check index 6edfdf2b1e..25107c4dbe 100644 --- a/test/files/neg/t5148.check +++ b/test/files/neg/t5148.check @@ -1,3 +1,9 @@ -error: bad symbolic reference to value global in class IMain - referenced from t5148.scala (a classfile may be missing) -error: bad symbolic reference to value memberHandlers in class IMain - referenced from t5148.scala (a classfile may be missing) +error: bad symbolic reference. A signature in Imports.class refers to term global +in class scala.tools.nsc.interpreter.IMain which is not available. +It may be completely missing from the current classpath, or the version on +the classpath might be incompatible with the version used when compiling Imports.class. +error: bad symbolic reference. A signature in Imports.class refers to term memberHandlers +in class scala.tools.nsc.interpreter.IMain which is not available. +It may be completely missing from the current classpath, or the version on +the classpath might be incompatible with the version used when compiling Imports.class. two errors found diff --git a/test/files/run/t6440.check b/test/files/run/t6440.check index b5684daee4..69c253eab4 100644 --- a/test/files/run/t6440.check +++ b/test/files/run/t6440.check @@ -1 +1,4 @@ -Stream((), ?) +pos: source-newSource1,line-9,offset=109 bad symbolic reference. A signature in U.class refers to term pack1 +in package which is not available. +It may be completely missing from the current classpath, or the version on +the classpath might be incompatible with the version used when compiling U.class. ERROR diff --git a/test/files/run/t6440.scala b/test/files/run/t6440.scala index 2b690f31e1..5a3a4150d9 100644 --- a/test/files/run/t6440.scala +++ b/test/files/run/t6440.scala @@ -1,7 +1,48 @@ -object Test { +import scala.tools.partest._ +import java.io.File - def main(args: Array[String]): Unit = { - println(Stream.continually(()).filterNot(_ => false).take(2)) +object Test extends StoreReporterDirectTest { + def code = ??? + + def compileCode(code: String) = { + val classpath = List(sys.props("partest.lib"), testOutput.path) mkString sys.props("path.separator") + compileString(newCompiler("-cp", classpath, "-d", testOutput.path))(code) } + def library1 = """ + package pack1 + trait T + """ + + def library2 = """ + package pack2 + trait U extends pack1.T + """ + + def app = """ + package pack3 + object X { + trait U + } + import X._ + import pack2._ + + trait V extends U + """ + + def show(): Unit = { + Seq(library1, library2) foreach compileCode + assert(filteredInfos.isEmpty, filteredInfos) + + // blow away the entire package + val pack1 = new File(testOutput.path, "pack1") + val tClass = new File(pack1, "T.class") + assert(tClass.exists) + assert(tClass.delete()) + assert(pack1.delete()) + + // bad symbolic reference error expected (but no stack trace!) + compileCode(app) + println(filteredInfos.mkString("\n")) + } } diff --git a/test/files/run/t6440b.check b/test/files/run/t6440b.check new file mode 100644 index 0000000000..9771ce5efb --- /dev/null +++ b/test/files/run/t6440b.check @@ -0,0 +1,4 @@ +pos: NoPosition bad symbolic reference. A signature in U.class refers to type T +in package pack1 which is not available. +It may be completely missing from the current classpath, or the version on +the classpath might be incompatible with the version used when compiling U.class. ERROR diff --git a/test/files/run/t6440b.scala b/test/files/run/t6440b.scala new file mode 100644 index 0000000000..974aca2844 --- /dev/null +++ b/test/files/run/t6440b.scala @@ -0,0 +1,61 @@ +import scala.tools.partest._ +import java.io.File + +object Test extends StoreReporterDirectTest { + def code = ??? + + def compileCode(code: String) = { + val classpath = List(sys.props("partest.lib"), testOutput.path) mkString sys.props("path.separator") + compileString(newCompiler("-cp", classpath, "-d", testOutput.path))(code) + } + + def library1 = """ + package pack1 + trait T + class U { + def t = new T {} + def one = 1 + } + """ + + def library2 = """ + package pack2 + object V { + def u = new pack1.U + } + """ + + def app1 = """ + package pack3 + object Test { + pack2.V.u.one // okay + } + """ + + def app2 = """ + package pack3 + object Test { + pack2.V.u.t // we have to fail if T.class is misisng + } + """ + + def show(): Unit = { + compileCode(library1) + val pack1 = new File(testOutput.path, "pack1") + val tClass = new File(pack1, "T.class") + assert(tClass.exists) + assert(tClass.delete()) + + // allowed to compile, no direct reference to `T` + compileCode(library2) + assert(filteredInfos.isEmpty, filteredInfos) + + // allowed to compile, no direct reference to `T` + compileCode(app1) + assert(filteredInfos.isEmpty, filteredInfos) + + // bad symbolic reference error expected (but no stack trace!) + compileCode(app2) + println(filteredInfos.mkString("\n")) + } +} diff --git a/test/files/run/typetags_without_scala_reflect_typetag_lookup.check b/test/files/run/typetags_without_scala_reflect_typetag_lookup.check index 53df68cfc2..8c558ced60 100644 --- a/test/files/run/typetags_without_scala_reflect_typetag_lookup.check +++ b/test/files/run/typetags_without_scala_reflect_typetag_lookup.check @@ -1,3 +1,2 @@ -newSource1:9: error: could not find implicit value for evidence parameter of type reflect.runtime.package.universe.TypeTag[Int] - Library.foo[Int] - ^ + +pos: source-newSource1,line-9,offset=466 could not find implicit value for evidence parameter of type reflect.runtime.package.universe.TypeTag[Int] ERROR diff --git a/test/files/run/typetags_without_scala_reflect_typetag_lookup.scala b/test/files/run/typetags_without_scala_reflect_typetag_lookup.scala index e51ecdb180..1fbdc62a1e 100644 --- a/test/files/run/typetags_without_scala_reflect_typetag_lookup.scala +++ b/test/files/run/typetags_without_scala_reflect_typetag_lookup.scala @@ -1,6 +1,6 @@ import scala.tools.partest._ -object Test extends DirectTest { +object Test extends StoreReporterDirectTest { def code = ??? def library = """ @@ -32,14 +32,12 @@ object Test extends DirectTest { } def show(): Unit = { - val prevErr = System.err - val baos = new java.io.ByteArrayOutputStream(); - System.setErr(new java.io.PrintStream(baos)); compileLibrary(); + println(filteredInfos.mkString("\n")) + storeReporter.infos.clear() compileApp(); - // we should get bad symbolic reference errors, because we're trying to call a method that can't be unpickled + // we should get bad symbolic reference errors, because we're trying to use an implicit that can't be unpickled // but we don't know the number of these errors and their order, so I just ignore them all - baos.toString.split("\n") filter (!_.startsWith("error: bad symbolic reference")) foreach println - System.setErr(prevErr) + println(filteredInfos.filterNot(_.msg.contains("bad symbolic reference")).mkString("\n")) } -} \ No newline at end of file +} diff --git a/test/files/run/typetags_without_scala_reflect_typetag_manifest_interop.check b/test/files/run/typetags_without_scala_reflect_typetag_manifest_interop.check index 729c0715df..acfecce628 100644 --- a/test/files/run/typetags_without_scala_reflect_typetag_manifest_interop.check +++ b/test/files/run/typetags_without_scala_reflect_typetag_manifest_interop.check @@ -1,3 +1,2 @@ -newSource1:9: error: No Manifest available for App.this.T. - manifest[T] - ^ + +pos: source-newSource1,line-9,offset=479 No Manifest available for App.this.T. ERROR diff --git a/test/files/run/typetags_without_scala_reflect_typetag_manifest_interop.scala b/test/files/run/typetags_without_scala_reflect_typetag_manifest_interop.scala index e984127583..6804baa0c3 100644 --- a/test/files/run/typetags_without_scala_reflect_typetag_manifest_interop.scala +++ b/test/files/run/typetags_without_scala_reflect_typetag_manifest_interop.scala @@ -1,6 +1,7 @@ import scala.tools.partest._ +import scala.tools.nsc.Settings -object Test extends DirectTest { +object Test extends StoreReporterDirectTest { def code = ??? def library = """ @@ -29,18 +30,18 @@ object Test extends DirectTest { """ def compileApp() = { val classpath = List(sys.props("partest.lib"), testOutput.path) mkString sys.props("path.separator") + val global = newCompiler("-cp", classpath, "-d", testOutput.path) compileString(newCompiler("-cp", classpath, "-d", testOutput.path))(app) + //global.reporter.ERROR.foreach(println) } def show(): Unit = { - val prevErr = System.err - val baos = new java.io.ByteArrayOutputStream(); - System.setErr(new java.io.PrintStream(baos)); compileLibrary(); + println(filteredInfos.mkString("\n")) + storeReporter.infos.clear() compileApp(); // we should get bad symbolic reference errors, because we're trying to use an implicit that can't be unpickled // but we don't know the number of these errors and their order, so I just ignore them all - baos.toString.split("\n") filter (!_.startsWith("error: bad symbolic reference")) foreach println - System.setErr(prevErr) + println(filteredInfos.filterNot (_.msg.contains("bad symbolic reference")).mkString("\n")) } -} \ No newline at end of file +} -- cgit v1.2.3 From 2e0cbe0aa276720ceaf54a3448f7e04558e255b8 Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Tue, 13 Nov 2012 16:57:33 +0100 Subject: sane printing of renamed imports Having a select named "foo" with an underlying symbol named "bar" and trying to make sense of all that by prettyprinting is very confusing --- src/reflect/scala/reflect/internal/Printers.scala | 7 +++++-- test/files/run/showraw_aliases.check | 2 ++ test/files/run/showraw_aliases.scala | 15 +++++++++++++++ 3 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 test/files/run/showraw_aliases.check create mode 100644 test/files/run/showraw_aliases.scala diff --git a/src/reflect/scala/reflect/internal/Printers.scala b/src/reflect/scala/reflect/internal/Printers.scala index e513ccb32c..80d247c0ea 100644 --- a/src/reflect/scala/reflect/internal/Printers.scala +++ b/src/reflect/scala/reflect/internal/Printers.scala @@ -561,8 +561,11 @@ trait Printers extends api.Printers { self: SymbolTable => if (isError) print(": error>") } else if (hasSymbol) { tree match { - case _: Ident | _: Select | _: SelectFromTypeTree => print(tree.symbol) - case _ => print(tree.symbol.name) + case refTree: RefTree => + if (tree.symbol.name != refTree.name) print("[", tree.symbol, " aka ", refTree.name, "]") + else print(tree.symbol) + case _ => + print(tree.symbol.name) } } else { print(name) diff --git a/test/files/run/showraw_aliases.check b/test/files/run/showraw_aliases.check new file mode 100644 index 0000000000..1838bf9bec --- /dev/null +++ b/test/files/run/showraw_aliases.check @@ -0,0 +1,2 @@ +Block(List(Import(Select(Select(Ident(scala), scala.reflect), scala.reflect.runtime), List(ImportSelector(newTermName("universe"), 52, newTermName("ru"), 64)))), Select(Select(Select(Select(Ident(scala), scala.reflect), scala.reflect.runtime), scala.reflect.runtime.package), [newTermName("universe") aka newTermName("ru")])) +Block(List(Import(Select(Select(Ident(scala#), scala.reflect#), scala.reflect.runtime#), List(ImportSelector(newTermName("universe"), 52, newTermName("ru"), 64)))), Select(Select(Select(Select(Ident(scala#), scala.reflect#), scala.reflect.runtime#), scala.reflect.runtime.package#), [newTermName("universe")# aka newTermName("ru")])) diff --git a/test/files/run/showraw_aliases.scala b/test/files/run/showraw_aliases.scala new file mode 100644 index 0000000000..3a68ca37b2 --- /dev/null +++ b/test/files/run/showraw_aliases.scala @@ -0,0 +1,15 @@ +import scala.reflect.runtime.universe._ +import scala.tools.reflect.ToolBox + +object Test extends App { + val tb = runtimeMirror(getClass.getClassLoader).mkToolBox() + val tree = tb.parse(""" + import scala.reflect.runtime.{universe => ru} + ru + """) + val ttree = tb.typeCheck(tree) + + def stabilize(s: String) = """#\d+""".r.replaceAllIn(s, "#") + println(showRaw(ttree)) + println(stabilize(showRaw(ttree, printIds = true))) +} \ No newline at end of file -- cgit v1.2.3 From d08060ee326345970cd1dbfbb73d227105e35d67 Mon Sep 17 00:00:00 2001 From: Julien Richard-Foy Date: Fri, 10 Aug 2012 00:34:39 +0200 Subject: Fix raw string interpolator: string parts which were after the first argument were still escaped --- src/library/scala/StringContext.scala | 2 +- test/files/run/rawstrings.check | 2 +- test/files/run/rawstrings.scala | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/library/scala/StringContext.scala b/src/library/scala/StringContext.scala index f72547724d..8be0cb1619 100644 --- a/src/library/scala/StringContext.scala +++ b/src/library/scala/StringContext.scala @@ -120,7 +120,7 @@ case class StringContext(parts: String*) { val bldr = new java.lang.StringBuilder(process(pi.next())) while (ai.hasNext) { bldr append ai.next - bldr append treatEscapes(pi.next()) + bldr append process(pi.next()) } bldr.toString } diff --git a/test/files/run/rawstrings.check b/test/files/run/rawstrings.check index 36e63594df..2b6c40725a 100644 --- a/test/files/run/rawstrings.check +++ b/test/files/run/rawstrings.check @@ -1 +1 @@ -[\n\t'"$] +[\n\t'"$\n] diff --git a/test/files/run/rawstrings.scala b/test/files/run/rawstrings.scala index 9df64f6625..b4d6e0c40a 100644 --- a/test/files/run/rawstrings.scala +++ b/test/files/run/rawstrings.scala @@ -1,3 +1,3 @@ object Test extends App { - println(raw"[\n\t'${'"'}$$]") + println(raw"[\n\t'${'"'}$$\n]") } -- cgit v1.2.3 From 1587a77eca46b265dc677b9be2536f1f60503f65 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Sun, 11 Nov 2012 14:02:24 +0100 Subject: SI-6648 copyAttrs must preserve TypeTree#wasEmpty This field tracks whether the type is an inferred on, subject to removal in `resetAttrs`, or an explicit type, which must remain. In ae5ff662, `ResetAttrs` was modified to duplicate trees, rather than mutate trees in place. But the tree copier didn't pass `wasEmpty` on to the new tree, which in turn meant that the subsequent typing run on the tree would not re-infer the types. If the type refers to a local class, e.g. the anonymous function in the enclosed test case, the reference to the old symbol would persist. This commit overrides `copyAttrs` in TypeTree to copy `wasEmpty`. We might consider representing this as a tree attachment, but this would need to be validated for the performance impact. --- src/reflect/scala/reflect/internal/Trees.scala | 13 +++++++++++++ test/files/pos/t6648.scala | 24 ++++++++++++++++++++++++ 2 files changed, 37 insertions(+) create mode 100644 test/files/pos/t6648.scala diff --git a/src/reflect/scala/reflect/internal/Trees.scala b/src/reflect/scala/reflect/internal/Trees.scala index 53b40da8f6..b8f5e73acb 100644 --- a/src/reflect/scala/reflect/internal/Trees.scala +++ b/src/reflect/scala/reflect/internal/Trees.scala @@ -482,6 +482,10 @@ trait Trees extends api.Trees { self: SymbolTable => case class TypeTree() extends TypTree with TypeTreeContextApi { private var orig: Tree = null + /** Was this type tree originally empty? That is, does it now contain + * an inferred type that must be forgotten in `resetAttrs` to + * enable retyping. + */ private[scala] var wasEmpty: Boolean = false override def symbol = typeTreeSymbol(this) // if (tpe == null) null else tpe.typeSymbol @@ -502,6 +506,15 @@ trait Trees extends api.Trees { self: SymbolTable => wasEmpty = isEmpty setType(tp) } + + override private[scala] def copyAttrs(tree: Tree) = { + super.copyAttrs(tree) + tree match { + case other: TypeTree => wasEmpty = other.wasEmpty // SI-6648 Critical for correct operation of `resetAttrs`. + case _ => + } + this + } } object TypeTree extends TypeTreeExtractor diff --git a/test/files/pos/t6648.scala b/test/files/pos/t6648.scala new file mode 100644 index 0000000000..9593ebfee9 --- /dev/null +++ b/test/files/pos/t6648.scala @@ -0,0 +1,24 @@ +abstract class Node extends NodeSeq +trait NodeSeq extends Seq[Node] +object NodeSeq { + implicit def seqToNodeSeq(ns: Seq[Node]): NodeSeq = ??? + def foo[B, That](f: Seq[B])(implicit bf: scala.collection.generic.CanBuildFrom[Seq[Int], B, That]): That = ??? +} + +class Transformer { + def apply(nodes: Any): Any = ??? +} + +object transformer1 extends Transformer { + // Adding explicit type arguments, or making the impilcit view + // seqToNodeSeq explicit avoids the crash + NodeSeq.foo { + // These both avoid the crash: + // val t = new Transformer {}; t.apply(null) + // new Transformer().apply(null) + new Transformer {}.apply(null) + + null: NodeSeq + }: NodeSeq +} + -- cgit v1.2.3 From 88b4b91b0a9191c98dc3f80c1a2cbea1a5ff4e12 Mon Sep 17 00:00:00 2001 From: Josh Suereth Date: Wed, 24 Oct 2012 10:59:50 -0400 Subject: Fixes SI-6559 - StringContext not using passed in escape function. As reported by Curtis Stanford, with indication of what to fix. standardInterpolator was not correctly calling the passed in process function, so raw strings were not really raw. --- test/files/run/t6559.scala | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 test/files/run/t6559.scala diff --git a/test/files/run/t6559.scala b/test/files/run/t6559.scala new file mode 100644 index 0000000000..5c671f7275 --- /dev/null +++ b/test/files/run/t6559.scala @@ -0,0 +1,17 @@ + +object Test { + + def main(args: Array[String]) = { + val one = "1" + val two = "2" + + val raw = raw"\n$one\n$two\n" + val escaped = s"\n$one\n$two\n" + val buggy = "\\n1\n2\n" + val correct = "\\n1\\n2\\n" + + assert(raw != escaped, "Raw strings should not be escaped.") + assert(raw != buggy, "Raw strings after variables should not be escaped.") + assert(raw == correct, "Raw strings should stay raw.") + } +} -- cgit v1.2.3 From 65778d760fd7b80b8f0fb9e3cfe87cc87e3523ae Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Wed, 14 Nov 2012 11:24:54 -0800 Subject: SI-5330, SI-6014 deal with existential self-type This has been broken since https://github.com/scala/scala/commit/b7b81ca2#L0L567. The existential rash is treated in a similar manner as in fc24db4c. Conceptually, the fix would be `def selfTypeSkolemized = widen.skolemizeExistential.narrow`, but simply widening before narrowing achieves the same thing. Since we're in existential voodoo territory, let's go for the minimal fix: replacing `this.narrow` by `widen.narrow`. -- Original patch by @retronym in #1074, refined by @paulp to only perform widen.narrow incantation if there are existentials present in the widened type, as narrowing is expensive when the type is not a singleton. The result is that compiling the entirety of quick, that code path is hit only 143 times. All the other calls hit .narrow directly as before. It looks like the definition of negligible in the diff of -Ystatistics when compiling src/library/scala/collection: < #symbols : 306315 --- > #symbols : 306320 12c13 < #unique types : 293859 --- > #unique types : 293865 I'm assuming based on the 2/1000ths of a percent increase in symbol and type creation that wall clock is manageable, but I didn't measure it. --- src/reflect/scala/reflect/internal/Types.scala | 18 +++++++++++++++--- test/files/pos/t5330.scala | 22 ++++++++++++++++++++++ test/files/pos/t5330b.scala | 6 ++++++ test/files/pos/t5330c.scala | 5 +++++ test/files/pos/t6014.scala | 13 +++++++++++++ 5 files changed, 61 insertions(+), 3 deletions(-) create mode 100644 test/files/pos/t5330.scala create mode 100644 test/files/pos/t5330b.scala create mode 100644 test/files/pos/t5330c.scala create mode 100644 test/files/pos/t6014.scala diff --git a/src/reflect/scala/reflect/internal/Types.scala b/src/reflect/scala/reflect/internal/Types.scala index de6c6285ca..6df6ed4417 100644 --- a/src/reflect/scala/reflect/internal/Types.scala +++ b/src/reflect/scala/reflect/internal/Types.scala @@ -325,6 +325,18 @@ trait Types extends api.Types { self: SymbolTable => } } + /** Same as a call to narrow unless existentials are visible + * after widening the type. In that case, narrow from the widened + * type instead of the proxy. This gives buried existentials a + * chance to make peace with the other types. See SI-5330. + */ + private def narrowForFindMember(tp: Type): Type = { + val w = tp.widen + // Only narrow on widened type when we have to -- narrow is expensive unless the target is a singleton type. + if ((tp ne w) && containsExistential(w)) w.narrow + else tp.narrow + } + /** The base class for all types */ abstract class Type extends TypeApiImpl with Annotatable[Type] { /** Types for which asSeenFrom always is the identity, no matter what @@ -1079,7 +1091,7 @@ trait Types extends api.Types { self: SymbolTable => (other ne sym) && ((other.owner eq sym.owner) || (flags & PRIVATE) != 0 || { - if (self eq null) self = this.narrow + if (self eq null) self = narrowForFindMember(this) if (symtpe eq null) symtpe = self.memberType(sym) !(self.memberType(other) matches symtpe) })}) { @@ -1161,7 +1173,7 @@ trait Types extends api.Types { self: SymbolTable => if ((member ne sym) && ((member.owner eq sym.owner) || (flags & PRIVATE) != 0 || { - if (self eq null) self = this.narrow + if (self eq null) self = narrowForFindMember(this) if (membertpe eq null) membertpe = self.memberType(member) !(membertpe matches self.memberType(sym)) })) { @@ -1176,7 +1188,7 @@ trait Types extends api.Types { self: SymbolTable => (other ne sym) && ((other.owner eq sym.owner) || (flags & PRIVATE) != 0 || { - if (self eq null) self = this.narrow + if (self eq null) self = narrowForFindMember(this) if (symtpe eq null) symtpe = self.memberType(sym) !(self.memberType(other) matches symtpe) })}) { diff --git a/test/files/pos/t5330.scala b/test/files/pos/t5330.scala new file mode 100644 index 0000000000..813acd4b83 --- /dev/null +++ b/test/files/pos/t5330.scala @@ -0,0 +1,22 @@ +trait FM[A] { + def map(f: A => Any) +} + +trait M[A] extends FM[A] { + def map(f: A => Any) +} + +trait N[A] extends FM[A] + +object test { + def kaboom(xs: M[_]) = xs map (x => ()) // missing parameter type. + + def okay1[A](xs: M[A]) = xs map (x => ()) + def okay2(xs: FM[_]) = xs map (x => ()) + def okay3(xs: N[_]) = xs map (x => ()) +} + +class CC2(xs: List[_]) { + def f(x1: Any, x2: Any) = null + def g = xs map (x => f(x, x)) +} diff --git a/test/files/pos/t5330b.scala b/test/files/pos/t5330b.scala new file mode 100644 index 0000000000..dbeb165cd8 --- /dev/null +++ b/test/files/pos/t5330b.scala @@ -0,0 +1,6 @@ +abstract trait Base { + def foo: this.type +}; +class Derived[T] extends Base { + def foo: Nothing = sys.error("!!!") +} diff --git a/test/files/pos/t5330c.scala b/test/files/pos/t5330c.scala new file mode 100644 index 0000000000..af31f3dfd1 --- /dev/null +++ b/test/files/pos/t5330c.scala @@ -0,0 +1,5 @@ +object t5330c { + val s: Set[_ >: Char] = Set('A') + s forall ("ABC" contains _) + s.forall( c => "ABC".toSeq.contains( c )) +} diff --git a/test/files/pos/t6014.scala b/test/files/pos/t6014.scala new file mode 100644 index 0000000000..46e03bb552 --- /dev/null +++ b/test/files/pos/t6014.scala @@ -0,0 +1,13 @@ +object Test { + case class CC[T](key: T) + type Alias[T] = Seq[CC[T]] + + def f(xs: Seq[CC[_]]) = xs map { case CC(x) => CC(x) } // ok + def g(xs: Alias[_]) = xs map { case CC(x) => CC(x) } // fails + // ./a.scala:11: error: missing parameter type for expanded function + // The argument types of an anonymous function must be fully known. (SLS 8.5) + // Expected type was: ? + // def g(xs: Alias[_]) = xs map { case CC(x) => CC(x) } // fails + // ^ + // one error found +} \ No newline at end of file -- cgit v1.2.3 From 1f0e4880ad7ad816fd82c04f6814c5b165f86981 Mon Sep 17 00:00:00 2001 From: Aleksandar Prokopec Date: Wed, 14 Nov 2012 20:42:24 +0100 Subject: Fixes SI-6150 - backport to 2.10.x branch. --- src/library/scala/collection/IndexedSeq.scala | 10 ++++- .../collection/generic/GenTraversableFactory.scala | 9 ++--- .../collection/generic/IndexedSeqFactory.scala | 21 +++++++++++ .../scala/collection/immutable/IndexedSeq.scala | 5 ++- .../scala/collection/immutable/Vector.scala | 33 ++++++---------- test/files/run/t6150.scala | 44 ++++++++++++++++++++++ 6 files changed, 91 insertions(+), 31 deletions(-) create mode 100644 src/library/scala/collection/generic/IndexedSeqFactory.scala create mode 100644 test/files/run/t6150.scala diff --git a/src/library/scala/collection/IndexedSeq.scala b/src/library/scala/collection/IndexedSeq.scala index e30d2f07bb..63e5adf428 100644 --- a/src/library/scala/collection/IndexedSeq.scala +++ b/src/library/scala/collection/IndexedSeq.scala @@ -28,8 +28,14 @@ trait IndexedSeq[+A] extends Seq[A] * @define coll indexed sequence * @define Coll `IndexedSeq` */ -object IndexedSeq extends SeqFactory[IndexedSeq] { - implicit def canBuildFrom[A]: CanBuildFrom[Coll, A, IndexedSeq[A]] = ReusableCBF.asInstanceOf[GenericCanBuildFrom[A]] +object IndexedSeq extends IndexedSeqFactory[IndexedSeq] { + // A single CBF which can be checked against to identify + // an indexed collection type. + override val ReusableCBF: GenericCanBuildFrom[Nothing] = new GenericCanBuildFrom[Nothing] { + override def apply() = newBuilder[Nothing] + } def newBuilder[A]: Builder[A, IndexedSeq[A]] = immutable.IndexedSeq.newBuilder[A] + implicit def canBuildFrom[A]: CanBuildFrom[Coll, A, IndexedSeq[A]] = + ReusableCBF.asInstanceOf[GenericCanBuildFrom[A]] } diff --git a/src/library/scala/collection/generic/GenTraversableFactory.scala b/src/library/scala/collection/generic/GenTraversableFactory.scala index d5edf5961e..a43862abaf 100644 --- a/src/library/scala/collection/generic/GenTraversableFactory.scala +++ b/src/library/scala/collection/generic/GenTraversableFactory.scala @@ -36,15 +36,12 @@ import scala.language.higherKinds * @see GenericCanBuildFrom */ abstract class GenTraversableFactory[CC[X] <: GenTraversable[X] with GenericTraversableTemplate[X, CC]] - extends GenericCompanion[CC] { +extends GenericCompanion[CC] { - // A default implementation of GenericCanBuildFrom which can be cast - // to whatever is desired. - private class ReusableCBF extends GenericCanBuildFrom[Nothing] { + private[this] val ReusableCBFInstance: GenericCanBuildFrom[Nothing] = new GenericCanBuildFrom[Nothing] { override def apply() = newBuilder[Nothing] } - // Working around SI-4789 by using a lazy val instead of an object. - lazy val ReusableCBF: GenericCanBuildFrom[Nothing] = new ReusableCBF + def ReusableCBF: GenericCanBuildFrom[Nothing] = ReusableCBFInstance /** A generic implementation of the `CanBuildFrom` trait, which forwards * all calls to `apply(from)` to the `genericBuilder` method of diff --git a/src/library/scala/collection/generic/IndexedSeqFactory.scala b/src/library/scala/collection/generic/IndexedSeqFactory.scala new file mode 100644 index 0000000000..200d033c2d --- /dev/null +++ b/src/library/scala/collection/generic/IndexedSeqFactory.scala @@ -0,0 +1,21 @@ +/* __ *\ +** ________ ___ / / ___ Scala API ** +** / __/ __// _ | / / / _ | (c) 2003-2011, LAMP/EPFL ** +** __\ \/ /__/ __ |/ /__/ __ | http://scala-lang.org/ ** +** /____/\___/_/ |_/____/_/ | | ** +** |/ ** +\* */ + +package scala.collection +package generic + +import language.higherKinds + +/** A template for companion objects of IndexedSeq and subclasses thereof. + * + * @since 2.10 + */ +abstract class IndexedSeqFactory[CC[X] <: IndexedSeq[X] with GenericTraversableTemplate[X, CC]] extends SeqFactory[CC] { + override def ReusableCBF: GenericCanBuildFrom[Nothing] = + scala.collection.IndexedSeq.ReusableCBF.asInstanceOf[GenericCanBuildFrom[Nothing]] +} diff --git a/src/library/scala/collection/immutable/IndexedSeq.scala b/src/library/scala/collection/immutable/IndexedSeq.scala index e98df46c9b..bf4ba3a381 100644 --- a/src/library/scala/collection/immutable/IndexedSeq.scala +++ b/src/library/scala/collection/immutable/IndexedSeq.scala @@ -31,11 +31,12 @@ trait IndexedSeq[+A] extends Seq[A] * @define coll indexed sequence * @define Coll `IndexedSeq` */ -object IndexedSeq extends SeqFactory[IndexedSeq] { +object IndexedSeq extends IndexedSeqFactory[IndexedSeq] { class Impl[A](buf: ArrayBuffer[A]) extends AbstractSeq[A] with IndexedSeq[A] with Serializable { def length = buf.length def apply(idx: Int) = buf.apply(idx) } - implicit def canBuildFrom[A]: CanBuildFrom[Coll, A, IndexedSeq[A]] = ReusableCBF.asInstanceOf[GenericCanBuildFrom[A]] def newBuilder[A]: Builder[A, IndexedSeq[A]] = Vector.newBuilder[A] + implicit def canBuildFrom[A]: CanBuildFrom[Coll, A, IndexedSeq[A]] = + ReusableCBF.asInstanceOf[GenericCanBuildFrom[A]] } diff --git a/src/library/scala/collection/immutable/Vector.scala b/src/library/scala/collection/immutable/Vector.scala index dff221ad05..f083e80175 100644 --- a/src/library/scala/collection/immutable/Vector.scala +++ b/src/library/scala/collection/immutable/Vector.scala @@ -18,16 +18,10 @@ import scala.collection.parallel.immutable.ParVector /** Companion object to the Vector class */ -object Vector extends SeqFactory[Vector] { - private[collection] class VectorReusableCBF extends GenericCanBuildFrom[Nothing] { - override def apply() = newBuilder[Nothing] - } - - private val VectorReusableCBF: GenericCanBuildFrom[Nothing] = new VectorReusableCBF - - implicit def canBuildFrom[A]: CanBuildFrom[Coll, A, Vector[A]] = - VectorReusableCBF.asInstanceOf[CanBuildFrom[Coll, A, Vector[A]]] +object Vector extends IndexedSeqFactory[Vector] { def newBuilder[A]: Builder[A, Vector[A]] = new VectorBuilder[A] + implicit def canBuildFrom[A]: CanBuildFrom[Coll, A, Vector[A]] = + ReusableCBF.asInstanceOf[GenericCanBuildFrom[A]] private[immutable] val NIL = new Vector[Nothing](0, 0, 0) override def empty[A]: Vector[A] = NIL } @@ -137,20 +131,17 @@ override def companion: GenericCompanion[Vector] = Vector // SeqLike api - override def updated[B >: A, That](index: Int, elem: B)(implicit bf: CanBuildFrom[Vector[A], B, That]): That = bf match { - case _: Vector.VectorReusableCBF => updateAt(index, elem).asInstanceOf[That] // just ignore bf - case _ => super.updated(index, elem)(bf) - } + override def updated[B >: A, That](index: Int, elem: B)(implicit bf: CanBuildFrom[Vector[A], B, That]): That = + if (bf eq IndexedSeq.ReusableCBF) updateAt(index, elem).asInstanceOf[That] // just ignore bf + else super.updated(index, elem)(bf) - override def +:[B >: A, That](elem: B)(implicit bf: CanBuildFrom[Vector[A], B, That]): That = bf match { - case _: Vector.VectorReusableCBF => appendFront(elem).asInstanceOf[That] // just ignore bf - case _ => super.+:(elem)(bf) - } + override def +:[B >: A, That](elem: B)(implicit bf: CanBuildFrom[Vector[A], B, That]): That = + if (bf eq IndexedSeq.ReusableCBF) appendFront(elem).asInstanceOf[That] // just ignore bf + else super.+:(elem)(bf) - override def :+[B >: A, That](elem: B)(implicit bf: CanBuildFrom[Vector[A], B, That]): That = bf match { - case _: Vector.VectorReusableCBF => appendBack(elem).asInstanceOf[That] // just ignore bf - case _ => super.:+(elem)(bf) - } + override def :+[B >: A, That](elem: B)(implicit bf: CanBuildFrom[Vector[A], B, That]): That = + if (bf eq IndexedSeq.ReusableCBF) appendBack(elem).asInstanceOf[That] // just ignore bf + else super.:+(elem)(bf) override def take(n: Int): Vector[A] = { if (n <= 0) diff --git a/test/files/run/t6150.scala b/test/files/run/t6150.scala new file mode 100644 index 0000000000..bd8af5d460 --- /dev/null +++ b/test/files/run/t6150.scala @@ -0,0 +1,44 @@ + + + + +object Test { + import collection.{ immutable, mutable, generic } + def TheOneTrueCBF = collection.IndexedSeq.ReusableCBF + + val cbf1 = implicitly[generic.CanBuildFrom[immutable.Vector[Int], Int, collection.IndexedSeq[Int]]] + val cbf2 = implicitly[generic.CanBuildFrom[immutable.IndexedSeq[Int], Int, collection.IndexedSeq[Int]]] + val cbf3 = implicitly[generic.CanBuildFrom[collection.IndexedSeq[Int], Int, collection.IndexedSeq[Int]]] + + val cbf4 = implicitly[generic.CanBuildFrom[immutable.Vector[Int], Int, immutable.IndexedSeq[Int]]] + val cbf5 = implicitly[generic.CanBuildFrom[immutable.Vector[Int], Int, immutable.Vector[Int]]] + val cbf6 = implicitly[generic.CanBuildFrom[immutable.IndexedSeq[Int], Int, immutable.IndexedSeq[Int]]] + + def check[C](v: C) = { + assert(v == Vector(1, 2, 3, 4)) + assert(v.isInstanceOf[Vector[_]]) + } + def checkRealMccoy(x: AnyRef) = { + assert(x eq TheOneTrueCBF, cbf1) + } + + val v = immutable.Vector(1, 2, 3) + val iiv: immutable.IndexedSeq[Int] = immutable.Vector(1, 2, 3) + val iv: IndexedSeq[Int] = immutable.Vector(1, 2, 3) + + def main(args: Array[String]): Unit = { + List(cbf1, cbf2, cbf3, cbf4, cbf5, cbf6) foreach checkRealMccoy + check(v.:+(4)(cbf1)) + check(v.:+(4)(cbf2)) + check(v.:+(4)(cbf3)) + + check(iiv.:+(4)(cbf2)) + check(iiv.:+(4)(cbf3)) + + check(iv.:+(4)(cbf3)) + } +} + + + + -- cgit v1.2.3 From ed7bd017852cf5e98d1a8089d0a0f6aefc0d4f68 Mon Sep 17 00:00:00 2001 From: phaller Date: Tue, 13 Nov 2012 17:45:11 +0100 Subject: SI-6661 - Remove obsolete implicit parameter of scala.concurrent.promise method Clarification of @heathermiller: This is an inconsistency introduced after refactoring implicit ExecutionContexts. In commit 1dfce90246f7d334 the implicit ExecutionContexts were removed from everything else in Promise.scala, but it appears that method promise was missed in the scala.concurrent package object, which would've made sense to remove back then. --- src/library/scala/concurrent/package.scala | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/library/scala/concurrent/package.scala b/src/library/scala/concurrent/package.scala index f7c732b851..3e849f1722 100644 --- a/src/library/scala/concurrent/package.scala +++ b/src/library/scala/concurrent/package.scala @@ -29,13 +29,12 @@ package object concurrent { */ def future[T](body: =>T)(implicit execctx: ExecutionContext): Future[T] = Future[T](body) - /** Creates a promise object which can be completed with a value. + /** Creates a promise object which can be completed with a value or an exception. * * @tparam T the type of the value in the promise - * @param execctx the execution context on which the promise is created on * @return the newly created `Promise` object */ - def promise[T]()(implicit execctx: ExecutionContext): Promise[T] = Promise[T]() + def promise[T](): Promise[T] = Promise[T]() /** Used to designate a piece of code which potentially blocks, allowing the current [[BlockContext]] to adjust * the runtime's behavior. -- cgit v1.2.3 From 16b5a7e722bc9272815006b9a0629733d455d039 Mon Sep 17 00:00:00 2001 From: Paul Phillips Date: Thu, 15 Nov 2012 09:26:16 -0800 Subject: Fixes SI-6628, Revert "Fix for view isEmpty." This reverts commit caf7eb6b56817fd1e1fbc1cf017f30e6f94c6bea. I don't have a better idea right now than wholesale reversion. --- src/library/scala/collection/IterableLike.scala | 3 +-- src/library/scala/collection/SeqLike.scala | 1 - src/library/scala/collection/TraversableLike.scala | 5 ++-- .../scala/collection/TraversableViewLike.scala | 13 ++-------- .../scala/collection/immutable/Stream.scala | 1 - .../scala/collection/mutable/IndexedSeqLike.scala | 1 - .../collection/parallel/ParIterableLike.scala | 29 +++++++++++----------- .../scala/collection/parallel/ParSeqLike.scala | 3 +-- test/files/run/t6628.check | 2 ++ test/files/run/t6628.scala | 11 ++++++++ 10 files changed, 33 insertions(+), 36 deletions(-) create mode 100644 test/files/run/t6628.check create mode 100644 test/files/run/t6628.scala diff --git a/src/library/scala/collection/IterableLike.scala b/src/library/scala/collection/IterableLike.scala index 8d1fa80815..540bd84b79 100644 --- a/src/library/scala/collection/IterableLike.scala +++ b/src/library/scala/collection/IterableLike.scala @@ -171,7 +171,7 @@ self => * fewer elements than size. */ def sliding(size: Int): Iterator[Repr] = sliding(size, 1) - + /** Groups elements in fixed size blocks by passing a "sliding window" * over them (as opposed to partitioning them, as is done in grouped.) * @see [[scala.collection.Iterator]], method `sliding` @@ -293,7 +293,6 @@ self => override /*TraversableLike*/ def view = new IterableView[A, Repr] { protected lazy val underlying = self.repr - override def isEmpty = self.isEmpty override def iterator = self.iterator } diff --git a/src/library/scala/collection/SeqLike.scala b/src/library/scala/collection/SeqLike.scala index 0ffd99c88c..6871a3cb73 100644 --- a/src/library/scala/collection/SeqLike.scala +++ b/src/library/scala/collection/SeqLike.scala @@ -630,7 +630,6 @@ trait SeqLike[+A, +Repr] extends Any with IterableLike[A, Repr] with GenSeqLike[ override def view = new SeqView[A, Repr] { protected lazy val underlying = self.repr - override def isEmpty = self.isEmpty override def iterator = self.iterator override def length = self.length override def apply(idx: Int) = self.apply(idx) diff --git a/src/library/scala/collection/TraversableLike.scala b/src/library/scala/collection/TraversableLike.scala index fc7202b96b..5f193eb211 100644 --- a/src/library/scala/collection/TraversableLike.scala +++ b/src/library/scala/collection/TraversableLike.scala @@ -86,7 +86,7 @@ trait TraversableLike[+A, +Repr] extends Any def repr: Repr = this.asInstanceOf[Repr] final def isTraversableAgain: Boolean = true - + /** The underlying collection seen as an instance of `$Coll`. * By default this is implemented as the current collection object itself, * but this can be overridden. @@ -174,7 +174,7 @@ trait TraversableLike[+A, +Repr] extends Any * * @usecase def ++:[B](that: TraversableOnce[B]): $Coll[B] * @inheritdoc - * + * * Example: * {{{ * scala> val x = List(1) @@ -660,7 +660,6 @@ trait TraversableLike[+A, +Repr] extends Any def view = new TraversableView[A, Repr] { protected lazy val underlying = self.repr override def foreach[U](f: A => U) = self foreach f - override def isEmpty = self.isEmpty } /** Creates a non-strict view of a slice of this $coll. diff --git a/src/library/scala/collection/TraversableViewLike.scala b/src/library/scala/collection/TraversableViewLike.scala index 0925fe4770..14f865c2f0 100644 --- a/src/library/scala/collection/TraversableViewLike.scala +++ b/src/library/scala/collection/TraversableViewLike.scala @@ -59,7 +59,7 @@ trait ViewMkString[+A] { * $viewInfo * * All views for traversable collections are defined by creating a new `foreach` method. - * + * * @author Martin Odersky * @version 2.8 * @since 2.8 @@ -162,7 +162,7 @@ trait TraversableViewLike[+A, // if (b.isInstanceOf[NoBuilder[_]]) newFlatMapped(f).asInstanceOf[That] // else super.flatMap[B, That](f)(bf) } - override def flatten[B](implicit asTraversable: A => /*<: /*<: Boolean): (This, This) = (newTakenWhile(p), newDroppedWhile(p)) override def splitAt(n: Int): (This, This) = (newTaken(n), newDropped(n)) - // Without this, isEmpty tests go back to the Traversable default, which - // involves starting a foreach, which can force the first element of the - // view. This is just a backstop - it's overridden at all the "def view" - // instantiation points in the collections where the Coll type is known. - override def isEmpty = underlying match { - case x: GenTraversableOnce[_] => x.isEmpty - case _ => super.isEmpty - } - override def scanLeft[B, That](z: B)(op: (B, A) => B)(implicit bf: CanBuildFrom[This, B, That]): That = newForced(thisSeq.scanLeft(z)(op)).asInstanceOf[That] diff --git a/src/library/scala/collection/immutable/Stream.scala b/src/library/scala/collection/immutable/Stream.scala index 426ab6f0fb..be2cd91c68 100644 --- a/src/library/scala/collection/immutable/Stream.scala +++ b/src/library/scala/collection/immutable/Stream.scala @@ -937,7 +937,6 @@ self => override def view = new StreamView[A, Stream[A]] { protected lazy val underlying = self.repr - override def isEmpty = self.isEmpty override def iterator = self.iterator override def length = self.length override def apply(idx: Int) = self.apply(idx) diff --git a/src/library/scala/collection/mutable/IndexedSeqLike.scala b/src/library/scala/collection/mutable/IndexedSeqLike.scala index 21cff70473..f0c31ec7fb 100644 --- a/src/library/scala/collection/mutable/IndexedSeqLike.scala +++ b/src/library/scala/collection/mutable/IndexedSeqLike.scala @@ -53,7 +53,6 @@ trait IndexedSeqLike[A, +Repr] extends Any with scala.collection.IndexedSeqLike[ */ override def view = new IndexedSeqView[A, Repr] { protected lazy val underlying = self.repr - override def isEmpty = self.isEmpty override def iterator = self.iterator override def length = self.length override def apply(idx: Int) = self.apply(idx) diff --git a/src/library/scala/collection/parallel/ParIterableLike.scala b/src/library/scala/collection/parallel/ParIterableLike.scala index 9825587b0e..0f06ff37af 100644 --- a/src/library/scala/collection/parallel/ParIterableLike.scala +++ b/src/library/scala/collection/parallel/ParIterableLike.scala @@ -171,9 +171,9 @@ self: ParIterableLike[T, Repr, Sequential] => /** The task support object which is responsible for scheduling and * load-balancing tasks to processors. - * + * * @see [[scala.collection.parallel.TaskSupport]] - */ + */ def tasksupport = { val ts = _tasksupport if (ts eq null) { @@ -188,18 +188,18 @@ self: ParIterableLike[T, Repr, Sequential] => * A task support object can be changed in a parallel collection after it * has been created, but only during a quiescent period, i.e. while there * are no concurrent invocations to parallel collection methods. - * - * Here is a way to change the task support of a parallel collection: - * - * {{{ - * import scala.collection.parallel._ - * val pc = mutable.ParArray(1, 2, 3) - * pc.tasksupport = new ForkJoinTaskSupport( - * new scala.concurrent.forkjoin.ForkJoinPool(2)) - * }}} + * + * Here is a way to change the task support of a parallel collection: + * + * {{{ + * import scala.collection.parallel._ + * val pc = mutable.ParArray(1, 2, 3) + * pc.tasksupport = new ForkJoinTaskSupport( + * new scala.concurrent.forkjoin.ForkJoinPool(2)) + * }}} * * @see [[scala.collection.parallel.TaskSupport]] - */ + */ def tasksupport_=(ts: TaskSupport) = _tasksupport = ts def seq: Sequential @@ -848,7 +848,6 @@ self: ParIterableLike[T, Repr, Sequential] => override def seq = self.seq.view def splitter = self.splitter def size = splitter.remaining - override def isEmpty = size == 0 } override def toArray[U >: T: ClassTag]: Array[U] = { @@ -878,13 +877,13 @@ self: ParIterableLike[T, Repr, Sequential] => override def toSet[U >: T]: immutable.ParSet[U] = toParCollection[U, immutable.ParSet[U]](() => immutable.ParSet.newCombiner[U]) override def toMap[K, V](implicit ev: T <:< (K, V)): immutable.ParMap[K, V] = toParMap[K, V, immutable.ParMap[K, V]](() => immutable.ParMap.newCombiner[K, V]) - + override def toVector: Vector[T] = to[Vector] override def to[Col[_]](implicit cbf: CanBuildFrom[Nothing, T, Col[T @uncheckedVariance]]): Col[T @uncheckedVariance] = if (cbf().isCombiner) { toParCollection[T, Col[T]](() => cbf().asCombiner) } else seq.to(cbf) - + /* tasks */ protected trait StrictSplitterCheckTask[R, Tp] extends Task[R, Tp] { diff --git a/src/library/scala/collection/parallel/ParSeqLike.scala b/src/library/scala/collection/parallel/ParSeqLike.scala index 4f1c3fa7a2..201b624c72 100644 --- a/src/library/scala/collection/parallel/ParSeqLike.scala +++ b/src/library/scala/collection/parallel/ParSeqLike.scala @@ -44,7 +44,7 @@ trait ParSeqLike[+T, +Repr <: ParSeq[T], +Sequential <: Seq[T] with SeqLike[T, S extends scala.collection.GenSeqLike[T, Repr] with ParIterableLike[T, Repr, Sequential] { self => - + type SuperParIterator = IterableSplitter[T] /** A more refined version of the iterator found in the `ParallelIterable` trait, @@ -330,7 +330,6 @@ self => def apply(idx: Int) = self(idx) override def seq = self.seq.view def splitter = self.splitter - override def isEmpty = size == 0 } /* tasks */ diff --git a/test/files/run/t6628.check b/test/files/run/t6628.check new file mode 100644 index 0000000000..bb101b641b --- /dev/null +++ b/test/files/run/t6628.check @@ -0,0 +1,2 @@ +true +true diff --git a/test/files/run/t6628.scala b/test/files/run/t6628.scala new file mode 100644 index 0000000000..84524a7a35 --- /dev/null +++ b/test/files/run/t6628.scala @@ -0,0 +1,11 @@ +object Test { + def coll = new Traversable[String] { + override def foreach[U](f:String=>U) { f("1") } + } + val dropped = coll.view drop 1 + + def main(args: Array[String]): Unit = { + println(dropped.isEmpty) + println(dropped.force.isEmpty) + } +} -- cgit v1.2.3 From 2c23acf39e810fd43f9ce5af0a4c3e4d952f2081 Mon Sep 17 00:00:00 2001 From: Simon Ochsenreither Date: Fri, 16 Nov 2012 01:18:08 +0100 Subject: SI-6634 Fixes data corruption issue in ListBuffer#remove This is the cut-down version with minimally invasive changes, e. g. keeping the "auto-correcting" bounds algorithm. --- .../scala/collection/mutable/ListBuffer.scala | 5 ++ test/files/run/t6634.check | 31 +++++++++ test/files/run/t6634.scala | 80 ++++++++++++++++++++++ 3 files changed, 116 insertions(+) create mode 100644 test/files/run/t6634.check create mode 100644 test/files/run/t6634.scala diff --git a/src/library/scala/collection/mutable/ListBuffer.scala b/src/library/scala/collection/mutable/ListBuffer.scala index 53b2b57f50..4715b6818a 100644 --- a/src/library/scala/collection/mutable/ListBuffer.scala +++ b/src/library/scala/collection/mutable/ListBuffer.scala @@ -249,7 +249,12 @@ final class ListBuffer[A] * @param n the index which refers to the first element to remove. * @param count the number of elements to remove. */ + @annotation.migration("Invalid input values will be rejected in future releases.", "2.11") override def remove(n: Int, count: Int) { + if (n >= len) + return + if (count < 0) + throw new IllegalArgumentException(s"removing negative number ($count) of elements") if (exported) copy() val n1 = n max 0 val count1 = count min (len - n1) diff --git a/test/files/run/t6634.check b/test/files/run/t6634.check new file mode 100644 index 0000000000..f6cbb30c67 --- /dev/null +++ b/test/files/run/t6634.check @@ -0,0 +1,31 @@ +Trying lb0 ... +Checking ... +String OK. +Length OK. + +Trying lb1 ... +Checking ... +String OK. +Length OK. + +Trying lb2 ... +Checking ... +String OK. +Length OK. + +Trying lb3 ... +Checking ... +String OK. +Length OK. + +Trying lb4 ... +Checking ... +String OK. +Length OK. + +Trying lb5 ... +java.lang.IllegalArgumentException: removing negative number (-1) of elements +Checking ... +String OK. +Length OK. + diff --git a/test/files/run/t6634.scala b/test/files/run/t6634.scala new file mode 100644 index 0000000000..759e6d519d --- /dev/null +++ b/test/files/run/t6634.scala @@ -0,0 +1,80 @@ +import collection.mutable.ListBuffer + +object Test extends App { + def newLB = ListBuffer('a, 'b, 'c, 'd, 'e) + + val lb0 = newLB + println("Trying lb0 ...") + try { + lb0.remove(5, 0) + } catch { + // Not thrown in 2.10, will be thrown in 2.11 + case ex: IndexOutOfBoundsException => println(ex) + } + checkNotCorrupted(lb0) + + val lb1 = newLB + println("Trying lb1 ...") + try { + lb1.remove(6, 6) + } catch { + // Not thrown in 2.10, will be thrown in 2.11 + case ex: IndexOutOfBoundsException => println(ex) + } + checkNotCorrupted(lb1) + + val lb2 = newLB + println("Trying lb2 ...") + try { + lb2.remove(99, 6) + } catch { + // Not thrown in 2.10, will be thrown in 2.11 + case ex: IndexOutOfBoundsException => println(ex) + } + checkNotCorrupted(lb2) + + val lb3 = newLB + println("Trying lb3 ...") + try { + lb3.remove(1, 9) + } catch { + // Not thrown in 2.10, will be thrown in 2.11 + case ex: IllegalArgumentException => println(ex) + } + checkNotCorrupted(lb3, "ListBuffer('a)", 1) + + val lb4 = newLB + println("Trying lb4 ...") + try { + lb4.remove(-1, 1) + } catch { + // Not thrown in 2.10, will be thrown in 2.11 + case ex: IndexOutOfBoundsException => println(ex) + } + checkNotCorrupted(lb4, "ListBuffer('b, 'c, 'd, 'e)", 4) + + val lb5 = newLB + println("Trying lb5 ...") + try { + lb5.remove(1, -1) + } catch { + case ex: IllegalArgumentException => println(ex) + } + checkNotCorrupted(lb5) + + // buffer should neither be changed nor corrupted after calling remove with invalid arguments + def checkNotCorrupted( + lb: ListBuffer[Symbol], + expectedString: String = "ListBuffer('a, 'b, 'c, 'd, 'e)", + expectedLength: Int = 5) = { + println("Checking ...") + val replStr = scala.runtime.ScalaRunTime.replStringOf(lb, 100) + if (replStr == expectedString + "\n") println("String OK.") + else println("!!! replStringOf FAILED: " + replStr) + + val len = lb.length + if (len == expectedLength) println("Length OK.") + else println("!!! length FAILED: " + len) + println() + } +} \ No newline at end of file -- cgit v1.2.3 From 74ca558412b6ef9a8694a6f9d1034b9e09ba4af0 Mon Sep 17 00:00:00 2001 From: Nada Amin Date: Sun, 21 Oct 2012 16:12:00 +0200 Subject: SI-6551: don't insert apply call in polymorphic expression. Don't rewrite an explicit apply method to dynamic polytypes. --- src/compiler/scala/tools/nsc/typechecker/Typers.scala | 9 +++++---- test/files/pos/t6551.scala | 13 +++++++++++++ 2 files changed, 18 insertions(+), 4 deletions(-) create mode 100644 test/files/pos/t6551.scala diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index d3847de894..c1fd008e62 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -1100,6 +1100,10 @@ trait Typers extends Modes with Adaptations with Tags { instantiateToMethodType(mt) case _ => + def shouldInsertApply(tree: Tree) = inAllModes(mode, EXPRmode | FUNmode) && (tree.tpe match { + case _: MethodType | _: OverloadedType | _: PolyType => false + case _ => applyPossible + }) def applyPossible = { def applyMeth = member(adaptToName(tree, nme.apply), nme.apply) dyna.acceptsApplyDynamic(tree.tpe) || ( @@ -1117,10 +1121,7 @@ trait Typers extends Modes with Adaptations with Tags { macroExpand(this, tree, mode, pt) else if ((mode & (PATTERNmode | FUNmode)) == (PATTERNmode | FUNmode)) adaptConstrPattern() - else if (inAllModes(mode, EXPRmode | FUNmode) && - !tree.tpe.isInstanceOf[MethodType] && - !tree.tpe.isInstanceOf[OverloadedType] && - applyPossible) + else if (shouldInsertApply(tree)) insertApply() else if (!context.undetparams.isEmpty && !inPolyMode(mode)) { // (9) assert(!inHKMode(mode), modeString(mode)) //@M diff --git a/test/files/pos/t6551.scala b/test/files/pos/t6551.scala new file mode 100644 index 0000000000..fb68663809 --- /dev/null +++ b/test/files/pos/t6551.scala @@ -0,0 +1,13 @@ +import language.dynamics + +object Test { + def main(args: Array[String]) { + class Lenser[T] extends Dynamic { + def selectDynamic(propName: String) = ??? + } + + def lens[T] = new Lenser[T] + + val qq = lens[String] + } +} -- cgit v1.2.3 From c6569209dab006e74ccecc0ede6ce7815ac8629c Mon Sep 17 00:00:00 2001 From: Jan Niehusmann Date: Tue, 13 Nov 2012 22:14:34 +0100 Subject: SI-6663: don't ignore type parameter on selectDynamic invocation Fix mkInvoke to handle selectDynamic calls of the form new C.foo[T].xyz or new C.foo[T].xyz :U (where C extends Dynamic) Without this patch, the type parameter was silently ignored, and possibly inferred to a different. This patch fixes mkInvoke to handle these cases, where ctxTree has the form Select(TypeApply(fun, targs), nme) or Typed(...) --- src/compiler/scala/tools/nsc/typechecker/Typers.scala | 7 ++++++- test/files/neg/t6663.check | 6 ++++++ test/files/neg/t6663.scala | 19 +++++++++++++++++++ test/files/run/t6663.check | 1 + test/files/run/t6663.scala | 17 +++++++++++++++++ 5 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 test/files/neg/t6663.check create mode 100644 test/files/neg/t6663.scala create mode 100644 test/files/run/t6663.check create mode 100644 test/files/run/t6663.scala diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index d3847de894..80785cee2f 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -3935,9 +3935,14 @@ trait Typers extends Modes with Adaptations with Tags { case t: ValOrDefDef => t.rhs case t => t } - val (outer, explicitTargs) = cxTree1 match { + val cxTree2 = cxTree1 match { + case Typed(t, tpe) => t // ignore outer type annotation + case t => t + } + val (outer, explicitTargs) = cxTree2 match { case TypeApply(fun, targs) => (fun, targs) case Apply(TypeApply(fun, targs), args) => (Apply(fun, args), targs) + case Select(TypeApply(fun, targs), nme) => (Select(fun, nme), targs) case t => (t, Nil) } def hasNamedArg(as: List[Tree]) = as.collectFirst{case AssignOrNamedArg(lhs, rhs) =>}.nonEmpty diff --git a/test/files/neg/t6663.check b/test/files/neg/t6663.check new file mode 100644 index 0000000000..aa4faa4a46 --- /dev/null +++ b/test/files/neg/t6663.check @@ -0,0 +1,6 @@ +t6663.scala:16: error: type mismatch; + found : String + required: Int + var v = new C(42).foo[String].get :Int + ^ +one error found diff --git a/test/files/neg/t6663.scala b/test/files/neg/t6663.scala new file mode 100644 index 0000000000..4a358dfbc5 --- /dev/null +++ b/test/files/neg/t6663.scala @@ -0,0 +1,19 @@ +import language.dynamics + +class C(v: Any) extends Dynamic { + def selectDynamic[T](n: String): Option[T] = Option(v.asInstanceOf[T]) + def applyDynamic[T](n: String)(): Option[T] = Option(v.asInstanceOf[T]) +} + +object Test extends App { + // this should be converted to + // C(42).selectDynamic[String]("foo").get + // causing a compile error. + + // but, before fixing SI-6663, became + // C(42).selectDynamic("foo").get, ignoring + // the [String] type parameter + var v = new C(42).foo[String].get :Int + println(v) +} + diff --git a/test/files/run/t6663.check b/test/files/run/t6663.check new file mode 100644 index 0000000000..d81cc0710e --- /dev/null +++ b/test/files/run/t6663.check @@ -0,0 +1 @@ +42 diff --git a/test/files/run/t6663.scala b/test/files/run/t6663.scala new file mode 100644 index 0000000000..6818d286d9 --- /dev/null +++ b/test/files/run/t6663.scala @@ -0,0 +1,17 @@ +import language.dynamics + +class C(v: Any) extends Dynamic { + def selectDynamic[T](n: String): Option[T] = Option(v.asInstanceOf[T]) + def applyDynamic[T](n: String)(): Option[T] = Option(v.asInstanceOf[T]) +} + +object Test extends App { + // this should be converted to + // C(42).selectDynamic[Int]("foo").get + // but, before fixing SI-6663, became + // C(42).selectDynamic[Nothing]("foo").get + // leading to a ClassCastException + var v = new C(42).foo[Int].get + println(v) +} + -- cgit v1.2.3 From af8b45fe354adb03d9b7a9012fb8885bffdd1a45 Mon Sep 17 00:00:00 2001 From: Simon Schaefer Date: Thu, 15 Nov 2012 01:39:49 +0100 Subject: Scaladoc update for collection.mutable.MultiMap Addition of source code example on how to use a MultiMap and its defined methods. Minor correction in documentation for method `removeBinding`. --- .../scala/collection/mutable/MultiMap.scala | 35 ++++++++++++++++++++-- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/src/library/scala/collection/mutable/MultiMap.scala b/src/library/scala/collection/mutable/MultiMap.scala index 31c8a50a84..4635bfbe64 100644 --- a/src/library/scala/collection/mutable/MultiMap.scala +++ b/src/library/scala/collection/mutable/MultiMap.scala @@ -15,8 +15,36 @@ package mutable /** A trait for mutable maps with multiple values assigned to a key. * * This class is typically used as a mixin. It turns maps which map `A` - * to `Set[B]` objects into multi maps which map `A` to - * `B` objects. + * to `Set[B]` objects into multimaps that map `A` to `B` objects. + * + * @example {{{ + * // first import all necessary types from package `collection.mutable` + * import collection.mutable.{ HashMap, MultiMap, Set } + * + * // to create a `MultiMap` the easiest way is to mixin it into a normal + * // `Map` instance + * val mm = new HashMap[Int, Set[String]] with MultiMap[Int, String] + * + * // to add key-value pairs to a multimap it is important to use + * // the method `addBinding` because standard methods like `+` will + * // overwrite the complete key-value pair instead of adding the + * // value to the existing key + * mm.addBinding(1, "a") + * mm.addBinding(2, "b") + * mm.addBinding(1, "c") + * + * // mm now contains `Map(2 -> Set(b), 1 -> Set(c, a))` + * + * // to check if the multimap contains a value there is method + * // `entryExists`, which allows to traverse the including set + * mm.entryExists(1, _ == "a") == true + * mm.entryExists(1, _ == "b") == false + * mm.entryExists(2, _ == "b") == true + * + * // to remove a previous added value there is the method `removeBinding` + * mm.removeBinding(1, "a") + * mm.entryExists(1, _ == "a") == false + * }}} * * @define coll multimap * @define Coll `MultiMap` @@ -57,7 +85,8 @@ trait MultiMap[A, B] extends Map[A, Set[B]] { this } - /** Removes the binding of `value` to `key` if it exists. + /** Removes the binding of `value` to `key` if it exists, otherwise this + * operation doesn't have any effect. * * If this was the last value assigned to the specified key, the * set assigned to that key will be removed as well. -- cgit v1.2.3 From 32c57329a6cb7ff1fa479a1fce884b8166f3fc50 Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Thu, 15 Nov 2012 16:48:03 -0800 Subject: SI-6624 set info of case pattern binder to help find case field accessors sometimes the type checker infers a weird type for a sub-pattern of a case class/extractor pattern this confuses the pattern matcher and it can't find the case field accessors for the sub-pattern use the expected argument type of the extractor corresponding to the case class that we're matching as the info for the sub-pattern binder -- this type more readily admits querying its caseFieldAccessors --- .../tools/nsc/typechecker/PatternMatching.scala | 12 +++++++++- test/files/pos/t6624.scala | 28 ++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 test/files/pos/t6624.scala diff --git a/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala b/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala index 5beba77155..938866deef 100644 --- a/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala +++ b/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala @@ -417,7 +417,17 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL // (the prefix of the argument passed to the unapply must equal the prefix of the type of the binder) val treeMaker = TypeTestTreeMaker(patBinder, patBinder, extractor.paramType, extractor.paramType)(pos, extractorArgTypeTest = true) (List(treeMaker), treeMaker.nextBinder) - } else (Nil, patBinder) + } else { + // no type test needed, but the tree maker relies on `patBinderOrCasted` having type `extractor.paramType` (and not just some type compatible with it) + // SI-6624 shows this is necessary because apparently patBinder may have an unfortunate type (.decls don't have the case field accessors) + // TODO: get to the bottom of this -- I assume it happens when type checking infers a weird type for an unapply call + // by going back to the parameterType for the extractor call we get a saner type, so let's just do that for now + /* TODO: uncomment when `settings.developer` and `devWarning` become available + if (settings.developer.value && !(patBinder.info =:= extractor.paramType)) + devWarning(s"resetting info of $patBinder: ${patBinder.info} to ${extractor.paramType}") + */ + (Nil, patBinder setInfo extractor.paramType) + } withSubPats(typeTestTreeMaker :+ extractor.treeMaker(patBinderOrCasted, pos), extractor.subBindersAndPatterns: _*) } diff --git a/test/files/pos/t6624.scala b/test/files/pos/t6624.scala new file mode 100644 index 0000000000..1a92b92d53 --- /dev/null +++ b/test/files/pos/t6624.scala @@ -0,0 +1,28 @@ +sealed trait KList[+M[_]] + +case class KCons[M[_], +T <: KList[M]]( + tail: T +) extends KList[M] + +case class KNil[M[_]]() extends KList[M] + +object Test { + val klist: KCons[Option, KCons[Option, KCons[Option, KNil[Nothing]]]] = ??? + + // crashes with + // "Exception in thread "main" scala.reflect.internal.Types$TypeError: value _1 is not a member + // of KCons[Option,KCons[Option,KNil[Nothing]]]" + klist match { + case KCons(KCons(KCons(_))) => + } + + // fails with a similar message as an error, rather than a crash. + klist match { + case KCons(KCons(_)) => + } + + // succeeds + klist match { + case KCons(_) => + } +} \ No newline at end of file -- cgit v1.2.3 From db0bf8f406c35ecee038e297dc58b109e0baef04 Mon Sep 17 00:00:00 2001 From: Eugene Vigdorchik Date: Sun, 18 Nov 2012 19:26:52 +0400 Subject: Restore the opimization apparently lost after merge. --- .../scala/tools/nsc/doc/model/ModelFactory.scala | 26 ++++++---------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/src/compiler/scala/tools/nsc/doc/model/ModelFactory.scala b/src/compiler/scala/tools/nsc/doc/model/ModelFactory.scala index 8dd7e7801e..3ae1210ebf 100644 --- a/src/compiler/scala/tools/nsc/doc/model/ModelFactory.scala +++ b/src/compiler/scala/tools/nsc/doc/model/ModelFactory.scala @@ -493,28 +493,16 @@ class ModelFactory(val global: Global, val settings: doc.Settings) { def inheritanceDiagram = makeInheritanceDiagram(this) def contentDiagram = makeContentDiagram(this) - def groupSearch[T](extractor: Comment => T, default: T): T = { - // query this template - if (comment.isDefined) { - val entity = extractor(comment.get) - if (entity != default) return entity + def groupSearch[T](extractor: Comment => Option[T]): Option[T] = { + val comments = comment +: linearizationTemplates.collect { case dtpl: DocTemplateImpl => dtpl.comment } + comments.flatten.map(extractor).flatten.headOption orElse { + Option(inTpl) flatMap (_.groupSearch(extractor)) } - // query linearization - if (!sym.isPackage) - for (tpl <- linearizationTemplates.collect{ case dtpl: DocTemplateImpl if dtpl!=this => dtpl}) { - val entity = tpl.groupSearch(extractor, default) - if (entity != default) return entity - } - // query inTpl, going up the ownerChain - if (inTpl != null) - inTpl.groupSearch(extractor, default) - else - default } - def groupDescription(group: String): Option[Body] = groupSearch(_.groupDesc.get(group), if (group == defaultGroup) defaultGroupDesc else None) - def groupPriority(group: String): Int = groupSearch(_.groupPrio.get(group) match { case Some(prio) => prio; case _ => 0 }, if (group == defaultGroup) defaultGroupPriority else 0) - def groupName(group: String): String = groupSearch(_.groupNames.get(group) match { case Some(name) => name; case _ => group }, if (group == defaultGroup) defaultGroupName else group) + def groupDescription(group: String): Option[Body] = groupSearch(_.groupDesc.get(group)) orElse { if (group == defaultGroup) defaultGroupDesc else None } + def groupPriority(group: String): Int = groupSearch(_.groupPrio.get(group)) getOrElse { if (group == defaultGroup) defaultGroupPriority else 0 } + def groupName(group: String): String = groupSearch(_.groupNames.get(group)) getOrElse { if (group == defaultGroup) defaultGroupName else group } } abstract class PackageImpl(sym: Symbol, inTpl: PackageImpl) extends DocTemplateImpl(sym, inTpl) with Package { -- cgit v1.2.3 From 907d6ea06ee2e2116dc24838b73990dca3d4c651 Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Fri, 16 Nov 2012 14:13:33 +0100 Subject: SI-6673 fixes macro problems with eta expansions Eta expansions previously caused the typer to disable macros. That was done in order to detect eta expansion of macro defs and show the user an appropriate error message. Macros were disabled because to find out whether we're expanding a macro def, we need to get its symbol, and to get a symbol of something we need to typecheck that something. However typechecking automatically expands macros, so, unless we disable macros, after a typecheck we won't be able to analyze macro occurrences anymore. Unfortunately this solution has a fatal flaw. By disabling macros we not only prevent the eta-expandee from macro expanding, but also all the subtrees of that eta-expandee (see SI-6673). This commit adds a mechanism for fine-grained control over macro expansion. Now it's possible to prohibit only the node, but not its children from macro expanding. --- src/compiler/scala/tools/nsc/typechecker/Typers.scala | 9 +++++---- src/reflect/scala/reflect/internal/StdAttachments.scala | 2 ++ test/files/run/t6673.check | 1 + test/files/run/t6673.scala | 5 +++++ 4 files changed, 13 insertions(+), 4 deletions(-) create mode 100644 test/files/run/t6673.check create mode 100644 test/files/run/t6673.scala diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index df3731794a..789211fa98 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -1113,7 +1113,8 @@ trait Typers extends Modes with Adaptations with Tags { adaptType() else if ( inExprModeButNot(mode, FUNmode) && !tree.isDef && // typechecking application - tree.symbol != null && tree.symbol.isTermMacro) // of a macro + tree.symbol != null && tree.symbol.isTermMacro && // of a macro + !tree.attachments.get[SuppressMacroExpansionAttachment.type].isDefined) macroExpand(this, tree, mode, pt) else if ((mode & (PATTERNmode | FUNmode)) == (PATTERNmode | FUNmode)) adaptConstrPattern() @@ -5215,9 +5216,9 @@ trait Typers extends Modes with Adaptations with Tags { // find out whether the programmer is trying to eta-expand a macro def // to do that we need to typecheck the tree first (we need a symbol of the eta-expandee) // that typecheck must not trigger macro expansions, so we explicitly prohibit them - // Q: "but, " - you may ask - ", `typed1` doesn't call adapt, which does macro expansion, so why explicit check?" - // A: solely for robustness reasons. this mechanism might change in the future, which might break unprotected code - val exprTyped = context.withMacrosDisabled(typed1(expr, mode, pt)) + // however we cannot do `context.withMacrosDisabled` + // because `expr` might contain nested macro calls (see SI-6673) + val exprTyped = typed1(expr updateAttachment SuppressMacroExpansionAttachment, mode, pt) exprTyped match { case macroDef if macroDef.symbol != null && macroDef.symbol.isTermMacro && !macroDef.symbol.isErroneous => MacroEtaError(exprTyped) diff --git a/src/reflect/scala/reflect/internal/StdAttachments.scala b/src/reflect/scala/reflect/internal/StdAttachments.scala index 9fe443bf50..c1ed33ee77 100644 --- a/src/reflect/scala/reflect/internal/StdAttachments.scala +++ b/src/reflect/scala/reflect/internal/StdAttachments.scala @@ -24,4 +24,6 @@ trait StdAttachments { case class CompoundTypeTreeOriginalAttachment(parents: List[Tree], stats: List[Tree]) case class MacroExpansionAttachment(original: Tree) + + case object SuppressMacroExpansionAttachment } diff --git a/test/files/run/t6673.check b/test/files/run/t6673.check new file mode 100644 index 0000000000..ef2aa551dc --- /dev/null +++ b/test/files/run/t6673.check @@ -0,0 +1 @@ +List(x) diff --git a/test/files/run/t6673.scala b/test/files/run/t6673.scala new file mode 100644 index 0000000000..115bbdf234 --- /dev/null +++ b/test/files/run/t6673.scala @@ -0,0 +1,5 @@ +object Test extends App { + def foo(f: String => Array[String])(s: String) = f(s) + val test = foo(Array(_)) _ + println(test("x").toList) +} \ No newline at end of file -- cgit v1.2.3 From 1fd3a2a289dae54840b091b822c29019d2ccb565 Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Sun, 18 Nov 2012 21:31:28 +0100 Subject: adds comments to standard attachments --- src/reflect/scala/reflect/internal/StdAttachments.scala | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/reflect/scala/reflect/internal/StdAttachments.scala b/src/reflect/scala/reflect/internal/StdAttachments.scala index c1ed33ee77..1df91a67b0 100644 --- a/src/reflect/scala/reflect/internal/StdAttachments.scala +++ b/src/reflect/scala/reflect/internal/StdAttachments.scala @@ -19,11 +19,26 @@ trait StdAttachments { def setPos(newpos: Position): this.type = { pos = newpos; this } } + /** When present, indicates that the host `Ident` has been created from a backquoted identifier. + */ case object BackquotedIdentifierAttachment + /** Stores the trees that give rise to a refined type to be used in reification. + * Unfortunately typed `CompoundTypeTree` is lacking essential info, and the reifier cannot use `CompoundTypeTree.tpe`. + * Therefore we need this hack (see `Reshape.toPreTyperTypeTree` for a detailed explanation). + */ case class CompoundTypeTreeOriginalAttachment(parents: List[Tree], stats: List[Tree]) + /** Is added by the macro engine to the results of macro expansions. + * Stores the original expandee as it entered the `macroExpand` function. + */ case class MacroExpansionAttachment(original: Tree) + /** When present, suppresses macro expansion for the host. + * This is occasionally necessary, e.g. to prohibit eta-expansion of macros. + * + * Does not affect expandability of child nodes, there's context.withMacrosDisabled for that + * (but think thrice before using that API - see the discussion at https://github.com/scala/scala/pull/1639). + */ case object SuppressMacroExpansionAttachment } -- cgit v1.2.3 From 7376ad78db80b7db04687edb3907aaa500c91c88 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 21 Nov 2012 07:37:29 +0100 Subject: SI-6695 Test case for fixed Array match bug --- test/files/run/t6695.scala | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 test/files/run/t6695.scala diff --git a/test/files/run/t6695.scala b/test/files/run/t6695.scala new file mode 100644 index 0000000000..b527238a51 --- /dev/null +++ b/test/files/run/t6695.scala @@ -0,0 +1,18 @@ +object Test extends App { + try { + Array("a", "b", "c") match { + case Array("a", "x", "c") => println("x") + case Array("a", "b", "x") => println("a"); + case Array("a", "d", _*) => println("wrongly positive") + } + assert(false, "match succeeded") + } catch { + case _: MatchError => // okay + } + + Array("a", "b", "c") match { + case Array("a", "x", "c") => println("x") + case Array("a", "b", "x") => println("a"); + case Array("a", "b", _*) => // okay + } +} -- cgit v1.2.3 From ea7246046ee7140ea07c8daa9c14cc2cd3111f5f Mon Sep 17 00:00:00 2001 From: Josh Suereth Date: Wed, 21 Nov 2012 08:29:41 -0500 Subject: Removing controversial `either` method from Futures API. * Removes `either` from Future * No tests need to change, since this was an untested method. --- src/library/scala/concurrent/Future.scala | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/src/library/scala/concurrent/Future.scala b/src/library/scala/concurrent/Future.scala index 320a4f22b8..4b9e74708d 100644 --- a/src/library/scala/concurrent/Future.scala +++ b/src/library/scala/concurrent/Future.scala @@ -522,29 +522,6 @@ trait Future[+T] extends Awaitable[T] { p.future } - /** Creates a new future which holds the result of either this future or `that` future, depending on - * which future was completed first. - * - * $nonDeterministic - * - * Example: - * {{{ - * val f = future { sys.error("failed") } - * val g = future { 5 } - * val h = f either g - * await(h, 0) // evaluates to either 5 or throws a runtime exception - * }}} - */ - def either[U >: T](that: Future[U]): Future[U] = { - val p = Promise[U]() - val completePromise: PartialFunction[Try[U], _] = { case result => p tryComplete result } - - this onComplete completePromise - that onComplete completePromise - - p.future - } - } -- cgit v1.2.3 From 548a54d708d8aaceea6abe0931aabaa70b2cd925 Mon Sep 17 00:00:00 2001 From: Your Name Date: Thu, 22 Nov 2012 20:45:50 +0100 Subject: SI-6023 reify abstract vals Type trees created by MethodSynthesis for abstract val getters carry symless originals, which are unusable for reification purposes (or the result of reification will be unhygienic). To combat this, type trees for such getters are now created empty, i.e. without any `tpe` set, just having an original assigned. Subsequent `typedTypeTree` invocations fill in the `tpe` and update the original to be symful. --- .../scala/tools/nsc/typechecker/MethodSynthesis.scala | 1 + src/compiler/scala/tools/nsc/typechecker/Typers.scala | 10 ++++++++-- test/files/run/t6023.check | 12 ++++++++++++ test/files/run/t6023.scala | 17 +++++++++++++++++ 4 files changed, 38 insertions(+), 2 deletions(-) create mode 100644 test/files/run/t6023.check create mode 100644 test/files/run/t6023.scala diff --git a/src/compiler/scala/tools/nsc/typechecker/MethodSynthesis.scala b/src/compiler/scala/tools/nsc/typechecker/MethodSynthesis.scala index e8a2c9f43c..acc4f7ff67 100644 --- a/src/compiler/scala/tools/nsc/typechecker/MethodSynthesis.scala +++ b/src/compiler/scala/tools/nsc/typechecker/MethodSynthesis.scala @@ -426,6 +426,7 @@ trait MethodSynthesis { // spot that brand of them. In other words it's an artifact of the implementation. val tpt = derivedSym.tpe.finalResultType match { case ExistentialType(_, _) => TypeTree() + case _ if mods.isDeferred => TypeTree() case tp => TypeTree(tp) } tpt setPos derivedSym.pos.focus diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index 222c9ff8c3..a2aca45e8f 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -5368,8 +5368,14 @@ trait Typers extends Modes with Adaptations with Tags { } def typedTypeTree(tree: TypeTree) = { - if (tree.original != null) - tree setType typedType(tree.original, mode).tpe + if (tree.original != null) { + val newTpt = typedType(tree.original, mode) + tree setType newTpt.tpe + newTpt match { + case tt @ TypeTree() => tree setOriginal tt.original + case _ => tree + } + } else // we should get here only when something before failed // and we try again (@see tryTypedApply). In that case we can assign diff --git a/test/files/run/t6023.check b/test/files/run/t6023.check new file mode 100644 index 0000000000..ee93565234 --- /dev/null +++ b/test/files/run/t6023.check @@ -0,0 +1,12 @@ +{ + abstract trait Foo extends AnyRef { + def a: Int + }; + () +} +{ + abstract trait Foo extends AnyRef { + def a: Int + }; + () +} diff --git a/test/files/run/t6023.scala b/test/files/run/t6023.scala new file mode 100644 index 0000000000..07af3685a5 --- /dev/null +++ b/test/files/run/t6023.scala @@ -0,0 +1,17 @@ +import scala.reflect.runtime.universe._ +import scala.reflect.runtime.{currentMirror => cm} +import scala.tools.reflect.ToolBox + +object Test extends App { + // test 1: reify + val tree = reify{ trait Foo { val a: Int } }.tree + println(tree.toString) + + // test 2: import and typecheck + val toolbox = cm.mkToolBox() + val ttree = toolbox.typeCheck(tree) + println(ttree.toString) + + // test 3: import and compile + toolbox.eval(tree) +} -- cgit v1.2.3