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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to