Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

final vals in traits not handled properly in Scala 2.12 #193

Closed
gslowikowski opened this issue Nov 27, 2016 · 3 comments
Closed

final vals in traits not handled properly in Scala 2.12 #193

gslowikowski opened this issue Nov 27, 2016 · 3 comments
Assignees
Labels

Comments

@gslowikowski
Copy link
Member

The original issue is here: scoverage/sbt-scoverage#203

I investigated, how it works with different Scala versions (2.10.6, 2.11.8 and 2.12.0) on very simple trait:

package test

trait TraitWithFinalVal {
  final val FOO = "Bar"
}

I don't feel qualified to fix this issue, this description is for @sksamuel (or anyone knowing scalac internals).

Scoverage during instrumentation replaces simple constant

"Bar"

with a block:

{
  scoverage.Invoker.invoked(1, "path/to/scoverage-data");
  "Bar"
}

The type of this block should be equal to the type of the last statement ("Bar").
It's set by this typer call:

            val apply = invokeCall(id)
            val block = Block(List(apply), tree)
            localTyper.typed(atPos(tree.pos)(block)) // <------ here

What are the types of uninstrumented and instrumented val in different Scala versions:

Scala version uninstrumented (simple value) type instrumented (block) type difference? problem?
2.10.6 String String no no
2.11.8 String("Bar") String yes no
2.12.0 String("Bar") String yes YES!

As you can see, after instrumentation the resulting block type is different from val type even with Scala 2.11, but it doesn't lead to compiler error. I found the exact git commit which changed things - it's scala/scala@50462a4

I'm not sure if typed method returns wrong type, or Scoverage should not depend on it and set the type of the block to the type of the constant (something like setType(tree.tpe)).

@sksamuel
Copy link
Member

This is fixed now.

@gslowikowski
Copy link
Member Author

Summary of the problem, from my point of view:

There was a bug in Scala 2.12.0 - scala/scala-dev#219.
It's fixed in 2.12.1 (commit scala/scala@71a2cad)
and it fixes our scoverage problem.

Commits: aa3c5ed and 8bc8c22 change too much.

Most importantly this change:

case v: ValDef if v.rhs.isInstanceOf[Literal] => tree

(aa3c5ed#diff-e85576328aa1f74addcb13a5f7546600R585)

E.g. the number of statements in scala library projects changes from 37918 to 37261. This is not acceptable for minor bugfix release.

This change aa3c5ed#diff-e85576328aa1f74addcb13a5f7546600L608 introduces a bug.

I've tested this on Scala project and "updateLocation" should be restored here.
Check for example Specializable class (https://github.com/scala/scala/blob/2.12.x/src/library/scala/Specializable.scala).

All statements after Group class definition have classType = "Class" and fullClassName = "Specializable.Group".
They should have classType = "Object" and fullClassName = "Specializable".

Simplified class to test:

object Specializable {
  trait SpecializedGroup { }

  class Group[T >: Null](value: T) extends SpecializedGroup { }

  final val Primitives  = new Group((Byte, Short, Int, Long, Char, Float, Double, Boolean, Unit))
}

This should be fixed/cleaned because, from my point of view, it blocks further scoverage development.

@gslowikowski
Copy link
Member Author

@sksamuel I'd like to progress with development, but this issue blocks me.

gslowikowski added a commit to gslowikowski/scalac-scoverage-plugin that referenced this issue Aug 14, 2017
Changes:
- revert two changes in `plugin.scala` file (see comment scoverage#193 (comment))
- upgrade Scala version from 2.12.0 to 2.12.1
gslowikowski added a commit that referenced this issue Aug 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants