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

@getter StaticAnnotations on ValDefs are nondeterminstically visible to macros depending on order of typechecking #7561

Open
scabug opened this Issue Jun 6, 2013 · 11 comments

Comments

Projects
None yet
2 participants
@scabug
Copy link

scabug commented Jun 6, 2013

See attached. The behaviour of the rewrite macro is dependent on the order of class Foo and object Consumer in Consumer.scala. If you put object Consumer before class Foo the error will disappear as the macro does not see the annotation.

In the macro code you can see an attempt to force potentially (uninitialized) infos of annotations. However, that doesn't help because annotations list is empty.

Also note that if you change annotated val k to be def then the problem disappears.

@scabug

This comment has been minimized.

Copy link

scabug commented Jun 6, 2013

Imported From: https://issues.scala-lang.org/browse/SI-7561?orig=1
Reporter: @gkossakowski
Affected Versions: 2.10.2-RC2
See #7404

@scabug

This comment has been minimized.

Copy link

scabug commented Jun 6, 2013

@gkossakowski said:
It's most likely some bad interaction between namer and typer. Assigning to Adriaan who is looking into that area.

@scabug

This comment has been minimized.

Copy link

scabug commented Jun 6, 2013

@xeno-by said:
Can't see the attached code. Could you maybe post it in a gist?

@scabug

This comment has been minimized.

Copy link

scabug commented Jun 6, 2013

@xeno-by said:
Also see #7424 for a similar issue with a minimized reproduction and a workaround.

@scabug

This comment has been minimized.

Copy link

scabug commented Jun 6, 2013

@gkossakowski said:
Eugene, thanks for spotting my mistake. Here's the gist: https://gist.github.com/gkossakowski/5724506

Also, I saw #7424 but suggested work-around doesn't help in this case.

@scabug

This comment has been minimized.

Copy link

scabug commented Jun 6, 2013

@xeno-by said:
Yeah it looks like I was too rash to close that one as a duplicate.

@scabug

This comment has been minimized.

Copy link

scabug commented Oct 25, 2013

Emre Çelikten (emre) said:
Any workarounds for this one?

@scabug

This comment has been minimized.

Copy link

scabug commented Oct 26, 2013

@retronym said (edited on Oct 26, 2013 1:56:56 PM UTC):
A workaround is to also inspect the annotations of the underlying field. You must try both the accessor and the underlying field.

  /code/gist/5724506-SI-7561 tail -n 1000 *.scala
==> Consumer.scala <==
package cons

import mac._

class Bar {
  @fancy val k: Int = 5
  def k$fancy: Int = ???
}


class Consumer {
  val f: Foo = new Foo
  val b: Bar = new Bar
  Rewrite.rewrite(f.k)
  Rewrite.rewrite(b.k)
}

class Foo {
  @fancy val k: Int = 5
  def k$fancy: Int = ???
}

==> Rewrite.scala <==
package mac

@scala.annotation.meta.getter
class fancy extends scala.annotation.StaticAnnotation

object Rewrite {
  import scala.language.experimental.macros
  import scala.reflect.macros.Context

  def rewrite[A](a: A) = macro rewriteImpl[A]
  def rewriteImpl[A: c.WeakTypeTag](c: Context)(a: c.Expr[A]): c.Expr[A] = {
    import c.universe._


    def accessedOrSelf(s: Symbol) = if (s.isTerm) s.asTerm.accessed orElse s else s

    // workaround for SI-7561. The namer phase in Scalac creates a getter symbol
    // derived from the primary symbol (the field) of a ValDef tree. Only the primary
    // symbol reads annotations in its type completer.
    //
    // When typechecking the enclosing template, the annotations from the primary
    // symbol are copied or moved to the derived symbols, based on meta-annotations
    // like @getter. But a macro could easily observe the symbols before this stage
    // and see a getter without annotations.
    //
    // This method searchs all annotations on the given symbol (perhaps an accessor)
    // and its underlying field for one that matches `f`
    def isAnnotated(s: Symbol)(f: AnnotationApi => Boolean) = {
      def check(s: Symbol) = s.annotations.exists(f)
      Set(s, accessedOrSelf(s)).exists(s => check(s))
    }

    val fancySym = weakTypeOf[fancy].typeSymbol

    def isFancy(a: AnnotationApi) = a.tpe.typeSymbol == fancySym

    val t = a.tree match {
      case sel @ Select(qual, name) if isAnnotated(sel.symbol)(isFancy) =>
        val result = Select(qual, newTermName(name + "$fancy"))
        println(s"rewriting $sel to $result")
        result
      case s => s
    }

    c.Expr[A](t)
  }
}
% scalac -version
Scala compiler version 2.10.3 -- Copyright 2002-2013, LAMP/EPFL

% scalac ./Rewrite.scala && scalac -Xprint:parser,typer Consumer.scala
[[syntax trees at end of                    parser]] // Consumer.scala
package cons {
  import mac._;
  class Bar extends scala.AnyRef {
    def <init>() = {
      super.<init>();
      ()
    };
    @new fancy() val k: Int = 5;
    def k$fancy: Int = $qmark$qmark$qmark
  };
  class Consumer extends scala.AnyRef {
    def <init>() = {
      super.<init>();
      ()
    };
    val f: Foo = new Foo();
    val b: Bar = new Bar();
    Rewrite.rewrite(f.k);
    Rewrite.rewrite(b.k)
  };
  class Foo extends scala.AnyRef {
    def <init>() = {
      super.<init>();
      ()
    };
    @new fancy() val k: Int = 5;
    def k$fancy: Int = $qmark$qmark$qmark
  }
}

rewriting Consumer.this.f.k to Consumer.this.f.k$fancy
rewriting Consumer.this.b.k to Consumer.this.b.k$fancy
[[syntax trees at end of                     typer]] // Consumer.scala
package cons {
  import mac._;
  class Bar extends scala.AnyRef {
    def <init>(): cons.Bar = {
      Bar.super.<init>();
      ()
    };
    private[this] val k: Int = 5;
    @mac.fancy <stable> <accessor> def k: Int = Bar.this.k;
    def k$fancy: Int = scala.this.Predef.???
  };
  class Consumer extends scala.AnyRef {
    def <init>(): cons.Consumer = {
      Consumer.super.<init>();
      ()
    };
    private[this] val f: cons.Foo = new Foo();
    <stable> <accessor> def f: cons.Foo = Consumer.this.f;
    private[this] val b: cons.Bar = new Bar();
    <stable> <accessor> def b: cons.Bar = Consumer.this.b;
    Consumer.this.f.k$fancy;
    Consumer.this.b.k$fancy
  };
  class Foo extends scala.AnyRef {
    def <init>(): cons.Foo = {
      Foo.super.<init>();
      ()
    };
    private[this] val k: Int = 5;
    @mac.fancy <stable> <accessor> def k: Int = Foo.this.k;
    def k$fancy: Int = scala.this.Predef.???
  }
}
@scabug

This comment has been minimized.

Copy link

scabug commented Oct 26, 2013

@retronym said:
Here's the place where we should attempt to fix this in the compiler:

git diff -U8 -- src/compiler/scala/tools/nsc/typechecker/Namers.scala
diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala
index e3d7bfd..451cfab 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala
@@ -766,16 +766,17 @@ trait Namers extends MethodSynthesis {
     /* Explicit isSetter required for bean setters (beanSetterSym.isSetter is false) */
     def accessorTypeCompleter(tree: ValDef, isSetter: Boolean) = mkTypeCompleter(tree) { sym =>
       logAndValidate(sym) {
         sym setInfo {
           val tp = if (isSetter) MethodType(List(sym.newSyntheticValueParam(typeSig(tree))), UnitTpe)
                    else NullaryMethodType(typeSig(tree))
           pluginsTypeSigAccessor(tp, typer, tree, sym)
         }
+        sym.setAnnotations(tree.symbol.annotations)
       }
     }

     def selfTypeCompleter(tree: Tree) = mkTypeCompleter(tree) { sym =>
       val selftpe = typer.typedType(tree).tpe
       sym setInfo {
         if (selftpe.typeSymbol isNonBottomSubClass sym.owner) selftpe
         else intersectionType(List(sym.owner.tpe, selftpe))

This patch just blindly copies all annotations to the getter symbol in the getter's type completer. (Actually, it just shares references). The question will be whether we can do the necessary filtering here (based on meta-annotations) without risking triggering cycles.

@scabug

This comment has been minimized.

Copy link

scabug commented Oct 28, 2013

@dragos said:
Somewhat related

@scabug

This comment has been minimized.

Copy link

scabug commented Dec 23, 2015

@michael72 said:
I stumbled on the same bug (I guess) and added a simplified sample project at github. I really would like to be able to check for special annotations in classes in macro code.
The above workarounds applied in the macro (also the one mentioned in 7424) do not seem to work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment