<div dir="ltr"><a href="https://reviews.llvm.org/D49170">https://reviews.llvm.org/D49170</a> shows that approach.<br><div><br></div><div>I've updated clangd's <a href="https://reviews.llvm.org/D49008">https://reviews.llvm.org/D49008</a> to work around this when wrapping formatv, so we're not blocked by this either way.</div></div><br><div class="gmail_quote"><div dir="ltr">On Tue, Jul 10, 2018 at 7:57 PM Sam McCall <<a href="mailto:sammccall@google.com">sammccall@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="auto">Happy to put together a patch for that, deleting the template seems like it'd give good diagnostics.<div dir="auto"><br><div dir="auto">It still requires changes to formatv, so I guess this is preferred syntax rather than complexity.</div><div dir="auto">I'd like to understand why move semantics aren't explicit enough, when they are for toString.</div><div dir="auto"><br></div><div dir="auto">In any case, we can make clangd's log functions add the adapter automatically, so the explicit/noise tradeoff can be tweaked for that use case.</div></div></div><br><div class="gmail_quote"><div dir="ltr">On Tue, Jul 10, 2018, 18:29 Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">The more I think about it the more I feel like the explicit format adapter is the way to go. To prevent it from crashing if someone passes by value, can we just explicitly delete the instantiation of `formatProvider<Error>`?<br><div class="gmail_quote"><div dir="ltr">On Tue, Jul 10, 2018 at 9:22 AM Sam McCall <<a href="mailto:sammccall@google.com" rel="noreferrer" target="_blank">sammccall@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="auto"><div><div class="gmail_quote"><div dir="ltr">On Tue, Jul 10, 2018, 18:04 David Blaikie <<a href="mailto:dblaikie@gmail.com" rel="noreferrer" target="_blank">dblaikie@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">That seems plausible (op<< for Error) - but does that handle the case mentioned above "we may not actually want to produce the string (eg: if the Logger implementation decides to filter out the formatv_object_base based on level)" - is op<< still called even with this filtering? (seems like if it was, then it wouldn't avoid the stringification costs? but if it doesn't call the op<< then it'll still fail the consume semantic requirement of llvm::Error)<br></div></blockquote></div></div><div dir="auto">Ack, you're right. If you don't print the formatv_object, the Error isn't consumed. Worse, if you print it twice, the second call sees a moved-From error. Fundamentally, a non-const <<Error can't be used in a const <<formatv_object.</div><div dir="auto"><br></div><div dir="auto">Thanks for pointing this out. I think D49129 is a dead end.</div><div dir="auto">(Why can't Error just be a normal value type... so hard to reason about :-( )</div></div><div dir="auto"><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_quote"><div dir="ltr">On Tue, Jul 10, 2018 at 7:03 AM Sam McCall via Phabricator <<a href="mailto:reviews@reviews.llvm.org" rel="noreferrer noreferrer" target="_blank">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">sammccall added a comment.<br>
<br>
An alternate approach in <a href="https://reviews.llvm.org/D49129" rel="noreferrer noreferrer noreferrer" target="_blank">https://reviews.llvm.org/D49129</a>: give `Error` an overloaded `operator<<` that has the same consume semantics at `toString` when passed rvalues.<br>
<br>
This makes `formatv`, `to_string`, `toString` all do basically the same thing (which is the only sensible thing to do when passed an rvalue).<br>
<br>
It does require minor changes to `formatv` and `to_string` to make them propagate the right kind of reference, but these seem reasonable.<br>
<br>
Overall I think I like that approach better than this one (more consistent, Error handling lives in Error.h). WDYT?<br>
<br>
<br>
Repository:<br>
rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D49013" rel="noreferrer noreferrer noreferrer" target="_blank">https://reviews.llvm.org/D49013</a><br>
<br>
<br>
<br>
</blockquote></div></div>
</blockquote></div></div></div>
</blockquote></div>
</blockquote></div>
</blockquote></div>