[PATCH] D25587: Introduce llvm FormatVariadic

[Bob Haarman via llvm-commits]

inglorion added inline comments.


================
Comment at: include/llvm/Support/FormatVariadic.h:152
+//             are ignored and the replacement sequence is replaced with an
+//             empty string.
+// alignment - (Optional) A string of the form:
----------------
zturner wrote:
> inglorion wrote:
> > Do you have a use case in mind for having a replacement sequence without an index, with or without the other fields?
> Not specifically, it was a choice between replacing with an empty string vs leaving it in the string as a literal.
> 
> This does raise an interesting point though, which is that I'm not currently consistent.  Sometimes I replace with empty string, sometimes I leave the invalid specifier in un-replaced.  Perhaps we should be consistent.  Always delete from the output, or always leave it in exactly.
> 
I would say this is another case where the intended behavior is not clear, so we should treat it as an error. Compared to picking one of those behaviors now and specifying that as expected, treating it as an error has the advantage that if we later come up with some clearly useful behavior for the case, we can change the specified behavior from error to whatever it is we want it to do, and it will not break any previously correct code.


https://reviews.llvm.org/D25587