[llvm-dev] [cfe-dev] RFC: Add an llvm::vfs::OutputManager to allow Clang to virtualize compiler outputs
Duncan P. N. Exon Smith via llvm-dev
llvm-dev at lists.llvm.org
Thu Jan 28 18:12:24 PST 2021
A few follow-ups:
> On 2021 Jan 28, at 11:24, Duncan P. N. Exon Smith via llvm-dev <llvm-dev at lists.llvm.org> wrote:
>
> Thirdly, there's some boilerplate related to lifetimes of multiple destinations. Probably having an explicit `MirroringOutputDestination` would be better.
This is what it looks like if the mirroring stuff is localized to MirroringOutputDestination:
https://github.com/dexonsmith/llvm-project/commit/183cd0923497c83038a561c11b38553539d0e7a8 <https://github.com/dexonsmith/llvm-project/commit/183cd0923497c83038a561c11b38553539d0e7a8>
It's cleaner / more clear / less error-prone. I'll update the patch when I get a moment.
> Probably it's better to have the OutputBackend return an Output directly (and get rid of the OutputManager).
I haven't had a chance to prototype this, but I imagine it's fairly straightforward. This is just moving OutputManager::createOutput over to OutputBackend, and changing createDestination to take a PartialOutputConfig (which can be ignored or not).
> - Is `Output::getPath()` an abstraction leak?
Removing Output::getConfig is easy:
https://github.com/dexonsmith/llvm-project/commit/1903a40f253dbbc1ad0e701a634951561c550b08 <https://github.com/dexonsmith/llvm-project/commit/1903a40f253dbbc1ad0e701a634951561c550b08>
I'll remove it from the patch when I get a moment.
Removing Output::getPath is a bit harder, but doable:
https://github.com/dexonsmith/llvm-project/commit/10ffef5090bde4b55b4abd099cd0233663cbf448 <https://github.com/dexonsmith/llvm-project/commit/10ffef5090bde4b55b4abd099cd0233663cbf448>
I think this is the right thing to do, per the longer discussion in my previous email... but it's a bit more awkward.
Output also doesn't need to be in a unique_ptr, as long as it's move-only. Here's what it looks like without the extra level indirection:
https://github.com/dexonsmith/llvm-project/commit/125148ec2fde7fdbc4e70a9799dc263dd9f20ed8 <https://github.com/dexonsmith/llvm-project/commit/125148ec2fde7fdbc4e70a9799dc263dd9f20ed8>
Probably it should have a NoneType constructor / etc., since it's fairly Optional-like.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210128/6bcc5ad1/attachment.html>
More information about the llvm-dev
mailing list