-
Notifications
You must be signed in to change notification settings - Fork 10.6k
[ParseableInterface] Fix printing for properties with initializers and observers #21269
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
[ParseableInterface] Fix printing for properties with initializers and observers #21269
Conversation
@swift-ci please test |
e943288
to
b828ea3
Compare
lib/AST/ASTPrinter.cpp
Outdated
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.
Hm, this if
should probably be guarding the next section…
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.
Yeah, this was left over from the original implementation. I'll just go ahead and change it. Should we even have the option to turn it off, now that var initializers are part of the public interface anyway?
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.
I…guess not? Although I suppose we could have an option that chooses between "all of them" and "only the ones that are public".
Build failed |
lib/AST/ASTPrinter.cpp
Outdated
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.
Can you add a comment stating why this whole mess is necessary?
lib/AST/ASTPrinter.cpp
Outdated
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.
This seems like it'll print the same entry twice when there are two VarDecls in the same entry.
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.
It might just be more profitable and cleaner to figure out when we print a single VarDecl without going through the pattern binding. That's what's really happening here.
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 _hasInitialValue
is suspicious. We shouldn't present that knowledge in a resilient struct. (And arguably not in a fragile struct either, but let's not worry about that right now.)
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 makes sense -- that's the existing behavior, so I'll fix that in a follow-up patch.
Build failed |
b828ea3
to
2f13f1b
Compare
@jrose-apple This should be much simpler. |
Don’t print var initializers if `Options.VarInitializers` is false.
2f13f1b
to
05f7cdb
Compare
… pattern bindings
05f7cdb
to
b3e6f1a
Compare
Your test case got lost. |
b3e6f1a
to
0b62614
Compare
@jrose-apple Thanks! |
@swift-ci please test and merge |
…h storage Instead of only printing through the pattern binding, and potentially missing stored properties with property observers, defer to the pattern binding for all stored properties, and print accessors if applicable while printing pattern bindings.
0b62614
to
7fe74b2
Compare
@swift-ci please test and merge |
@swift-ci please test |
Build failed |
Build failed |
Instead of only printing through the pattern binding, and potentially missing stored properties with property observers, defer to the pattern binding for all stored properties, and print accessors if applicable while printing pattern bindings.
Fallout from rdar://43815861