Skip to content

Private field with non-alphanumeric identifier exhibits incorrect behavior at runtime #14667

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

Closed
FlorianCassayre opened this issue Mar 11, 2022 · 1 comment · Fixed by #14749
Closed
Assignees
Milestone

Comments

@FlorianCassayre
Copy link
Contributor

Compiler version

Affects 3.0.0 to 3.1.2-RC1.

Does not affect 2.13.8.

Minimized code

trait A {
  private val ? = ""
  def a = ?
}

@main def run(): Unit = println(new A {}.a)

Link to 3.1.1 Scastie

Output

null

Expectation

This program should print an empty line, not null.

Observations: the problem does not occur when

  • ? is renamed to an alphanumeric identifier (!)
  • ? is a definition instead of a value
  • A is a class instead of a trait
  • ? is protected or public

The ? can be replaced with any other symbol e.g. *, |||, ... (all of those will trigger the same problem)

When replacing "" by e.g. 1, the program prints 0 which confirms that it is a problem related to initialization.

@FlorianCassayre FlorianCassayre added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Mar 11, 2022
@odersky
Copy link
Contributor

odersky commented Mar 11, 2022

here's what we see after the Memoize phase:

final class $anon() extends Object, A {
            super()
            private val A$$?: String
            def A$$?(): String = this.A$$?
            super[A].$init()
            def A$_setter_$A$$$qmark_$eq(x$0: String): Unit = ()
            def a(): String = super[A].a()
          }

The problem is that the trait setter A$_setter_$A$$$qmark_$eq has a mangled name but the field A$$? is unmangled. As a consequence, the memoization code in line 171 falls into this case.

        val field = sym.field
        if !field.exists then
          // When transforming the getter, we determined that no field was needed.
          // In that case we can keep the setter as is, with a () rhs.
          tree

We should preferably use an unmangled name for the trait setter. If that does not work we have to take mangling into account when computing field.

@odersky odersky added area:transform and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Mar 11, 2022
sjrd pushed a commit to gagandeepkalra/dotty that referenced this issue Jul 19, 2022
…ameSeparated.

This brings the `case name @ AnyQualfiedName(_, _)` in line with
the behavior of `case naem: SimpleName`, which already did not
mangle before this change.

This fixes scala#14667 because `Mixin.traitSetterName` indirectly calls
`fullNameSeparated`, and expanded private symbolic member names
would get mangled, and therefore could not identify their
corresponding fields anymore.
odersky added a commit that referenced this issue Jul 20, 2022
Fix #14667: Remove mangling of inner AnyQualifiedName's in fullNameSeparated
@Kordyjan Kordyjan added this to the 3.2.1 milestone Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants