Skip to content

Do not consider generated getters to be update def #24906

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.

Sign up for GitHub

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

Merged
SolalPirelli merged 1 commit into from
Jan 7, 2026
Merged

Do not consider generated getters to be update def #24906

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions compiler/src/dotty/tools/dotc/cc/Mutability.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,7 @@ object Mutability:
*/
def isUpdateMethod(using Context): Boolean =
sym.isAllOf(Mutable | Method)
&& (if sym.isSetter then
sym.owner.derivesFrom(defn.Caps_Stateful)
&& !sym.field.hasAnnotation(defn.UntrackedCapturesAnnot)
else true
)
&& (!sym.is(Accessor) || (sym.isSetter && sym.owner.derivesFrom(defn.Caps_Stateful) && !sym.field.hasAnnotation(defn.UntrackedCapturesAnnot)))
|| ccConfig.strictMutability && sym.name == nme.update && sym == defn.Array_update
Copy link
Contributor Author

@SolalPirelli SolalPirelli Jan 6, 2026

Choose a reason for hiding this comment

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

There are two things I don't quite understand here:

  1. The method doc says "update is implicit for consume methods" but the code doesn't handle them at all, is that because they're implicitly handled by being Mutable | Method?
  2. What's the purpose of this hardcoding of Array.update? Can it not be defined as an update method? (And if not, should that be documented somewhere?)

Copy link
Member

@bishabosha bishabosha Jan 6, 2026

Choose a reason for hiding this comment

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

Array class itself is hardcoded to be considered as mutable but not inherit from it

Copy link
Contributor Author

@SolalPirelli SolalPirelli Jan 6, 2026

Choose a reason for hiding this comment

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

Why? Java interop? (sorry if this is a silly question...)

Copy link
Contributor

@bracevac bracevac Jan 6, 2026

Choose a reason for hiding this comment

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

The hard-coding of Array was a fairly recent addition #24649. IIRC, for backwards compatibility, we cannot retroactively make Array extend Mutable. We plan to have a new collections-library design based around separation checking.

Copy link
Contributor

@bracevac bracevac Jan 6, 2026

Choose a reason for hiding this comment

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

That also reminds me: We don't have a good way right now to represent this Array treatment in the library's API docs.

Copy link
Contributor

@bracevac bracevac Jan 6, 2026

Choose a reason for hiding this comment

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

  1. The method doc says "update is implicit for consume methods" but the code doesn't handle them at all, is that because they're implicitly handled by being Mutable | Method?

Yes, update methods receive the Mutable flag, which is done by private def normalizeFlags in typer/Namer.scala.


/** A read-only member is a lazy val or a method that is not an update method. */
Expand Down
3 changes: 3 additions & 0 deletions compiler/test/dotty/tools/dotc/printing/PrintingTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,7 @@ class PrintingTest {

@Test
def transformedPrinting: Unit = testIn("tests/printing/transformed", "repeatableAnnotations")

@Test
def getters: Unit = testIn("tests/printing/getters", "getters")
}
10 changes: 10 additions & 0 deletions tests/printing/getters/var-should-not-be-update-def-get.check
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[[syntax trees at end of MegaPhase{pruneErasedDefs, uninitialized, inlinePatterns, vcInlineMethods, seqLiterals, intercepted, getters, specializeFunctions, specializeTuples, collectNullableFields, elimOuterSelect, resolveSuper, functionXXLForwarders, paramForwarding, genericTuples, letOverApply, arrayConstructors}]] // tests/printing/getters/var-should-not-be-update-def-get.scala
package <empty> {
import scala.language.experimental.captureChecking
@SourceFile("tests/printing/getters/var-should-not-be-update-def-get.scala")
trait Foo() extends Object, scala.caps.Mutable {
def x: Int
update def x_=(x$1: Int): Unit
}
}

4 changes: 4 additions & 0 deletions tests/printing/getters/var-should-not-be-update-def-get.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import scala.language.experimental.captureChecking

trait Foo extends caps.Mutable:
var x: Int
Loading