[PrintAsObjC] Pull the actual decl/type printer out to a separate file #27163
Conversation
I've tried several times to separate the type printing from the decl-printing and found it tricky, but at least both can be separated from the logic of walking the top-level decls in a module and scheduling them appropriately. No functionality change.
No functionality change
|
@swift-ci Please smoke test |
|
@swift-ci Please test source compatibility |
All persistent info should live in the parent DeclAndTypePrinter, not in the Implementation helper class. We can move the rarely-used members out too even if they're not lazily-populated.
|
@swift-ci Please smoke test |
|
@swift-ci Please test source compatibility |
It can get it from the owning DeclAndTypePrinter, and most of the time it's only using it to get the ASTContext anyway. No functionality change.
|
@swift-ci Please smoke test |
There was a problem hiding this comment.
Github didn't make it easy to understand what happened to DeclAndTypePrinter.cpp, so I built a gist to show the differences: https://gist.github.com/brentdax/ec2d1070c867de20274f02f32402778a/revisions?diff=unified
I still didn't give that diff a really detailed read, but skimming it LGTM; I just have one small request.
|
|
||
| using NameAndOptional = std::pair<StringRef, bool>; | ||
| llvm::DenseMap<std::pair<Identifier, Identifier>, NameAndOptional> |
There was a problem hiding this comment.
Could you add a doc comment explaining how to interpret this member variable? The type and name are far from self-explanatory.
|
I was expecting reviewers to skip the first PR since it's "just" moving things, or maybe only looking at what changed on the PrintAsObjC and DeclAndTypePrinter header sides. But yeah, comment is fair. |
And comment the other cached members a little better.
|
@swift-ci Please smoke test |
I've tried several times to separate the type printing from the decl-printing and found it tricky, but at least both can be separated from the logic of walking the top-level decls in a module and scheduling them appropriately. No functionality change.