[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.


================
Comment at: clang/lib/Format/Format.cpp:2692-2695
+  for (int i = Matches.size() - 1; i > 0; i--) {
+    if (!Matches[i].empty())
+      return Matches[i];
+  }
----------------
I think you missed `Matches[0]`.


================
Comment at: clang/lib/Format/Format.cpp:3056
 
+inline StringRef trimInclude(StringRef IncludeName) {
+  return IncludeName.trim("\"<>;");
----------------
We should make it `static` if it's only used in this file.


================
Comment at: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp:176-188
 const char IncludeRegexPattern[] =
-    R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";
+    R"(^[\t\ ]*[@#][\t\ ]*(import|include)([^"]*("[^"]+")|[^<]*(<[^>]+>)|[\t\ ]*([^;]+;)))";
+
+// Returns the last match group in the above regex (IncludeRegexPattern) that
+// is not empty.
+StringRef getIncludeNameFromMatches(const SmallVectorImpl<StringRef> &Matches) {
+  for (int i = Matches.size() - 1; i > 0; i--) {
----------------
If these are the same as in `Format.cpp` above, we should move the definitions to `HeaderIncludes.h`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121370/new/

https://reviews.llvm.org/D121370