Never inspect twice the same macro application, fix (again) #1237 #1408
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The problem in 2.10.x is that macro expansions don't get duplicated like in 2.11.0, therefore the part of the expandee ends up being in an expansion creating the vicious cycle. In 2.11.0, the macro engine calls duplicate on all expansions, which creates new trees for every node of the expansion, preventing the cycles from occurring. |
|
I've restarted tests. |
|
Can one of the admins verify this patch? |
|
Restarted tests again. Hopefully, Travis will be more happy now. |
|
I set milestone to 0.13.6. I think this is important fix that should be included in the release. |
|
Hi @Duhemm! Could you rebase your PR on top of latest 0.13 branch? That should help with failing tests. |
added 2 commits
August 18, 2014 09:22
This test shows that macros that simply return their argument produce a stack overflow during extraction of used names. Consider a macro `foo(c: Context)(a: c.Expr[Any]) = a`. An application of this macro such as `foo(someVal)` will lead to the expansion `someVal`. sbt will extract the `original` tree from it, and find `foo(someVal)`, and recurse infinitely on `someVal`.
In Scala 2.10.4, this macro can produce a stack overflow :
def foo(a: Any): Any = macro impl
def impl(c: Context)(a: c.Expr[Any]): c.Expr[Any] = a
Here, an application such as `foo(someVal)` will produce the expansion
`someVal`. As expected, `someVal` has `original` tree `foo(someVal)`,
but if we inspect this tree, we will find that `someVal` has an
original tree, but it shouldn't.
Moreover, in Scala 2.11, some macros have their own application as
`original` trees.
See sbt#1237 for a description of these problems.
This commit fixes these two problems.
Fixes sbt#1237
|
Hi @gkossakowski ! It's done, I hope the tests will pass now ! |
|
|
|
Travis seems to be a bit too flaky. Restarting... |
|
Yay! Finally green! 💚 LGTM! |
gkossakowski
added a commit
that referenced
this pull request
Aug 18, 2014
Never inspect twice the same macro application, fix (again) #1237
Duhemm
pushed a commit
to Duhemm/sbt
that referenced
this pull request
Sep 1, 2014
In some cases, expanded macros report that their original tree and its expansion are the same, thus creating a cyclic chain. This chain may then produce a SOE during dependencies or used names extraction. This kind of problem was already reported in sbt#1237 and sbt#1408. Unfortunately, the fix that was applied to the dependencies extraction part was not sufficient. This commit ports the fix applied to the used names extraction part to the dependencies extraction part.
Duhemm
pushed a commit
to Duhemm/sbt
that referenced
this pull request
Sep 1, 2014
In some cases, expanded macros report that their original tree and its expansion are the same, thus creating a cyclic chain. This chain may then produce a SOE during dependencies or used names extraction. This kind of problem was already reported in sbt#1237 and sbt#1408. Unfortunately, the fix that was applied to the dependencies extraction part was not sufficient. This commit ports the fix applied to the used names extraction part to the dependencies extraction part.
Duhemm
pushed a commit
to Duhemm/sbt
that referenced
this pull request
Sep 1, 2014
In some cases, expanded macros report that their original tree and its expansion are the same, thus creating a cyclic chain. This chain may then produce a SOE during dependencies or used names extraction. This kind of problem was already reported in sbt#1237 and sbt#1408. Unfortunately, the fix that was applied to the dependencies extraction part was not sufficient. This commit ports the fix applied to the used names extraction part to the dependencies extraction part.
Duhemm
pushed a commit
to Duhemm/sbt
that referenced
this pull request
Sep 1, 2014
In some cases, expanded macros report that their original tree and its expansion are the same, thus creating a cyclic chain. This chain may then produce a SOE during dependencies or used names extraction. This kind of problem was already reported in sbt#1237 and sbt#1408. Unfortunately, the fix that was applied to the dependencies extraction part was not sufficient. This commit ports the fix applied to the used names extraction part to the dependencies extraction part. Mark test 'source-dependencies/macro' as passing Fixes sbt#1544
Duhemm
pushed a commit
to Duhemm/sbt
that referenced
this pull request
Sep 10, 2014
In some cases, expanded macros report that their original tree and its expansion are the same, thus creating a cyclic chain. This chain may then produce a SOE during dependencies or used names extraction. This kind of problem was already reported in sbt#1237 and sbt#1408. Unfortunately, the fix that was applied to the dependencies extraction part was not sufficient. Mark test 'source-dependencies/macro' as passing Fixes sbt#1544
eed3si9n
pushed a commit
to sbt/zinc
that referenced
this pull request
Aug 4, 2015
In some cases, expanded macros report that their original tree and its expansion are the same, thus creating a cyclic chain. This chain may then produce a SOE during dependencies or used names extraction. This kind of problem was already reported in sbt/sbt#1237 and sbt/sbt#1408. Unfortunately, the fix that was applied to the dependencies extraction part was not sufficient. Mark test 'source-dependencies/macro' as passing Fixes #1544
lrytz
pushed a commit
to lrytz/scala
that referenced
this pull request
Nov 5, 2019
In some cases, expanded macros report that their original tree and its expansion are the same, thus creating a cyclic chain. This chain may then produce a SOE during dependencies or used names extraction. This kind of problem was already reported in sbt/sbt#1237 and sbt/sbt#1408. Unfortunately, the fix that was applied to the dependencies extraction part was not sufficient. Mark test 'source-dependencies/macro' as passing Fixes scala#1544 Rewritten from sbt/zinc@92001e4
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Consider the following macro :
In Scala 2.10.4 (I've not observed this with any other version of Scala), an application of this macro such as :
will expand to
someVal. If we inspect this expansion, we will find thatsomeValis attached the original treefoo(someVal), as expected. However, if we inspect the components of this tree, we will find a cyclic chain ofMacroExpansionAttachment: The same original tree is attached tosomeVal, and so on, until the stack overflows.The solution used here consists to filter out the nodes that point to the current node in the
originaltree (I think that the diff may be clearer than my sentence).Thanks to @dwijnand for reporting the bug !
/cc @xeno-by
Fixes #1237 (again).