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

Regression involving macros, resetLocalAttrs, and singleton types #7516

Closed
scabug opened this issue May 24, 2013 · 10 comments
Closed

Regression involving macros, resetLocalAttrs, and singleton types #7516

scabug opened this issue May 24, 2013 · 10 comments
Assignees
Labels
Milestone

Comments

@scabug
Copy link

scabug commented May 24, 2013

The following REPL transcript works in 2.10.1, but not 2.10.2-RC1:

scala> final case class CV(p: Int = 3, g: Int = 2)
defined class CV

scala> import scala.reflect._,macros._, scala.language.experimental.macros
import scala.reflect._
import macros._
import scala.language.experimental.macros

scala> def macroImpl[T: c.WeakTypeTag](c: Context)(t: c.Expr[T]): c.Expr[List[T]] = {
     |   val r = c.universe.reify { List(t.splice) }
     |   c.Expr[List[T]]( c.resetLocalAttrs(r.tree) )
     | }
macroImpl: [T](c: scala.reflect.macros.Context)(t: c.Expr[T])(implicit evidence$1: c.WeakTypeTag[T])c.Expr[List[T]]

scala> def demo[T](t: T): List[T] = macro macroImpl[T]
demo: [T](t: T)List[T]

scala> demo { val d = 4; CV(g = d); "a" }
<console>:19: error: type mismatch;
 found   : d.type (with underlying type Int)
 required: d.type
              demo { val d = 4; CV(g = d); "a" }
                                       ^

The code compiles in 2.10.2-RC1 when one of:

  • resetLocalAttrs is removed
  • named/default arguments aren't used
  • CV(g = d: Int) is used
@scabug
Copy link
Author

scabug commented May 24, 2013

Imported From: https://issues.scala-lang.org/browse/SI-7516?orig=1
Reporter: @harrah
Affected Versions: 2.10.2-RC1
See #7234

@scabug
Copy link
Author

scabug commented May 25, 2013

@paulp said:
It's 83c9c764b5. Pulled back too far.

@scabug
Copy link
Author

scabug commented May 25, 2013

@retronym said:
We generated a ValDef whose tpt TypeTree has no original; this contains a reference to the symbol for d. resetAttrs and the retypecheck assigns a new symbol for d and leaves a the reference to the prior symbol dangling.

I think I'll revert the fix altogether on 2.10.x, and refine it on master to only use the stable type for the named arguments if needed to conform to the formal parameter type.

The real bug is the resetAttrs concept.

@scabug
Copy link
Author

scabug commented May 25, 2013

@gkossakowski said:
Jason, I found your last statement intriguing. Could you elaborate why you find resetAttrs a bad idea and how would you address use case resetAttrs tries to serve?

@scabug
Copy link
Author

scabug commented May 25, 2013

@retronym said (edited on May 25, 2013 5:12:31 PM UTC):
It has been a constant source of bugs. No one can really say with great certainty what it does. To maintain the invariant that typecheck(reset(typecheck(t)) =~= typecheck(t), all sorts of finicky tricks like TypeTree#original are needed, and they don't always suffice. See https://groups.google.com/d/msg/scala-internals/t6cpY7fGng4/0CIfXj1e-UEJ for a recent example.

I'm exploring ways that macros can avoid the need to use resetAttrs, e.g. if they just need to splice a tree into a new context, we just need to expose changeOwner transformations, see: https://issues.scala-lang.org/browse/SI-5797?focusedCommentId=63969&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-63969. Once we get through the current crop of releases, I'm going to finish my resetAttrs-free version of scala-async.

See also:

#6187
#5464
...

@scabug
Copy link
Author

scabug commented May 25, 2013

@paulp said:
I'm surprised that comment requires elaboration. reset* was sure to be a nightmare from the beginning. Do people not notice that almost every new mechanism which comes along is a means of avoiding, undoing, freezing, thawing, or reconstructing the mutation performed by earlier mechanisms? Instead of turtles all the way down, it's mutants all the way down.

The compiler as it is will never be robust. As long as methods with names like "resetFoo" are piled on top of the problem rather than doing something about the root cause, this will never change.

@scabug
Copy link
Author

scabug commented May 25, 2013

@gkossakowski said:
The only reason I asked for elaboration is to hear from people that fully understand it so I don't need to guess.

I agree that trying to reset mutable state in case of ad-hoc caching mechanisms we have in the compiler is not a good idea. I just wanted to know what breaks exactly in this particular case.

Thanks to both Jason and Paul.

@scabug
Copy link
Author

scabug commented May 25, 2013

@paulp said:
I don't mean to discourage questions. It's just that at this point I've wasted an awful lot of my life on issues which (it is increasingly clear to me) are completely unnecessary. And there's not really any end in sight.

@scabug
Copy link
Author

scabug commented May 27, 2013

@retronym said:
scala/scala#2601

@scabug scabug closed this as completed May 29, 2013
@scabug
Copy link
Author

scabug commented May 29, 2013

@harrah said:
Thanks!

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