[PATCH] D44831: [clang-format] Refine ObjC guesser to handle child lines of child lines

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton created this revision.
benhamilton added a reviewer: djasper.
Herald added subscribers: cfe-commits, klimek.

This fixes an issue brought up by djasper@ in his review of 
https://reviews.llvm.org/D44790. We
handled top-level child lines, but if those child lines themselves
had child lines, we didn't handle them.

Rather than use recursion (which could blow out the stack), I use a
DenseSet to hold the set of lines we haven't yet checked (since order
doesn't matter), and update the set to add the children of each
line as we check it.

Test Plan: New tests added. Confirmed tests failed before fix

  and passed after fix.


Repository:
  rC Clang

https://reviews.llvm.org/D44831

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


Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12171,10 +12171,12 @@
             guessLanguage("foo.h", "#define FOO ({ std::string s; })"));
   EXPECT_EQ(FormatStyle::LK_ObjC,
             guessLanguage("foo.h", "#define FOO ({ NSString *s; })"));
-  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; }) })"));
+  EXPECT_EQ(
+      FormatStyle::LK_Cpp,
+      guessLanguage("foo.h", "#define FOO ({ foo(); ({ std::string s; }) })"));
+  EXPECT_EQ(
+      FormatStyle::LK_ObjC,
+      guessLanguage("foo.h", "#define FOO ({ foo(); ({ NSString *s; }) })"));
 }
 
 } // end namespace
Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -31,6 +31,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/VirtualFileSystem.h"
 #include "clang/Lex/Lexer.h"
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Allocator.h"
@@ -1538,13 +1539,18 @@
       }
       return false;
     };
-    for (auto Line : AnnotatedLines) {
-      if (LineContainsObjCCode(*Line))
+    llvm::DenseSet<AnnotatedLine *> LinesToCheckSet;
+    LinesToCheckSet.reserve(AnnotatedLines.size());
+    LinesToCheckSet.insert(AnnotatedLines.begin(), AnnotatedLines.end());
+    while (!LinesToCheckSet.empty()) {
+      const auto NextLineIter = LinesToCheckSet.begin();
+      const auto NextLine = *NextLineIter;
+      if (LineContainsObjCCode(*NextLine))
         return true;
-      for (auto ChildLine : Line->Children) {
-        if (LineContainsObjCCode(*ChildLine))
-          return true;
-      }
+      LinesToCheckSet.erase(NextLineIter);
+      LinesToCheckSet.reserve(NextLine->Children.size());
+      LinesToCheckSet.insert(NextLine->Children.begin(),
+                             NextLine->Children.end());
     }
     return false;
   }



Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12171,10 +12171,12 @@
             guessLanguage("foo.h", "#define FOO ({ std::string s; })"));
   EXPECT_EQ(FormatStyle::LK_ObjC,
             guessLanguage("foo.h", "#define FOO ({ NSString *s; })"));
-  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; }) })"));
+  EXPECT_EQ(
+      FormatStyle::LK_Cpp,
+      guessLanguage("foo.h", "#define FOO ({ foo(); ({ std::string s; }) })"));
+  EXPECT_EQ(
+      FormatStyle::LK_ObjC,
+      guessLanguage("foo.h", "#define FOO ({ foo(); ({ NSString *s; }) })"));
 }
 
 } // end namespace
Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -31,6 +31,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/VirtualFileSystem.h"
 #include "clang/Lex/Lexer.h"
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Allocator.h"
@@ -1538,13 +1539,18 @@
       }
       return false;
     };
-    for (auto Line : AnnotatedLines) {
-      if (LineContainsObjCCode(*Line))
+    llvm::DenseSet<AnnotatedLine *> LinesToCheckSet;
+    LinesToCheckSet.reserve(AnnotatedLines.size());
+    LinesToCheckSet.insert(AnnotatedLines.begin(), AnnotatedLines.end());
+    while (!LinesToCheckSet.empty()) {
+      const auto NextLineIter = LinesToCheckSet.begin();
+      const auto NextLine = *NextLineIter;
+      if (LineContainsObjCCode(*NextLine))
         return true;
-      for (auto ChildLine : Line->Children) {
-        if (LineContainsObjCCode(*ChildLine))
-          return true;
-      }
+      LinesToCheckSet.erase(NextLineIter);
+      LinesToCheckSet.reserve(NextLine->Children.size());
+      LinesToCheckSet.insert(NextLine->Children.begin(),
+                             NextLine->Children.end());
     }
     return false;
   }

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