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

SI-8087 keep annotations on mixed-in private[this] fields #4010

Merged
merged 1 commit into from Oct 1, 2014

Conversation

lrytz
Copy link
Member

@lrytz lrytz commented Sep 29, 2014

Related to SI-2511 / eea7956, which fixed the same issue for non
private[this] fields.

If you have

trait T { private[this] val f = 0 }
class C extends T

Mixin geneartes an accessor method T.f with owner T. When
generating the field in C, the Mixin.mixinTraitMembers calls
fAccessor.accessed. The implementation of accessed does a lookup
for a member named "f " (note the space). The bug is that
private[this] fields are not renamed to have space
(LOCAL_SUFFIX_STRING) in their name, so the accessed was not found,
and no annotations were copied from it.

@lrytz
Copy link
Member Author

lrytz commented Sep 29, 2014

Review by @retronym – don't hesitate to think of a cleaner solution :)


val List(x, y) = bar.fields.asScala.toList.sortBy(_.name)
assertTrue((x.access & Opcodes.ACC_VOLATILE) != 0)
assertTrue((y.access & Opcodes.ACC_VOLATILE) != 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tend to think it would be simpler to test this using Java reflection, rather than with ASM.

scala> trait T { @volatile private[this] var x = 42 }; class C extends T
defined trait T
defined class C

scala> java.lang.reflect.Modifier.isVolatile(classOf[C].getDeclaredFields.head.getModifiers)
res24: Boolean = false

scala> class C { @volatile private[this] var x = 42 }
defined class C

scala> java.lang.reflect.Modifier.isVolatile(classOf[C].getDeclaredFields.head.getModifiers)
res25: Boolean = true

Related to SI-2511 / eea7956, which fixed the same issue for non
`private[this]` fields.

If you have

    trait T { private[this] val f = 0 }
    class C extends T

Mixin geneartes an accessor method `T.f` with owner `T`. When
generating the field in `C`, the Mixin.mixinTraitMembers calls
`fAccessor.accessed`. The implementation of `accessed` does a lookup
for a member named `"f "` (note the space). The bug is that
`private[this]` fields are not renamed to have space
(`LOCAL_SUFFIX_STRING`) in their name, so the accessed was not found,
and no annotations were copied from it.
@retronym
Copy link
Member

retronym commented Oct 1, 2014

LGTM

retronym added a commit that referenced this pull request Oct 1, 2014
SI-8087 keep annotations on mixed-in private[this] fields
@retronym retronym merged commit e5534f0 into scala:2.11.x Oct 1, 2014
@lrytz lrytz deleted the t8087 branch October 13, 2014 05:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants