Missing playground results for property accessors in a class wrapped in a struct #77530
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
abertelrud
requested review from
hborla,
slavapestov and
xedin
as code owners
|
@swift-ci Please smoke test |
abertelrud
changed the title
dbukowski
reviewed
Nov 11, 2024
chrismiles
approved these changes
Nov 12, 2024
…lass wrapped in a struct When a decl that has properties with accessors (`get`, `set`, `willSet`, `didSet`, `subscript(index:)`, etc) is nested inside another type, those accessor implementations aren't playground-transformed. The reason is that the playground transform uses an `ASTWalker` to get to the top-level structure, but then once it’s inside a type, it does directly nested `transformDecl()` calls. And that inner check is missing a case for accessors. This change adds an else-clause that checks if the declaration is a `VarDecl`, and if so, calls `transformDecl()` on each accessor. It's unfortunate that the playground transform reaches decls in two different ways (using an `ASTWalker` to get to the top-level decls but then using directly nested calls below). That issue seems to be worth resolving at some point (perhaps by using the `ASTWalker` for the whole traversal?)... but that would be a larger change requiring more though, and so the missing results being fixed here are worth addressing with a safer short-term fix. rdar://139656464
abertelrud
force-pushed
the
eng/anders/139656464-nested-accessor-playground-results
branch
from
53cc8fc to
31f75c6
Compare
|
@swift-ci Please smoke test |
|
@xedin I've updated the PR — could I ask for another look? Thank you! |
xedin
approved these changes
Dec 5, 2024
|
@swift-ci Please smoke test linux |
abertelrud
deleted the
eng/anders/139656464-nested-accessor-playground-results
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When a decl that has properties with accessors (
get,set,willSet,didSet, etc) is nested inside another type, those accessor implementations aren't playground-transformed.Details
The reason is that the playground transform uses an
ASTWalkerto get to the top-level structure, but then once it’s inside a type, it does directly nestedtransformDecl()calls. And that inner check is missing a case for accessors.This is a follow-on fix to #77498.
Future work
It's unfortunate that the playground transform reaches decls in two different ways (using an
ASTWalkerto get to the top-level decls but then using directly nested calls below). This seems to be worth resolving at some point (perhaps by using theASTWalkerfor the whole traversal?), but that’s a larger change, and so this is worth addressing with a safer short-term fix.Changes
VarDecl, and if so, callstransformDecl()on each accessor in PlaygroundTransform.cpprdar://139656464