ASTPrinter: remove ASTPrinter's dependency on sema. NFC #10985
Conversation
|
@swift-ci please smoke test |
1 similar comment
|
@swift-ci please smoke test |
|
Merging. @jrose-apple Please let me know if this unblocks your patch. |
| @@ -294,6 +275,9 @@ struct PrintOptions { | |||
| /// Whether to print the extensions from conforming protocols. | |||
| bool PrintExtensionFromConformingProtocols = false; | |||
|
|
|||
| std::shared_ptr<ShouldPrintChecker> CurrentPrintabilityChecker = | |||
There was a problem hiding this comment.
Small note: it seems a little funny to make this a shared_ptr rather than just an optional non-owning pointer.
There was a problem hiding this comment.
Like Optional<ShouldPrintChecker*>? does it mean we don't release the original one when when resetting the pointer?
There was a problem hiding this comment.
Just ShouldPrintChecker * is fine—if it's nullptr, you get the default behavior. Whenever someone wants to customize it, they can stack-allocate their checker and pass in the address.
There was a problem hiding this comment.
Stack allocation is usually insufficient for setting the checker because we set in the factory method of PrintOptions.
There was a problem hiding this comment.
:-/ All right. std::shared_ptr just seems very heavyweight to me, but I guess PrintOptions don't get copied around a lot and customizing this behavior doesn't happen very often anyway.
There was a problem hiding this comment.
Thanks for talking through it with me!
No description provided.