[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

Daniel Jasper djasper at google.com
Thu Sep 12 11:28:09 PDT 2013


  If we do this based on the language standard, we need to ensure that the different version of Visual Studio do the right thing under the corresponding configurations (i.e. VS 2010 if set to C++11).

  I'd probably be slightly more comfortable with a separate flag.


================
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\"") ||
----------------
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?

================
Comment at: lib/Format/Format.cpp:602
@@ -600,1 +601,3 @@
 private:
+  void maybeJoinLastTokens() {
+    size_t Last = Tokens.size() - 1;
----------------
I find "last" slightly ambiguous (could also be the last in the line, the file, ..). How about maybeJoinPreviousTokens()?

================
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) &&
----------------
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];

  ...

================
Comment at: unittests/Format/FormatTest.cpp:5501
@@ +5500,3 @@
+
+// FIXME: Handle embedded spaces in one iteration.
+//  EXPECT_EQ("_T(\"aaaaaaaaaaaaa\")\n"
----------------
Indent


http://llvm-reviews.chandlerc.com/D1640



More information about the cfe-commits mailing list