Typer inlines constants and makes dependency analysis impossible

[scabug]

The problem has been mentioned here:
https://groups.google.com/d/topic/scala-internals/b95Y-GbXVGA/discussion

The gist is that typer inlines compile time constants (e.g. references to static fields defined in Java) and forgets about the original tree that referred to a constant (e.g. to a field) which makes dependency analysis impossible.

According to Adriaan pattern matcher does not need constants to be inlined because it works with types. I couldn't readily find a reason why we need to inline those constants in typer so we probably should move this all the way to erasure.

I'm logging a ticket to not forget to investigate if we can do that for 2.10.2.

[scabug]

Imported From: https://issues.scala-lang.org/browse/SI-7173?orig=1
Reporter: @gkossakowski
Affected Versions: 2.10.0

[scabug]

@JamesIry said:
2.10.2 is about to be cut. Kicking down the road and un-assigning to foster work stealing.

[scabug]

@gkossakowski said:
It turns out that this is causing problems not only for dependency analysis.

The improved incremental compiler I'm working on relies on extracting used names in a given compilation unit. Once constant gets inlined we do not have access to the field that defined it therefore we can't extract its name.

I thought I could work-around it by inspecting symbol names of symbols in CompilationUnit.depends but we have only top-level classes recorded there, see:
https://github.com/scala/scala/blob/master/src/compiler/scala/tools/nsc/typechecker/Infer.scala#L263

Therefore I believe there's no other way than just fix the underlaying issue.

According to Adriaan we could perform constant inlining and folding based purely on types and leave trees intact. We still want constants to be folded in the bytecode so we could replace trees having constant types with actual constants in some later phase like erasure. This is an interesting idea that I would like to try out at some point.

[scabug]

@jvican said (edited on Feb 4, 2017 9:45:41 AM UTC):
Does this still affect 2.11 and 2.12 versions? I think it does.

The following test in Zinc is failing because of it. Could someone confirm this hasn't been fixed in new Scala versions and set those versions in the ticket?

This constant inlining could be perjudicial for a lot of Zinc users that make intense use of final vals. I think it would make sense to fix this and I'd be happy to take a stab at it. I'd like to know if @adriaanm or @retronym would accept such a PR for next 2.12.x and 2.11.x series (if there's to a be a new version for 2.11). A time estimate to fix this would also be very useful to pick the appropriate moment to implement it. I think this could be a blocker for a fully stable Zinc 1.0 release (though we could probably live with it since it depends on the compiler).

[scabug]

@milessabin said:
This will probably interact with the literal types PR ... please take a close look at that if/when working on this.

[scabug]

@dwijnand said:
scala/scala#4880 was an attempt at resolving this.

[scabug]

@milessabin said:
I think at least some of pull/4880 was folded into the literal types PR.

[scabug]

@jvican said:
I have a partial workaround for this here.
In the long term, I'll try to follow up on Adriaan's work to only use types to inline constants.

[retronym]

Here's another symptom of the fact that we constant fold term trees:

class C {
  final def foo = 1
  val s: String = {class D extends C { def foo = 2 }; ""}
}

-Xprint:typer shows val s: String = "";, and the invalid override of def foo is not flagged.

[lrytz]

See also #10340 (comment)