[PATCH] D44831: [clang-format] Refine ObjC guesser to handle child lines of child lines
This revision was automatically updated to reflect the committed changes.
Closed by commit rC328628: [clang-format] Refine ObjC guesser to handle child
lines of child lines (authored by benhamilton, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D44831?vs=139929&id=139932#toc
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
@@ -12159,6 +12159,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 ({ 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
@@ -1514,8 +1514,8 @@
"UIView",
};
-auto LineContainsObjCCode = [&Keywords](const AnnotatedLine &Line) {
- for (const FormatToken *FormatTok = Line.First; FormatTok;
+for (auto Line : AnnotatedLines) {
+ for (const FormatToken *FormatTok = Line->First; FormatTok;
FormatTok = FormatTok->Next) {
if ((FormatTok->Previous && FormatTok->Previous->is(tok::at) &&
(FormatTok->isObjCAtKeyword(tok::objc_interface) ||
@@ -1535,14 +1535,7 @@
TT_ObjCMethodSpecifier, TT_ObjCProperty)) {
return true;
}
- }
- return false;
-};
-for (auto Line : AnnotatedLines) {
- if (LineContainsObjCCode(*Line))
-return true;
- for (auto ChildLine : Line->Children) {
-if (LineContainsObjCCode(*ChildLine))
+if (guessIsObjC(Line->Children, Keywords))
return true;
}
}
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12159,6 +12159,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 ({ 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
@@ -1514,8 +1514,8 @@
"UIView",
};
-auto LineContainsObjCCode = [&Keywords](const AnnotatedLine &Line) {
- for (const FormatToken *FormatTok = Line.First; FormatTok;
+for (auto Line : AnnotatedLines) {
+ for (const FormatToken *FormatTok = Line->First; FormatTok;
FormatTok = FormatTok->Next) {
if ((FormatTok->Previous && FormatTok->Previous->is(tok::at) &&
(FormatTok->isObjCAtKeyword(tok::objc_interface) ||
@@ -1535,14 +1535,7 @@
TT_ObjCMethodSpecifier, TT_ObjCProperty)) {
return true;
}
- }
- return false;
-};
-for (auto Line : AnnotatedLines) {
- if (LineContainsObjCCode(*Line))
-return true;
- for (auto ChildLine : Line->Children) {
-if (LineContainsObjCCode(*ChildLine))
+if (guessIsObjC(Line->Children, Keywords))
return true;
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44831: [clang-format] Refine ObjC guesser to handle child lines of child lines
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. Repository: rC Clang https://reviews.llvm.org/D44831 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44831: [clang-format] Refine ObjC guesser to handle child lines of child lines
benhamilton added inline comments.
Comment at: lib/Format/Format.cpp:1449
const AdditionalKeywords &Keywords) {
+for (auto Line : AnnotatedLines)
+ if (LineContainsObjCCode(*Line, Keywords))
djasper wrote:
> I would not create a second function here. Just iterate over the tokens here
> and call guessIsObjC recursively with Line->Children. That means we need one
> less for loop overall, I think (I might be missing something).
Good call, done.
I don't know why I didn't do that first! For some reason, my brain thought
`Line.Children` was an incompatible type..
Comment at: lib/Format/Format.cpp:1455
+
+ static bool LineContainsObjCCode(const AnnotatedLine &Line,
+ const AdditionalKeywords &Keywords) {
djasper wrote:
> Convention would be lineContainsObjCCode.
Removed.
Repository:
rC Clang
https://reviews.llvm.org/D44831
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44831: [clang-format] Refine ObjC guesser to handle child lines of child lines
benhamilton updated this revision to Diff 139929.
benhamilton marked 2 inline comments as done.
benhamilton added a comment.
- Simplify recursion. Remove static method LineContainsObjCCode().
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,6 +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 ({ 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
@@ -1514,8 +1514,8 @@
"UIView",
};
-auto LineContainsObjCCode = [&Keywords](const AnnotatedLine &Line) {
- for (const FormatToken *FormatTok = Line.First; FormatTok;
+for (auto Line : AnnotatedLines) {
+ for (const FormatToken *FormatTok = Line->First; FormatTok;
FormatTok = FormatTok->Next) {
if ((FormatTok->Previous && FormatTok->Previous->is(tok::at) &&
(FormatTok->isObjCAtKeyword(tok::objc_interface) ||
@@ -1535,14 +1535,7 @@
TT_ObjCMethodSpecifier, TT_ObjCProperty)) {
return true;
}
- }
- return false;
-};
-for (auto Line : AnnotatedLines) {
- if (LineContainsObjCCode(*Line))
-return true;
- for (auto ChildLine : Line->Children) {
-if (LineContainsObjCCode(*ChildLine))
+if (guessIsObjC(Line->Children, Keywords))
return true;
}
}
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12171,6 +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 ({ 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
@@ -1514,8 +1514,8 @@
"UIView",
};
-auto LineContainsObjCCode = [&Keywords](const AnnotatedLine &Line) {
- for (const FormatToken *FormatTok = Line.First; FormatTok;
+for (auto Line : AnnotatedLines) {
+ for (const FormatToken *FormatTok = Line->First; FormatTok;
FormatTok = FormatTok->Next) {
if ((FormatTok->Previous && FormatTok->Previous->is(tok::at) &&
(FormatTok->isObjCAtKeyword(tok::objc_interface) ||
@@ -1535,14 +1535,7 @@
TT_ObjCMethodSpecifier, TT_ObjCProperty)) {
return true;
}
- }
- return false;
-};
-for (auto Line : AnnotatedLines) {
- if (LineContainsObjCCode(*Line))
-return true;
- for (auto ChildLine : Line->Children) {
-if (LineContainsObjCCode(*ChildLine))
+if (guessIsObjC(Line->Children, Keywords))
return true;
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44831: [clang-format] Refine ObjC guesser to handle child lines of child lines
djasper added inline comments.
Comment at: lib/Format/Format.cpp:1449
const AdditionalKeywords &Keywords) {
+for (auto Line : AnnotatedLines)
+ if (LineContainsObjCCode(*Line, Keywords))
I would not create a second function here. Just iterate over the tokens here
and call guessIsObjC recursively with Line->Children. That means we need one
less for loop overall, I think (I might be missing something).
Comment at: lib/Format/Format.cpp:1455
+
+ static bool LineContainsObjCCode(const AnnotatedLine &Line,
+ const AdditionalKeywords &Keywords) {
Convention would be lineContainsObjCCode.
Repository:
rC Clang
https://reviews.llvm.org/D44831
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44831: [clang-format] Refine ObjC guesser to handle child lines of child lines
benhamilton updated this revision to Diff 139804.
benhamilton added a comment.
- Remove stray semicolon.
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,6 +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 ({ 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
@@ -1446,6 +1446,14 @@
private:
static bool guessIsObjC(const SmallVectorImpl
&AnnotatedLines,
const AdditionalKeywords &Keywords) {
+for (auto Line : AnnotatedLines)
+ if (LineContainsObjCCode(*Line, Keywords))
+return true;
+return false;
+ }
+
+ static bool LineContainsObjCCode(const AnnotatedLine &Line,
+ const AdditionalKeywords &Keywords) {
// Keep this array sorted, since we are binary searching over it.
static constexpr llvm::StringLiteral FoundationIdentifiers[] = {
"CGFloat",
@@ -1514,37 +1522,29 @@
"UIView",
};
-auto LineContainsObjCCode = [&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) ||
- FormatTok->isObjCAtKeyword(tok::objc_implementation) ||
- FormatTok->isObjCAtKeyword(tok::objc_protocol) ||
- FormatTok->isObjCAtKeyword(tok::objc_end) ||
- FormatTok->isOneOf(tok::numeric_constant, tok::l_square,
- tok::l_brace))) ||
-(FormatTok->Tok.isAnyIdentifier() &&
- std::binary_search(std::begin(FoundationIdentifiers),
-std::end(FoundationIdentifiers),
-FormatTok->TokenText)) ||
-FormatTok->is(TT_ObjCStringLiteral) ||
-FormatTok->isOneOf(Keywords.kw_NS_ENUM, Keywords.kw_NS_OPTIONS,
- TT_ObjCBlockLBrace, TT_ObjCBlockLParen,
- TT_ObjCDecl, TT_ObjCForIn, TT_ObjCMethodExpr,
- TT_ObjCMethodSpecifier, TT_ObjCProperty)) {
- return true;
-}
- }
- return false;
-};
-for (auto Line : AnnotatedLines) {
- if (LineContainsObjCCode(*Line))
+for (const FormatToken *FormatTok = Line.First; FormatTok;
+ FormatTok = FormatTok->Next) {
+ if ((FormatTok->Previous && FormatTok->Previous->is(tok::at) &&
+ (FormatTok->isObjCAtKeyword(tok::objc_interface) ||
+FormatTok->isObjCAtKeyword(tok::objc_implementation) ||
+FormatTok->isObjCAtKeyword(tok::objc_protocol) ||
+FormatTok->isObjCAtKeyword(tok::objc_end) ||
+FormatTok->isOneOf(tok::numeric_constant, tok::l_square,
+ tok::l_brace))) ||
+ (FormatTok->Tok.isAnyIdentifier() &&
+ std::binary_search(std::begin(FoundationIdentifiers),
+ std::end(FoundationIdentifiers),
+ FormatTok->TokenText)) ||
+ FormatTok->is(TT_ObjCStringLiteral) ||
+ FormatTok->isOneOf(Keywords.kw_NS_ENUM, Keywords.kw_NS_OPTIONS,
+ TT_ObjCBlockLBrace, TT_ObjCBlockLParen,
+ TT_ObjCDecl, TT_ObjCForIn, TT_ObjCMethodExpr,
+ TT_ObjCMethodSpecifier, TT_ObjCProperty)) {
return true;
- for (auto ChildLine : Line->Children) {
-if (LineContainsObjCCode(*ChildLine))
- return true;
}
+ for (auto ChildLine : Line.Children)
+if (LineContainsObjCCode(*ChildLine, Keywords))
+ return true;
}
return false;
}
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12171,6 +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 ({ foo();
[PATCH] D44831: [clang-format] Refine ObjC guesser to handle child lines of child lines
benhamilton updated this revision to Diff 139802.
benhamilton added a comment.
- Use recursion. Split lambda out into its own static method since recursion on
lambdas is quite ugly until C++14
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,6 +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 ({ 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
@@ -1446,6 +1446,14 @@
private:
static bool guessIsObjC(const SmallVectorImpl
&AnnotatedLines,
const AdditionalKeywords &Keywords) {
+for (auto Line : AnnotatedLines)
+ if (LineContainsObjCCode(*Line, Keywords))
+return true;
+return false;
+ }
+
+ static bool LineContainsObjCCode(const AnnotatedLine &Line,
+ const AdditionalKeywords &Keywords) {
// Keep this array sorted, since we are binary searching over it.
static constexpr llvm::StringLiteral FoundationIdentifiers[] = {
"CGFloat",
@@ -1514,40 +1522,32 @@
"UIView",
};
-auto LineContainsObjCCode = [&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) ||
- FormatTok->isObjCAtKeyword(tok::objc_implementation) ||
- FormatTok->isObjCAtKeyword(tok::objc_protocol) ||
- FormatTok->isObjCAtKeyword(tok::objc_end) ||
- FormatTok->isOneOf(tok::numeric_constant, tok::l_square,
- tok::l_brace))) ||
-(FormatTok->Tok.isAnyIdentifier() &&
- std::binary_search(std::begin(FoundationIdentifiers),
-std::end(FoundationIdentifiers),
-FormatTok->TokenText)) ||
-FormatTok->is(TT_ObjCStringLiteral) ||
-FormatTok->isOneOf(Keywords.kw_NS_ENUM, Keywords.kw_NS_OPTIONS,
- TT_ObjCBlockLBrace, TT_ObjCBlockLParen,
- TT_ObjCDecl, TT_ObjCForIn, TT_ObjCMethodExpr,
- TT_ObjCMethodSpecifier, TT_ObjCProperty)) {
- return true;
-}
- }
- return false;
-};
-for (auto Line : AnnotatedLines) {
- if (LineContainsObjCCode(*Line))
+for (const FormatToken *FormatTok = Line.First; FormatTok;
+ FormatTok = FormatTok->Next) {
+ if ((FormatTok->Previous && FormatTok->Previous->is(tok::at) &&
+ (FormatTok->isObjCAtKeyword(tok::objc_interface) ||
+FormatTok->isObjCAtKeyword(tok::objc_implementation) ||
+FormatTok->isObjCAtKeyword(tok::objc_protocol) ||
+FormatTok->isObjCAtKeyword(tok::objc_end) ||
+FormatTok->isOneOf(tok::numeric_constant, tok::l_square,
+ tok::l_brace))) ||
+ (FormatTok->Tok.isAnyIdentifier() &&
+ std::binary_search(std::begin(FoundationIdentifiers),
+ std::end(FoundationIdentifiers),
+ FormatTok->TokenText)) ||
+ FormatTok->is(TT_ObjCStringLiteral) ||
+ FormatTok->isOneOf(Keywords.kw_NS_ENUM, Keywords.kw_NS_OPTIONS,
+ TT_ObjCBlockLBrace, TT_ObjCBlockLParen,
+ TT_ObjCDecl, TT_ObjCForIn, TT_ObjCMethodExpr,
+ TT_ObjCMethodSpecifier, TT_ObjCProperty)) {
return true;
- for (auto ChildLine : Line->Children) {
-if (LineContainsObjCCode(*ChildLine))
- return true;
}
+ for (auto ChildLine : Line.Children)
+if (LineContainsObjCCode(*ChildLine, Keywords))
+ return true;
}
return false;
- }
+ };
bool IsObjC;
};
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12171,6 +12171,12 @@
guessLanguage("foo.h", "#define FOO ({ std::string s; })"));
EXPECT_EQ(FormatStyle::LK_ObjC,
guessLanguage("foo.h", "#define FOO
[PATCH] D44831: [clang-format] Refine ObjC guesser to handle child lines of child lines
benhamilton added inline comments.
Comment at: lib/Format/Format.cpp:1542
};
-for (auto Line : AnnotatedLines) {
- if (LineContainsObjCCode(*Line))
+llvm::DenseSet LinesToCheckSet;
+LinesToCheckSet.reserve(AnnotatedLines.size());
benhamilton wrote:
> djasper wrote:
> > Wouldn't it be much easier to call this function recursively for Children
> > instead of using the lambda as well as this additional set? Lines and their
> > children should form a tree, I think, so you should never see the same line
> > again during recursion.
> I'm less worried about cycles and more worried about a maliciously deeply
> nested set of children blowing out the stack and causing a crash.
>
> It seemed safer to use the heap here to avoid that scenario (I don't know of
> any other way to avoid it, since we don't know the stack size and we don't
> control it.)
djasper@ informed me that `clang-format` is already full of places which rely
on the stack for recursion on user input, so that window has already been
broken. :)
I'll happily go ahead and change this.
Repository:
rC Clang
https://reviews.llvm.org/D44831
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44831: [clang-format] Refine ObjC guesser to handle child lines of child lines
benhamilton added inline comments.
Comment at: lib/Format/Format.cpp:1542
};
-for (auto Line : AnnotatedLines) {
- if (LineContainsObjCCode(*Line))
+llvm::DenseSet LinesToCheckSet;
+LinesToCheckSet.reserve(AnnotatedLines.size());
djasper wrote:
> Wouldn't it be much easier to call this function recursively for Children
> instead of using the lambda as well as this additional set? Lines and their
> children should form a tree, I think, so you should never see the same line
> again during recursion.
I'm less worried about cycles and more worried about a maliciously deeply
nested set of children blowing out the stack and causing a crash.
It seemed safer to use the heap here to avoid that scenario (I don't know of
any other way to avoid it, since we don't know the stack size and we don't
control it.)
Repository:
rC Clang
https://reviews.llvm.org/D44831
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44831: [clang-format] Refine ObjC guesser to handle child lines of child lines
djasper added inline comments.
Comment at: lib/Format/Format.cpp:1542
};
-for (auto Line : AnnotatedLines) {
- if (LineContainsObjCCode(*Line))
+llvm::DenseSet LinesToCheckSet;
+LinesToCheckSet.reserve(AnnotatedLines.size());
Wouldn't it be much easier to call this function recursively for Children
instead of using the lambda as well as this additional set? Lines and their
children should form a tree, I think, so you should never see the same line
again during recursion.
Repository:
rC Clang
https://reviews.llvm.org/D44831
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44831: [clang-format] Refine ObjC guesser to handle child lines of child lines
benhamilton updated this revision to Diff 139601.
benhamilton added a comment.
Update from correct base revision
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,6 +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 ({ 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 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,6 +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 ({ 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 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
[PATCH] D44831: [clang-format] Refine ObjC guesser to handle child lines of child lines
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 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 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());
