mprobst marked an inline comment as done.

================
Comment at: lib/Format/FormatTokenLexer.cpp:231
@@ -230,3 +230,3 @@
 
 void FormatTokenLexer::tryParseTemplateString() {
   FormatToken *BacktickToken = Tokens.back();
----------------
djasper wrote:
> I think, this could now use an elaborate comment on what it is actually 
> trying to parse. And part of that should probably make it into the patch 
> description.
Done. Also, it turns out my attempt here was overly optimistic, I actually need 
a proper state stack to handle this correctly.

================
Comment at: lib/Format/FormatTokenLexer.cpp:246
@@ +245,3 @@
+    if (Offset + 1 < Lex->getBuffer().end() && *Offset == '$' &&
+        *(Offset + 1) == '{') {
+      // '${' introduces an expression interpolation in the template string.
----------------
djasper wrote:
> Maybe use "Offset[1]" instead of "*(Offset + 1)"?
Is somePointer[1] idiomatic C for "the next element after this pointer"? To me 
`+ 1` seemed like a better expression of intent, if a bit more wordy. Done in 
any case.

================
Comment at: lib/Format/TokenAnnotator.cpp:1821
@@ -1819,1 +1820,3 @@
+        (Right.is(TT_TemplateString) && Right.TokenText.startswith("}")))
+      return 100;
   }
----------------
djasper wrote:
> mprobst wrote:
> > I've chosen 100 by fair dice roll. Please advise on how to pick a 
> > reasonable penalty :-)
> In what cases would you actually want to put a line break here? Presumably, 
> if otherwise the column limit is violated. Are there any other cases?
No, I think breaking in the template string should probably be a last resort. 
It's IMHO conceptually tighter than string concatenation using `+`. That being 
said, I have no idea what number that should translate into :-)


https://reviews.llvm.org/D22431



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to