bug icon indicating copy to clipboard operation
bug copied to clipboard

Regression: name clash in macro generated code after upgrading to 2.12.8

Open Atry opened this issue 7 years ago • 5 comments

I found some name clashes when upgrading Scala to 2.12.8 in Binding.scala, maybe due to https://github.com/retronym/scala/commit/e76420490300e036c7357afcd685ed0e856fd8e8 .

Name clash due to placeholder

This can be workaround by replacing q"{ _: Unit => ... }" to q"{${c.freshName[TermName]("unit")}: Unit => ... }".

Name clash due to freshName

This can be workaround by changing the prefix of the freshName

Atry avatar Dec 07 '18 01:12 Atry

Name clash happens when macro X generate some calls to macro Y, and X and Y use the same prefix for freshName. For example, x and a are common prefixes used in many places.

Atry avatar Dec 07 '18 03:12 Atry

Thanks for the report. The regression actually came a bit earlier in https://github.com/scala/scala/commit/f50ec3c866263448d803139e119b33afb04ec2bc.

$ cat build.sh
#! /bin/bash -ex

. ~/.bash_profile

scalac-ref $(git --git-dir=/code/scala/.git log -1 --format=%H BISECT_HEAD) @/Users/jz/code/scala-js-example-ap/target/compile.args

$ git bisect start --no-checkout v2.12.8 v2.12.7 -- src/compiler src/reflect; git bisect run bash /code/scala-js-example-ap/build.sh

...
f50ec3c866263448d803139e119b33afb04ec2bc is the first bad commit

The intent of that change was to make nested macro expansions share a fresh-name namespace, associated with innermost enclosing definition that is not itself enclosed by a term owner. I'll investigate why that isn't helping in this case.

retronym avatar Mar 26 '19 00:03 retronym

The issue is that during macro-paradise's annotation macro expansion of each annotated method, the callsiteTyper is associated with the enclosing class. Later, the expansion of def-macros during typechecking has a callsiteTyper associated with the particular method. So we end can up with foo$macro$1 generated twice within a method: first by the annotation macro that is free to modify the DefDef, and second by regular macro expansion.

-Ymacro-global-fresh-names:true could be used as another workaround (that reverts to using a global fresh name context for all macros)

I've tried to add a special case to the compiler to detect macro annotation expansion and use the method symbol as the fresh name context instead. I wasn't able to do this because the macro argument is a duplicated copy of the DefDef with its Symbol removed.

retronym avatar Mar 26 '19 02:03 retronym

Even the positions have been stripped from the DefDef due to use of the case class copy method in https://github.com/scalamacros/paradise/blob/2.12.8/plugin/src/main/scala/org/scalamacros/paradise/reflect/TreeInfo.scala#L29-L118, which bears the comment:

// TODO: no immediate idea how to write this in a sane way

retronym avatar Mar 26 '19 02:03 retronym

@retronym should we move this to 2.12.10 or Backlog?

lrytz avatar Jul 17 '19 14:07 lrytz