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; }
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; }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits