scala / bug

Scala 2 bug reports only. Please, no questions — proper bug reports only.
https://scala-lang.org
232 stars 21 forks source link

Seems like referential transparency optimizations are being applied after scaladoc runs thus breaking some macros #11285

Closed pshirshov closed 5 years ago

pshirshov commented 5 years ago

There is a macro in fastparse 2, doing this:

  def charsWhileInMacro(c: Context)
                       (s: c.Expr[String], min: c.Expr[Int])
                       (ctx: c.Expr[ParsingRun[Any]]): c.Expr[ParsingRun[Unit]] = {
    import c.universe._

    val literal = s.actualType match{
      case ConstantType(Constant(x: String)) => x
      case _ => c.abort(c.enclosingPosition, "Function can only accept constant singleton type")
    }

This code compiles fine with the following snippet:

  final val digits  = "0123456789"
  final val hexDigits = digits + "abcdefABCDEF"
  def HexNum[_: P]: P[Unit] = P("0x" ~ CharsWhileIn(hexDigits))

But scaladoc fails with Function can only accept constant singleton type message. This inconsistency is very weird and misleading. Is it possible to fix it somehow?

Related: https://github.com/lihaoyi/fastparse/issues/205

hrhino commented 5 years ago

I think this is on purpose; it's implemented in canAdaptConstantTypeToLiteral. The commit messages don't clarify much, but I assume it's got something to do with giving final val Foo = 1 type Int in scaladoc, rather than whatever the human-readable representation of ConstantType(Constant(1)) is.

Possibly this could be done during HTML generation.

som-snytt commented 5 years ago

Duplicates https://github.com/scala/bug/issues/9532

This shows a trivial macro that reports whether it sees a Literal expr and a ConstantType, where under scaladoc it sees the constant type in 2.13 milestone. Scaladoc still sees the original expression tree, however.

$ scalac mlit.scala
$ scalac litcheck.scala
expr unchecked
type unchecked
expr hello, world
type hello, world
$ scala mlit.Main
hello, world
$ ~/scala-2.12.6/bin/scaladoc -d /tmp litcheck.scala
other expr "un".+(Main.checked)
other type String
litcheck.scala:4: error: annotation argument needs to be a constant; found: "un".+(Main.checked)
@SuppressWarnings(Array(M.lit("un" + Main.checked)))
                                   ^
other expr "hello, ".+(Main.this.who)
other type String
model contains 4 documentable templates
one error found
$ scalacm mlit.scala
$ scalacm litcheck.scala
expr unchecked
type unchecked
expr hello, world
type hello, world
$ scalam mlit.Main
hello, world
$ ~/scala-2.13.0-M5/bin/scaladoc -d /tmp litcheck.scala
other expr "un".+(Main.checked)
type unchecked
other expr "hello, ".+(Main.this.who)
type hello, world
model contains 4 documentable templates
hrhino commented 5 years ago

thanks for the archaeology, @som-snytt! But on that ticket, I see that both Jason and Adriaan took a stab at the problem, and neither got their fix in. Does the availability of example solutions by two core Scala developers make #9532 a "good first issue" now?

som-snytt commented 5 years ago

@hrhino I assume it works because constant types are first class citizens. I don't know if there is benefit to exploiting OriginalTree attachment as described in the third ticket. Presentation compiler also uses the flag, as it has a similar requirement to see "what the user wrote".

lrytz commented 5 years ago

Would be good to see if OriginalTree is good enough for Scaladoc.

neko-kai commented 5 years ago

Is there a way in Scala 2.12 to detect if a macro is running inside scaladoc to be able to add workarounds for spurious failures?

som-snytt commented 5 years ago

@kaishh Maybe just inspect what 1 + 2 compiles to. It might be possible to cast the universe, get a Typer and check the method mentioned earlier? Or inspect the stack trace for the doc package.

adriaanm commented 1 year ago

btw, I just ran into this discrepancy at work today -- I see progress was made in the linked PRs, but this version of the issue remains.

// > scaladoc -language:experimental.macros -classpath ~/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-compiler/2.13.10/scala-compiler-2.13.10.jar:~/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-reflect/2.13.10/scala-reflect-2.13.10.jar:~/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/eu/timepit/refined_2.13/0.10.1/refined_2.13-0.10.1.jar:~/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/com/chuusai/shapeless_2.13/2.3.9/shapeless_2.13-2.3.9.jar /tmp/bug_scaladoc_constant.scala

import eu.timepit.refined.types.numeric.PosDouble
import eu.timepit.refined.auto.autoRefineV

class C { autoRefineV(2) : PosDouble }
/*
error: exception during macro expansion:
java.lang.ClassCastException: class java.lang.Integer cannot be cast to class java.lang.Double (java.lang.Integer and java.lang.Double are in module java.base of loader 'bootstrap')
    at scala.runtime.BoxesRunTime.unboxToDouble(BoxesRunTime.java:112)
    at scala.math.Numeric$DoubleIsFractional$.gt(Numeric.scala:177)
    at eu.timepit.refined.numeric$Greater$.$anonfun$greaterValidate$1(numeric.scala:104)
    at eu.timepit.refined.numeric$Greater$.$anonfun$greaterValidate$1$adapted(numeric.scala:104)
    at eu.timepit.refined.api.Validate$$anon$3.validate(Validate.scala:81)
    at eu.timepit.refined.macros.RefineMacro.impl(RefineMacro.scala:26)
*/

(It compiles fine during normal compilation because 2 gets turned into 2.0.)

/* likely culprit -- ClassCastException disappears after applying this patch to 2.13.10:

diff --git a/src/scaladoc/scala/tools/nsc/doc/ScaladocAnalyzer.scala b/src/scaladoc/scala/tools/nsc/doc/ScaladocAnalyzer.scala
index e361e72990..0eeca2ab82 100644
--- a/src/scaladoc/scala/tools/nsc/doc/ScaladocAnalyzer.scala
+++ b/src/scaladoc/scala/tools/nsc/doc/ScaladocAnalyzer.scala
@@ -27,7 +27,7 @@ trait ScaladocAnalyzer extends Analyzer {

   trait ScaladocTyper extends Typer {

-    override def canAdaptConstantTypeToLiteral = false
+    override def canAdaptConstantTypeToLiteral = true
*/