[PATCH] When in pre-c++11 mode, treat _T("xxx") as a single string literal, repeat the _T() part around each fragment. This addresses http://llvm.org/PR17122
Alexander Kornienko
alexfh at google.com
Fri Sep 13 05:40:07 PDT 2013
================
Comment at: lib/Format/ContinuationIndenter.cpp:679
@@ +678,3 @@
+ if ((Text.startswith(Prefix = "_T(\"") && Text.endswith(Postfix = "\")")) ||
+ (Text.endswith(Postfix = "\"") && (Text.startswith(Prefix = "\"") ||
+ Text.startswith(Prefix = "u\"") ||
----------------
Daniel Jasper wrote:
> Who came up with this format?
>
> Also these seem like a lot of string comparisons (albeit short strings). Would it make sense to precalculate this to get it out of the critical path?
Clang-format produced this format before I added parens around the second operand of ||.
Fixed.
================
Comment at: lib/Format/Format.cpp:602
@@ -600,1 +601,3 @@
private:
+ void maybeJoinLastTokens() {
+ size_t Last = Tokens.size() - 1;
----------------
Daniel Jasper wrote:
> I find "last" slightly ambiguous (could also be the last in the line, the file, ..). How about maybeJoinPreviousTokens()?
Ok.
================
Comment at: lib/Format/Format.cpp:604
@@ +603,3 @@
+ size_t Last = Tokens.size() - 1;
+ if (Style.Standard != format::FormatStyle::LS_Cpp11 && Last > 2 &&
+ Tokens[Last]->is(tok::r_paren) &&
----------------
Daniel Jasper wrote:
> I think this would be slightly easier to understand (and possibly extend) if written like:
>
> size_t Last = Tokens.size() - 1; // Or LastIndex or something...
> if (Last < 2 || Tokens.back()->isNot(tok::r_paren))
> return;
>
> if (Tokens[Last - 1]->isNot(tok::string_literal) ||
> Tokens[Last - 1]->IsMultiline)
> return;
> FormatToken *String = Tokens[Last - 1];
>
> if (Tokens[Last - 2]->isNot(tok::l_paren) ||
> Tokens[Last - 3]->TokenText != "_T")
> return;
> FormatToken *Macro = Tokens[Last - 3];
>
> ...
Not that I like it more, but if you prefer this layout, I've implemented a variant of it.
================
Comment at: unittests/Format/FormatTest.cpp:5501
@@ +5500,3 @@
+
+// FIXME: Handle embedded spaces in one iteration.
+// EXPECT_EQ("_T(\"aaaaaaaaaaaaa\")\n"
----------------
Daniel Jasper wrote:
> Indent
Done.
http://llvm-reviews.chandlerc.com/D1640
More information about the cfe-commits
mailing list