[PATCH] D44790: [clang-format] Fix ObjC style guesser to also iterate over child lines

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton created this revision.
benhamilton added reviewers: djasper, jolesiak, Wizard.
Herald added subscribers: cfe-commits, klimek.

When I wrote `ObjCHeaderStyleGuesser`, I incorrectly assumed the
correct way to iterate over all tokens in `AnnotatedLine` was to
iterate over the linked list tokens starting with
`AnnotatedLine::First`.

However, `AnnotatedLine` also contains a vector
`AnnotedLine::Children` with child `AnnotedLine`s which have their own
tokens which we need to iterate over.

Because I didn't iterate over the tokens in the children lines, the
ObjC style guesser would fail on syntax like:

  #define FOO ({ NSString *s = ... })

as the statement(s) inside { ... } are child lines.

This fixes the bug and adds a test. I confirmed the test
failed before the fix, and passed after the fix.

Test Plan: New tests added. Ran tests with:

  % make -j12 FormatTests && ./tools/clang/unittests/Format/FormatTests


Repository:
  rC Clang

https://reviews.llvm.org/D44790

Files:
  lib/Format/Format.cpp
  unittests/Format/FormatTest.cpp


Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12162,6 +12162,13 @@
       guessLanguage("foo.h", "int(^foo[(kNumEntries + 10)])(char, float);"));
 }
 
+TEST_F(FormatTest, GuessLanguageWithChildLines) {
+  EXPECT_EQ(FormatStyle::LK_Cpp,
+            guessLanguage("foo.h", "#define FOO ({ std::string s; })"));
+  EXPECT_EQ(FormatStyle::LK_ObjC,
+            guessLanguage("foo.h", "#define FOO ({ NSString *s; })"));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1514,8 +1514,8 @@
         "UIView",
     };
 
-    for (auto &Line : AnnotatedLines) {
-      for (FormatToken *FormatTok = Line->First; FormatTok;
+    auto CheckLineTokens = [&Keywords](const AnnotatedLine &Line) {
+      for (const FormatToken *FormatTok = Line.First; FormatTok;
            FormatTok = FormatTok->Next) {
         if ((FormatTok->Previous && FormatTok->Previous->is(tok::at) &&
              (FormatTok->isObjCAtKeyword(tok::objc_interface) ||
@@ -1536,6 +1536,15 @@
           return true;
         }
       }
+      return false;
+    };
+    for (auto Line : AnnotatedLines) {
+      if (CheckLineTokens(*Line))
+        return true;
+      for (auto ChildLine : Line->Children) {
+        if (CheckLineTokens(*ChildLine))
+          return true;
+      }
     }
     return false;
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D44790.139464.patch
Type: text/x-patch
Size: 1597 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180322/d381099d/attachment.bin>