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

sanity checks for macro expansions #8066

Closed
scabug opened this issue Dec 10, 2013 · 10 comments
Closed

sanity checks for macro expansions #8066

scabug opened this issue Dec 10, 2013 · 10 comments
Labels
Milestone

Comments

@scabug
Copy link

scabug commented Dec 10, 2013

There are some macro-induced compiler crashes in SuperAccessors, LambdaLift and GenICode (some of those documented at sbt/sbt@3b213e5) that we can easily avoid by providing a sanity-checking traverser for macro expansions. I will get to that this week.

@scabug
Copy link
Author

scabug commented Dec 10, 2013

Imported From: https://issues.scala-lang.org/browse/SI-8066?orig=1
Reporter: @xeno-by
Affected Versions: 2.11.0-M7
See #5797, #7920, #7829, #7516

@scabug
Copy link
Author

scabug commented Dec 10, 2013

@xeno-by said:
Problems that we'd like to prevent:

  1. Untyped trees nested inside types trees leading to trees with null tpe escaping typer (crash in SuperAccessors)
  2. Owner chain corruption (crashes in LambdaLift, GenICode): sbt/sbt@3b213e5#commitcomment-4735585
  3. Sharing of trees between different parts of the expansion (now that's not a problem per se, because expansions are duplicated anyway, but it might screw people up when they typecheck a shared tree and another part of the expansion unexpectedly gets typed).

So #1 is definitely a compilation error.

#2 looks like a compilation error as well, but we should actually consider detecting attributed trees spliced under symbol-introducing trees in the expansion and report that as as an error, because someone might end up splicing symbol-introducing trees there, even if in the current expansion it's just something simple. In 2.10.5 that should probably be an on-by-default warning that can be turned off by a -Y flag.

I think we should turn #3 into a compilation error as well. Even though it's going to work just fine, it's a symptom of something going wrong.

Finally, for better error reporting I think we could have quasiquotes remember their origins (maybe in attachments), so that we could say something like: "attempt to insert an untyped tree foo (defined in Foo.scala:42) under a type tree bar (coming from an argument of the macro expansion)".

@scabug
Copy link
Author

scabug commented Dec 11, 2013

@xeno-by said:
Also see #5755

@scabug
Copy link
Author

scabug commented Dec 26, 2013

@xeno-by said:
There's a really funny effect that happens when someone tries to return a statement (e.g. a MemberDef) from a macro impl. Then the macro engine says:

Test.scala:8: error: type mismatch;
 found   : <notype>
 required: Any
Note that <none> extends Any, not AnyRef.
Such types can participate in value classes, but instances
cannot appear in singleton types or in reference comparisons.
  Macros.foo
         ^
one error found

This should be caught by the validator. Also, it would be useful if the validator could tell people that such code doesn't make sense anyway, because the expanded definition is going to be local.

@scabug
Copy link
Author

scabug commented Jan 6, 2014

@xeno-by said:
Another fun thing that's not directly related to this bug, but still I'll post it here to fix it together with the rest of the stuff. It looks like providing c.Expr[_] as a return type of a macro impl leads to funny effects, so we should probably disallow that, especially since we now allow c.Tree as the return type.

@scabug
Copy link
Author

scabug commented Jan 8, 2014

@xeno-by said:
Somewhat related: scala/scala#3326 (comment)

@scabug
Copy link
Author

scabug commented Jan 10, 2014

@xeno-by said:
Something to keep in mind given our recent untypecheck idea (https://groups.google.com/forum/#!topic/scala-internals/TtCTPlj_qcQ): #7516 + scala/scala@83c9c764b5

@scabug
Copy link
Author

scabug commented Jan 22, 2014

@xeno-by said:
Even though we don't have time to implement the untypecheck idea by RC1, we could still write a validator that warns users about potential owner chain corruptions and other problems and links them to the docs that explain the situation in more detail.

@scabug
Copy link
Author

scabug commented Jan 22, 2014

@xeno-by said:
Related: #7920

@SethTisue
Copy link
Member

closing since this never happened and it's far from clear that a change like this is likely ever to happen in the current macro system

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

3 participants