[PATCH] D43904: [clang-format] Improve detection of ObjC for-in statements

Ben Hamilton via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 5 15:17:19 PST 2018


benhamilton added inline comments.


================
Comment at: lib/Format/TokenAnnotator.cpp:288
 
+        if (MightBeObjCForRangeLoop) {
+          FormatToken *ForInToken = Left;
----------------
djasper wrote:
> There can be only one ObjCForIn token in any for loop, right? If that's the case, can we just remember that in addition to (or instead of) MightBeObjCForRangeLoop? That way, we wouldn't have to re-iterate over all the tokens here, but could just set this directly.
Nice optimization, done.


================
Comment at: unittests/Format/FormatTest.cpp:12002
 
+TEST_F(FormatTest, GuessLanguageWithForIn) {
+  EXPECT_EQ(FormatStyle::LK_Cpp,
----------------
krasimir wrote:
> Please also add this instances as formatting tests.
Thanks, tests added.

The new formatting tests revealed a new regression I'd introduced: we were not inserting a space between keyword 'in' and the opening `[` of the ObjC message send.

This was because the ObjC message send parsing logic depended on the TT_ObjCForIn keyword being set before parsing the `[`, but by delaying setting the type until after parsing the for-loop was complete, we broke that logic.

Fixed regression and made sure tests passed.


Repository:
  rC Clang

https://reviews.llvm.org/D43904





More information about the cfe-commits mailing list