Failures on Wire up "why not promoted" messages for failed field promotion due to conflict. #53617
Assignees
Labels
legacy-area-front-end
Legacy: Use area-dart-model instead.
Comments
stereotype441
added
the
legacy-area-front-end
copybara-service bot
pushed a commit
that referenced
this issue
Oct 30, 2023
…ications. When a field is declared in a mixin, the front end creates a synthetic getter in the mixin application class that gets the value of the mixed in field. So if a piece of code accesses the mixed in field through the mixin application class rather than through the mixin directly, the resolved member is the synthetic getter rather than a field. In order to ensure that the field remains promotable even if it is accessed through the mixin application, the logic in `OperationsCfe.isPropertyPromotable` needs to be changed so that it doesn't treat these synthetic getters as non-promotable. The old logic was essentially this: 1. If the property is not private, it's not promotable. 2. Otherwise, if the property is listed in `FieldNonPromotabilityInfo.fieldNameInfo`, it's not promotable. (This happens either if the property is not promotable for an intrinsic reason, such as being a non-final field or a concrete getter, or if it has the same name as a non-promotable property elsewhere in the library). 3. Otherwise, if the property is a getter that was lowered from an abstract field, it's promotable. 4. Otherwise, if the property is a getter that was lowered from a late field, it's promotable. 5. Otherwise, the property isn't promotable. (This was intended to cover the case where the property is an abstract getter declaration). (Although conditions 3 and 4 were tested first, since they are more efficient to test). It turns out that once conditions 1-2 have been ruled out, the property must have been declared as a method (which is being torn off), a private abstract getter, or a (possibly abstract) non-external private final field. Of these three possibilities, only the last is promotable. So this can be simplified to: (conditions 1-2 as above) 3. Otherwise, if the property is a method tear-off, it's not promotable. 4. Otherwise, if the property is an abstract getter, it's not promotable. 5. Otherwise, the property is promotable. This makes the logic easier to follow, since conditions 1-4 are now all reasons for non-promotability (rather than a mix of promotability and non-promotability reasons). It also conveniently addresses the problem with fields accessed through mixin applications, since they aren't excluded by any of conditions 1-4. (We still test conditions 3 and 4 first, since they are more efficient to test.) Fixes #53742. Fixes #53617. Fixes #53436. Bug: #53742 Change-Id: If53cb3a42a62c64468cd84fd9586b60b6b10a196 Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/330168 Cherry-pick-request: #53849 Fixes: #53849 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/331750 Reviewed-by: Erik Ernst <[email protected]> Commit-Queue: Paul Berry <[email protected]> Reviewed-by: Chloe Stefantsova <[email protected]> Reviewed-by: Lasse Nielsen <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
There are new test failures on Wire up "why not promoted" messages for failed field promotion due to conflict..
The tests
are failing on configurations
I believe what is happening is that when there's a mixin application,
Memberobjects from the mixin are being copied into the mixin application, and so those new members can't be found in the data tables used by "why not promoted" logic (because they use theMemberobjects as a key).Although the problem is manifesting as a crash, it's in fact a very benign failure, since it's merely an assertion failure; in production code the effect is simply that these failed field promotions simply won't have any associated context messages. So I'm going to move forward with the CL and fix the problem in a follow-up.
The text was updated successfully, but these errors were encountered: