[PATCH] D45185: [clang-format] Support lightweight Objective-C generics

[Daniel Jasper via Phabricator via cfe-commits]

djasper added inline comments.


================
Comment at: lib/Format/UnwrappedLineParser.cpp:2135
+    nextToken();
+    if (FormatTok->Tok.is(tok::less))
+      NumOpenAngles++;
----------------
The UnwrappedLineParser is very much about error recovery. Implemented like this, it will consume the rest of the file if someone forgets to add the closing ">", which can very easily happen when formatting in an editor while coding.

Are there things (e.g. semicolons and braces) that clearly point to this having happened so that clang-format can recover?


================
Comment at: lib/Format/UnwrappedLineParser.cpp:2136
+    if (FormatTok->Tok.is(tok::less))
+      NumOpenAngles++;
+    else if (FormatTok->Tok.is(tok::greater))
----------------
I think we generally prefer:

  ++NumOpenAngles;


================
Comment at: lib/Format/UnwrappedLineParser.cpp:2138
+    else if (FormatTok->Tok.is(tok::greater))
+      NumOpenAngles--;
+  } while (!eof() && NumOpenAngles != 0);
----------------
I am slightly worried that this loop might be getting more complicated and it will be harder to determine that NumOpenAngles cannot be 0 here. Might be worth adding an assert. But might be overkill.


================
Comment at: lib/Format/UnwrappedLineParser.cpp:2181
+  if (FormatTok->Tok.is(tok::less))
+    parseObjCLightweightGenericList();
+
----------------
Are there more places where we might want to parse a lightweight generic list? If this is going to remain they only instance, I think I'd prefer a local lambda or just inlining the code. But up to you.


================
Comment at: lib/Format/UnwrappedLineParser.cpp:2182
+    parseObjCLightweightGenericList();
+
   if (FormatTok->Tok.is(tok::colon)) {
----------------
nitpick: As the comment refers to this following block as well, I'd remove the empty line.


Repository:
  rC Clang

https://reviews.llvm.org/D45185