<html><head><meta http-equiv="Content-Type" content="text/html; charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">A few follow-ups:<br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On 2021 Jan 28, at 11:24, Duncan P. N. Exon Smith via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" class="">llvm-dev@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><meta http-equiv="Content-Type" content="text/html; charset=utf-8" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class=""><div class="">Thirdly, there's some boilerplate related to lifetimes of multiple destinations. Probably having an explicit `MirroringOutputDestination` would be better.</div></div></div></div></blockquote><div><br class=""></div><div>This is what it looks like if the mirroring stuff is localized to MirroringOutputDestination:</div><div><a href="https://github.com/dexonsmith/llvm-project/commit/183cd0923497c83038a561c11b38553539d0e7a8" class="">https://github.com/dexonsmith/llvm-project/commit/183cd0923497c83038a561c11b38553539d0e7a8</a></div><div>It's cleaner / more clear / less error-prone. I'll update the patch when I get a moment.</div><div><br class=""></div><div><blockquote type="cite" class=""><div class="" style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;"><div class=""><div class="">Probably it's better to have the OutputBackend return an Output directly (and get rid of the OutputManager).</div></div></div></blockquote><div class=""><div class="" style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;"><div class=""><div class=""><br class=""></div><div class="">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).</div></div></div></div></div><br class=""><blockquote type="cite" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class=""><div class=""><div class="">- Is `Output::getPath()` an abstraction leak?</div></div></div></div></blockquote><div><br class=""></div><div>Removing Output::getConfig is easy:</div><div><a href="https://github.com/dexonsmith/llvm-project/commit/1903a40f253dbbc1ad0e701a634951561c550b08" class="">https://github.com/dexonsmith/llvm-project/commit/1903a40f253dbbc1ad0e701a634951561c550b08</a></div><div>I'll remove it from the patch when I get a moment.</div><div><br class=""></div><div>Removing Output::getPath is a bit harder, but doable:</div><div><a href="https://github.com/dexonsmith/llvm-project/commit/10ffef5090bde4b55b4abd099cd0233663cbf448" class="">https://github.com/dexonsmith/llvm-project/commit/10ffef5090bde4b55b4abd099cd0233663cbf448</a></div></div><div class="">I think this is the right thing to do, per the longer discussion in my previous email... but it's a bit more awkward.</div><br class=""><div class="">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:</div><div class=""><a href="https://github.com/dexonsmith/llvm-project/commit/125148ec2fde7fdbc4e70a9799dc263dd9f20ed8" class="">https://github.com/dexonsmith/llvm-project/commit/125148ec2fde7fdbc4e70a9799dc263dd9f20ed8</a></div><div class="">Probably it should have a NoneType constructor / etc., since it's fairly Optional-like.</div></body></html>