<div dir="ltr">My high-level design concerns are addressed. I've made some nit-picky code comments on the patch.</div><br><div class="gmail_quote"><div dir="ltr">On Mon, Nov 7, 2016 at 9:05 AM Zachary Turner via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</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" class="gmail_msg">Wanted to ping again on this now that the LLVM Developer Conference is over and people are (presumably) back to normal.<div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">chandlerc@: Are your concerns sufficiently addressed?</div><div class="gmail_msg">Anyone: Does anyone feel strongly that the proposed syntax is or is not the way to go, based on the arguments outlined previously?</div></div><br class="gmail_msg"><div class="gmail_quote gmail_msg"><div dir="ltr" class="gmail_msg">On Wed, Nov 2, 2016 at 3:54 PM Zachary Turner <<a href="mailto:zturner@google.com" class="gmail_msg" target="_blank">zturner@google.com</a>> wrote:<br class="gmail_msg"></div><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="gmail_msg">* UDL Syntax is removed in the latest version of the <a href="https://reviews.llvm.org/D25587" class="gmail_msg" target="_blank">patch</a>. <div class="gmail_msg">* Name changed to `formatv` since `format_string` is too much to type.</div><div class="gmail_msg">*<span class="m_3799112806732434487m_2795638995749778596inbox-inbox-Apple-converted-space gmail_msg"> Added conversion operators for `std::string` and `llvm::SmallString`.</span></div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">I had some feedback offline (not on this thread, unfortunately) that it might be worth using a printf style syntax instead of this Python-esque syntax. FTR, I actually somewhat object to this, for a couple of reasons:</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">1) It makes back-reference syntax ugly. "{0} {1} {0}" is much clearer to me than "%0$ %1$ %0$". The latter syntax is also not a very well known feature of printf and so unlikely to be used by people with a printf-style implementation, whereas it's un-missable with the python-style syntax.</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">2) I don't see why we should need to specify the type of the argument with %d if the compiler knows it's an integer. Even if the we can add compile-time checking to make it error, it seems unnecessary to even encounter this situation in the first place. I believe the compiler should simply format what you give it.</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">3) One of the most useful aspects of the current approach is the ability to plug in custom formatters for application specific data types. This is not straightforward with a printf-style syntax. </div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">You might be able to hook up a template-specialization like mechanic to the processing of %s (similar to my current approach), but it's not obvious how you proceed from there to get custom format strings for individual types. For example, a formatter which can print a TimeSpan in different units depending on style options you pass in. This is especially useful when trying to print ranges where you often want to be able to specify a different separator, or control the formatting of the underlying type. (e.g. it's not clear how you would elegantly format a range of integers in hex using this style of approach).</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">I'm open to feedback here, so if you have an opinion one way or the other, please LMK.</div></div><div dir="ltr" class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"><br class="gmail_msg"><div class="gmail_quote gmail_msg"><div dir="ltr" class="gmail_msg">On Tue, Nov 1, 2016 at 5:39 AM Zachary Turner <<a href="mailto:zturner@google.com" class="gmail_msg" target="_blank">zturner@google.com</a>> wrote:<br class="gmail_msg"></div><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">The big problem i see is that to get compile time checking without the UDL we're going to have to do something like FORMAT_STRING("{0}") where this is a macro. It just seems really gross. It is true that it is harder to find the documentation, but that could be alleviated by putting all of this in its own namespace like llvm::formatv, then one could search the namespace <br class="gmail_msg"><div class="gmail_quote gmail_msg"><div dir="ltr" class="gmail_msg">On Mon, Oct 31, 2016 at 11:41 PM Sean Silva <<a href="mailto:chisophugis@gmail.com" class="gmail_msg" target="_blank">chisophugis@gmail.com</a>> wrote:<br class="gmail_msg"></div><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="gmail_msg"><div class="gmail_extra gmail_msg"><div class="gmail_quote gmail_msg">On Mon, Oct 31, 2016 at 5:21 PM, Chandler Carruth via llvm-dev <span dir="ltr" class="gmail_msg"><<a href="mailto:llvm-dev@lists.llvm.org" class="gmail_msg" target="_blank">llvm-dev@lists.llvm.org</a>></span> wrote:<br class="gmail_msg"><blockquote class="gmail_quote gmail_msg" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class="gmail_msg"><div class="gmail_quote gmail_msg"><span class="m_3799112806732434487m_2795638995749778596m_-3227045290579798300m_-627083501889168641gmail- gmail_msg"><div dir="ltr" class="gmail_msg">On Mon, Oct 31, 2016 at 3:46 PM Zachary Turner via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" class="gmail_msg" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br class="gmail_msg"></div><blockquote class="gmail_quote gmail_msg" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class="m_3799112806732434487m_2795638995749778596m_-3227045290579798300m_-627083501889168641gmail-m_8296346337731727738gmail_msg gmail_msg"><div class="m_3799112806732434487m_2795638995749778596m_-3227045290579798300m_-627083501889168641gmail-m_8296346337731727738gmail_msg gmail_msg">Hi all,</div><div class="m_3799112806732434487m_2795638995749778596m_-3227045290579798300m_-627083501889168641gmail-m_8296346337731727738gmail_msg gmail_msg"><br class="m_3799112806732434487m_2795638995749778596m_-3227045290579798300m_-627083501889168641gmail-m_8296346337731727738gmail_msg gmail_msg"></div><div class="m_3799112806732434487m_2795638995749778596m_-3227045290579798300m_-627083501889168641gmail-m_8296346337731727738gmail_msg gmail_msg">Tentatively final version is up here: <a href="https://reviews.llvm.org/D25587" class="m_3799112806732434487m_2795638995749778596m_-3227045290579798300m_-627083501889168641gmail-m_8296346337731727738gmail_msg gmail_msg" target="_blank">https://reviews.llvm.org/D25587</a></div><div class="m_3799112806732434487m_2795638995749778596m_-3227045290579798300m_-627083501889168641gmail-m_8296346337731727738gmail_msg gmail_msg"><br class="m_3799112806732434487m_2795638995749778596m_-3227045290579798300m_-627083501889168641gmail-m_8296346337731727738gmail_msg gmail_msg"></div><div class="m_3799112806732434487m_2795638995749778596m_-3227045290579798300m_-627083501889168641gmail-m_8296346337731727738gmail_msg gmail_msg">It has a verbal LGTM, but I plan to wait a bit longer just in case anyone has some additional thoughts. It's a large patch, but if you're interested, one way you can help without doing a full-blown review is to look at the large comment blocks in FormatVariadic.h and FormatProviders.h. Here I provide a formal description of the grammar of the replacement sequences and format syntax. So you can look at this without looking at the code behind it and see if you have comments just on the format language.</div><div class="m_3799112806732434487m_2795638995749778596m_-3227045290579798300m_-627083501889168641gmail-m_8296346337731727738gmail_msg gmail_msg"><br class="m_3799112806732434487m_2795638995749778596m_-3227045290579798300m_-627083501889168641gmail-m_8296346337731727738gmail_msg gmail_msg"></div><div class="m_3799112806732434487m_2795638995749778596m_-3227045290579798300m_-627083501889168641gmail-m_8296346337731727738gmail_msg gmail_msg">Here's a summary of (most) everything contained in this patch:</div><div class="m_3799112806732434487m_2795638995749778596m_-3227045290579798300m_-627083501889168641gmail-m_8296346337731727738gmail_msg gmail_msg"><br class="m_3799112806732434487m_2795638995749778596m_-3227045290579798300m_-627083501889168641gmail-m_8296346337731727738gmail_msg gmail_msg"></div><div class="m_3799112806732434487m_2795638995749778596m_-3227045290579798300m_-627083501889168641gmail-m_8296346337731727738gmail_msg gmail_msg">1) UDL Syntax for outputting to a stream or converting to a string.</div><div class="m_3799112806732434487m_2795638995749778596m_-3227045290579798300m_-627083501889168641gmail-m_8296346337731727738gmail_msg gmail_msg"><font face="monospace" class="m_3799112806732434487m_2795638995749778596m_-3227045290579798300m_-627083501889168641gmail-m_8296346337731727738gmail_msg gmail_msg"> outs() << "{0}"_fmt.stream(1)</font></div><div class="m_3799112806732434487m_2795638995749778596m_-3227045290579798300m_-627083501889168641gmail-m_8296346337731727738gmail_msg gmail_msg"><font face="monospace" class="m_3799112806732434487m_2795638995749778596m_-3227045290579798300m_-627083501889168641gmail-m_8296346337731727738gmail_msg gmail_msg"> std::string S = "{0}"_fmt.string(1);</font></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></span><div class="gmail_msg">I continue to have a strong objection to using UDLs for this (or anything else in LLVM).</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">I think this feature is poorly known by many programmers. I think it will produce error messages that are confusing and hard to debug. I think it will have a significant negative impact on compile time. I also think that it will exercise substantially less well tested parts of every host compiler for LLVM and subject us to an increased rate of mysterious host compiler bugs.</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">I also think it forces programmers to be aware of a "magical" construct that doesn't really fit with the rest of the language.</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">It isn't that any of these issues in isolation cannot be overcome, it is that I think the value provided by the UDL specifically is substantially smaller than the cost.</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">I would *very strongly* prefer that this is accomplished with "normal" C++ syntax, and that compile time checking is done with constexpr when available. I think that will give the overwhelming majority of the benefit with dramatically lower cost.</div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div></div><div dir="ltr" class="gmail_msg"><div class="gmail_extra gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg">+1, the UDL seems a bit too automagical.</div><div class="gmail_msg">`format_string("{0}", 1)` is not that much longer than `"{0}"_fmt.string(1)`, but significantly less magical.</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">Simple example: what should I type into a search engine to find the LLVM doxygen for the UDL? I know to search "llvm format_string" for the format string, but just from looking at a use of the UDL syntax it might not be clear that format_string is even called.</div></div></div></div><div dir="ltr" class="gmail_msg"><div class="gmail_extra gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">-- Sean Silva</div></div></div></div><div dir="ltr" class="gmail_msg"><div class="gmail_extra gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg"> </div><blockquote class="gmail_quote gmail_msg" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br class="gmail_msg">_______________________________________________<br class="gmail_msg">
LLVM Developers mailing list<br class="gmail_msg">
<a href="mailto:llvm-dev@lists.llvm.org" class="gmail_msg" target="_blank">llvm-dev@lists.llvm.org</a><br class="gmail_msg">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" class="gmail_msg" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br class="gmail_msg">
<br class="gmail_msg"></blockquote></div></div></div></blockquote></div></blockquote></div></div></div></blockquote></div>
_______________________________________________<br class="gmail_msg">
LLVM Developers mailing list<br class="gmail_msg">
<a href="mailto:llvm-dev@lists.llvm.org" class="gmail_msg" target="_blank">llvm-dev@lists.llvm.org</a><br class="gmail_msg">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" class="gmail_msg" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br class="gmail_msg">
</blockquote></div>