-explaintypes got too verbose

[scabug]

With 2.10.0-M5 -explaintypes got much more verbose, compared to (for instance) Scala 2.9.x -- it produces extra output even when compilation does not fail.
While this might be done to help debugging, I don't think this is justified even for an intermediate release - debugging stuff should have separate options.

This problem was already briefly discussed on scala-language; Daniel Sobral suggested a bug report.
https://groups.google.com/forum/?fromgroups#!topic/scala-language/XG5SsELdwAg

[scabug]

Imported From: https://issues.scala-lang.org/browse/SI-6123?orig=1
Reporter: @Blaisorblade
Affected Versions: 2.10.0-M5, 2.10.0-M6
Attachments:

• ImplicitBugReport.scala (created on Jul 22, 2012 11:01:11 PM UTC, 3935 bytes)

[scabug]

@retronym said:
Can you provide a small sample program, and a diff of the output?

[scabug]

@Blaisorblade said (edited on Jul 22, 2012 11:05:30 PM UTC):
I found one available - it can be probably reduced further, but it's already just 100 lines. Please don't be confused by the complexity of code, I don't think it's necessary - I selected it from my codebase since it's self-contained and because for most warnings there's just no way to see where they come from, as you'll notice.

Normal output (Scala 2.9.1/.2): empty, as it's supposed to be. In particular, -explaintypes should never produce additional output on correct programs.

$ scalac -explaintypes src/main/scala/ivm/expressiontree/ImplicitBugReport.scala
$

With the new release:

$ ~/opt/scala-2.10.0-M5/bin/scalac -explaintypes src/main/scala/ivm/expressiontree/ImplicitBugReport.scala
Nothing <: ivm.expressiontree.ImplicitBugReport.Exp[Int]?
true
ivm.expressiontree.ImplicitBugReport.Exp[Int] <: ivm.expressiontree.ImplicitBugReport.Exp[T]?
false
Nothing <: ivm.expressiontree.ImplicitBugReport.Exp[Int]?
true
ivm.expressiontree.ImplicitBugReport.Exp[Int] <: ivm.expressiontree.ImplicitBugReport.Exp[T]?
  ivm.expressiontree.ImplicitBugReport.type = ivm.expressiontree.ImplicitBugReport.type?
  true
false
Unit <: ivm.expressiontree.ImplicitBugReport.CanBuildExp[Int,?]?
false
Nothing <: ivm.expressiontree.ImplicitBugReport.Exp[Int]?
true
ivm.expressiontree.ImplicitBugReport.Exp[Int] <: ivm.expressiontree.ImplicitBugReport.Exp[T]?
false
Nothing <: ivm.expressiontree.ImplicitBugReport.Exp[Int]?
true
ivm.expressiontree.ImplicitBugReport.Exp[Int] <: ivm.expressiontree.ImplicitBugReport.Exp[T]?
  ivm.expressiontree.ImplicitBugReport.type = ivm.expressiontree.ImplicitBugReport.type?
  true
false
Unit <: ivm.expressiontree.ImplicitBugReport.CanBuildExp[Int,?]?
false

I believe that you can take any sufficiently wide codebase of yours, activate -explaintypes and see tons of such warnings arise out of nothing. I had the same problem porting https://github.com/Blaisorblade/BAT to 2.10, even though the code base does not use advanced Scala features as much as mine (as far as I know).

[scabug]

@retronym said:
Regression between 2.10.0-M1 and -M2.

[scabug]

@Blaisorblade said:
For what it's worth, things got even worse between -M6 and -M7 -- output started appearing for
https://github.com/ps-mr/LinqOnSteroids/blob/a2546ff205d56c82baeedfdf02a12043a198386f/scratch/macros/Macros.scala. Sample output (maybe from a slightly different version):

scala.reflect.macros.Context <: scala.reflect.macros.Context?
true
c.universe.Expr[Any] <: c.universe.Expr[Any]?
true
c.universe.Expr[String] <: c.universe.Expr[String]?
true
scala.reflect.macros.Context <: scala.reflect.macros.Context?
true
c.universe.Expr[Any] <: c.universe.Expr[Any]?
true
c.universe.Expr[Unit] <: c.universe.Expr[Unit]?
true
scala.reflect.macros.Context <: scala.reflect.macros.Context?
true
c.universe.Expr[String] <: c.universe.Expr[String]?
true
c.universe.Expr[Any]* <: c.universe.Expr[Any]*?
true
c.universe.Expr[Unit] <: c.universe.Expr[Unit]?
true
scala.reflect.macros.Context <: scala.reflect.macros.Context?
true
c.universe.Expr[Any] <: c.universe.Expr[Any]?
true
c.universe.Expr[Any] <: c.universe.Expr[Any]?
true
scala.reflect.macros.Context <: scala.reflect.macros.Context?
true
c.universe.Expr[Any] <: c.universe.Expr[Any]?
true
c.universe.Expr[Any] <: c.universe.Expr[Any]?
true
scala.reflect.macros.Context <: scala.reflect.macros.Context?
true
c.universe.Expr[Any] <: c.universe.Expr[Any]?
true
c.universe.Expr[Any] <: c.universe.Expr[Any]?
true

I could try minimizing it if really needed.

[scabug]

@Blaisorblade said:
I tried adding "scala 2.10.0-M7" among versions, but it's not yet there.

[scabug]

@Blaisorblade said:
Is this not planned to be fixed for the whole 2.10 cycle? That's a bit sad.

[scabug]

@adriaanm said:
our priority target for major development is 2.11 -- happy to backport it to 2.10.x if possible
better yet, happy to merge a PR fixing and backporting it :-)

[scabug]

@Blaisorblade said (edited on Jan 20, 2013 4:38:48 PM UTC):
When Scala-IDE works and I'm patient enough for it (so, a lot of patience), a problem like this becomes easy to fix.

The problems are calls to Types.explainTypes when context.reportErrors is false, for instance because they happen during implicit resolution.

Types.explainTypes should thus check reportErrors but has no access to context, and not all calls go through Infer.explainTypes (although they probably should?), so I have to check context.reportErrors in multiple locations. Which means that the only reliable way to fix this problem is to add -explaintypes to scalac flags for all pos tests, and while at it ensure there are neg tests using -explaintypes and checking that it does produce output.

Pull request for the first part coming up soon.

The first problem is in c800d1fec5241ed8c29e5af30465856f9b583246, in particular in

+      def NotWithinBounds(tree: Tree, prefix: String, targs: List[Type],
+                          tparams: List[Symbol], kindErrors: List[String]) = {
+        if (settings.explaintypes.value) {
+          val bounds = tparams map (tp => tp.info.instantiateTypeParams(tparams, targs).bounds)
+          (targs, bounds).zipped foreach ((targ, bound) => explainTypes(bound.lo, targ))
+          (targs, bounds).zipped foreach ((targ, bound) => explainTypes(targ, bound.hi))
+          ()
+        }
+
+        issueNormalTypeError(tree,
+                prefix + "type arguments " + targs.mkString("[", ",", "]") +
+                " do not conform to " + tparams.head.owner + "'s type parameter bounds " +
+                (tparams map (_.defString)).mkString("[", ",", "]"))
+      }

[scabug]

@Blaisorblade said (edited on Jan 20, 2013 11:30:53 PM UTC):
Well, there's yet more than I thought. The other culprit is 78f9ef3906c78413ff8835fdad3849bfe5516be2, aka v2.10.0-M6-253-g78f9ef3 (I did say that things got worse in -M7). The code in this commit just logs subtype checks done during some parts of macro expansion, whether there is an error or not.
Maybe it's an instance of the broken window theory?

[scabug]

@Blaisorblade said:
I did not send a pull request yet, since I'm waiting for full test results, and it is not clear what exactly the best fix is. The current code is available here:
https://github.com/Blaisorblade/scala/tree/issue/6123
and fixes the bugs discussed above.

[scabug]

@hubertp said:
I commented a bit on the commits. One problem I can see is that if your type error gets buffered and all fallbacks fail then we report that buffered error. However at this point no explain types will be called.
Also before submitting PR please rebase your branch and squash to one commit (and it should be possible to add some tests for this particular case).

I am happy to review your stuff so just include me in the PR.

[scabug]

@Blaisorblade said:
Thanks for your comments.

One problem I can see is that if your type error gets buffered and all fallbacks fail then we report that buffered error. However at this point no explain types will be called.
I see your point; a better fix would be to buffer explainTypes messages, but that seems to call for more extensive refactorings. And as a user, I'd prefer for explainTypes to be usable sometimes (when it does explain me a type error) than not at all (because it explains too much).

On the most important tests, see
https://groups.google.com/d/topic/scala-internals/5D2EJViOMJo/discussion

But I'll also update some neg test.

[scabug]

@Blaisorblade said (edited on Jan 21, 2013 11:49:13 PM UTC):
Testing that -explaintypes works at all requires that partest inspects the standard output of the programs it runs. Hence I'm marking this as blocked by #7003.

[scabug]

@adriaanm said:
Un-assigning to foster work stealing, as announced in https://groups.google.com/forum/?fromgroups=#!topic/scala-internals/o8WG4plpNkw

[scabug]

@Blaisorblade said:
scala/scala#2259 is my current fix - the only problem is that it's hard to test correctly, because of limitations/bugs in partest (#7003 among others).

[scabug]

@Blaisorblade said:
The pull request was merged, but this should still be backported to 2.10.x. So I'm not closing this just yet.

[scabug]

@retronym said:
Please open a followup ticket if you want this backported. I'm closing to show this as fixed in 2.11.0-M3.