Skip to content

Fix range position for vars with default initializer#103

Closed
retronym wants to merge 22 commits into2.12.xfrom
ticket/12213
Closed

Fix range position for vars with default initializer#103
retronym wants to merge 22 commits into2.12.xfrom
ticket/12213

Conversation

@retronym
Copy link
Owner

@retronym retronym commented Oct 28, 2020

➜  scala git:(2.12.x) ✗ sbt "scala -Yrangepos"
[info] Loading settings for project global-plugins from idea.sbt,dirtymoney.sbt,metals.sbt,gpg.sbt ...
[info] Loading global plugins from /Users/jz/.sbt/1.0/plugins
[info] Loading settings for project scala-build-build from plugins.sbt ...
[info] Loading project definition from /Users/jz/code/scala/project/project
[info] Loading settings for project scala-build from plugins.sbt,build.sbt ...
[info] Loading project definition from /Users/jz/code/scala/project
[info] Loading settings for project root from build.sbt ...
[info] Resolving key references (18463 settings) ...
[info] *** Welcome to the sbt build definition for Scala! ***
[info] version=2.12.13-bin-SNAPSHOT scalaVersion=2.12.12
[info] Check README.md for more information.
[info] Compiling 1 Scala source to /Users/jz/code/scala/build/quick/classes/compiler ...
[info] running (fork) scala.tools.nsc.MainGenericRunner -usejavacp -Yrangepos
Welcome to Scala 2.12.13-20201022-125904-2ab9556 (OpenJDK 64-Bit Server VM, Java 1.8.0_262).
Type in expressions for evaluation. Or try :help.

scala> :power
Power mode enabled. :phase is at typer.
import scala.tools.nsc._, intp.global._, definitions._
Try :help or completions for vals._ and power._

scala> object Other { var x: Int = _; var y: Int = 42 }
defined object Other

scala> val trees = lastRequest.trees
trees: List[$r.intp.global.Tree] =
List(object Other extends scala.AnyRef {
  def <init>() = {
    super.<init>();
    ()
  };
  var x: Int = _;
  var y: Int = 42
})

scala> val List(x, y) = trees.flatMap(_.collect { case vd : ValDef => vd }.takeRight(2))
x: $r.intp.global.ValDef = var x: Int = _
y: $r.intp.global.ValDef = var y: Int = 42

scala> x.pos
res0: $r.intp.global.Position = RangePosition(<console>, 15, 19, 29)

scala> x.pos.source.content(x.pos.end)
res1: Char = ;

Fixes scala/bug#12213

@dwijnand
Copy link

I'm thinking a test would be good. Outside of a checkfile of -Yrangepos, do we have a good way to write a test based on this end position being right?

dwijnand and others added 21 commits November 3, 2020 21:32
This reverts commit 8662d4a.

It is a very important API, but the compiler's binary API isn't stable.

This got confused because scala-rewrites is special in the
community-build as it uses binary dependencies (for scalafix/scalameta
dependency graph problems).  Additionally I optimistically "fix"
semanticdb-scalac-core's cross version when consuming Scala nightlies...
…orter-Reporter

Revert "Restore Global.reporter()Reporter"
…ings

[nomerge] Emit warnings in the REPL
Now that `Typer.stabilize` ends up checking for `@uncheckedStable`
annotations to support defs's as stable paths, new opportunities
arise for cyclic errors.

This PR reduces the likelihood of cycles by typechecking the only core of
an annotation (_not_ the full application to the annotation arguments)
to determine its type symbol.

pos/unchecked-stable-cyclic-error.scala test case started to fail
since scala#8338 with:

```
|-- object Test BYVALmode-EXPRmode (site: package <empty>)
|    |-- super APPSELmode-EXPRmode-POLYmode-QUALmode (silent: <init> in Test)
|    |    |-- this EXPRmode (silent: <init> in Test)
|    |    |    \-> Test.type
|    |    \-> Test.type
|    |-- def foo BYVALmode-EXPRmode (site: object Test)
|    |    |-- attr EXPRmode (site: method foo in Test)
|    |    |    |-- Int TYPEmode (site: method attr in Test)
|    |    |    |    \-> Int
|    |    |    |-- new anno APPSELmode-BYVALmode-EXPRmode-FUNmode-POLYmode (site: object Test)
|    |    |    |    |-- new anno APPSELmode-EXPRmode-POLYmode-QUALmode (site: object Test)
|    |    |    |    |    |-- anno FUNmode-TYPEmode (site: object Test)
|    |    |    |    |    |    \-> anno
|    |    |    |    |    \-> anno
|    |    |    |    \-> (a: Any): anno
|    |    |    |-- (a: Any): anno : pt=anno EXPRmode (site: value <local Test> in Test)
|    |    |    |    |-- foo : pt=Any BYVALmode-EXPRmode (site: value <local Test> in Test)
|    |    |    |    |    caught scala.reflect.internal.Symbols$CyclicReference: illegal cyclic reference involving method foo: while typing foo
/Users/jz/code/scala/sandbox/test.scala:7: error: recursive method foo needs result type
  @anno(foo) def attr: Int = foo
        ^
|    |    |    |    \-> anno
```

Another variant, pos/annotation-cycle.scala, fails only on 2.12.x
(after the backport of scala#8338). Backporting this fix makes that test
start passing on 2.12.x.

Backports scala#9302
…ation-info

[backport] Make annotation typechecking lazier
…ompat

Avoid use of binary incompatible super accessors in SortedMap/Set
… JDK 15

In JDK 15 CharSequence has an isEmpty method with a default
implementation, which clashes with our Array[Char]#isEmpty,
IndexedSeq[Char]#isEmpty, as well as our StringBuilder and reflect's
Name.

Backport of scala#9292 from 2.13.x to 2.12.x
[backport] Make the CharSequence wrappers in Predef non-implicit, for JDK 15
this regressed in scala#9091

the problem turned up in the Scala 2.12 community build -- when Scala
3 support was added to the scala-collection-compat repo, a test case
there was altered in a way that happened to tickle this
fix never-released 2.12.13 regression where Array[Unit].empty caused NPE
Catch exceptions for the InlineInfoAttribute reader
The `= _` should be include in the range position
of the ValDef.
Pass the position of the RHS explicitly rather than
using `rhs.pos` within TreeBuilder, as that doesn't
carry the position of the `_` represented by the
singleton tree `EmptyTree`.
@retronym
Copy link
Owner Author

retronym commented Dec 2, 2020

@dwijnand Agreed. We can can test this sort of thing with a junit test employing scala.tools.testing.BytecodeTesting to drive the compiler programatically.

Submitted upstream as scala#9355

@retronym retronym closed this Dec 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants