-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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.
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
Conversation
| 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 |
There was a problem hiding this comment.
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:
- The method doc says "
updateis implicit forconsumemethods" but the code doesn't handle them at all, is that because they're implicitly handled by beingMutable | Method? - What's the purpose of this hardcoding of
Array.update? Can it not be defined as anupdatemethod? (And if not, should that be documented somewhere?)
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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...)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- The method doc says "
updateis implicit forconsumemethods" but the code doesn't handle them at all, is that because they're implicitly handled by beingMutable | Method?
Yes, update methods receive the Mutable flag, which is done by private def normalizeFlags in typer/Namer.scala.
00eda9d to
92f60f4
Compare
Fixes #24485