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

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.


================
Comment at: clang/lib/Format/Format.cpp:2759
+          IncludeName =
+              StringRef(Twine("<", IncludeName).concat(Twine(">")).str());
+        }
----------------
How does that not go out of scope and I have a dangling reference?


================
Comment at: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp:177
 const char IncludeRegexPattern[] =
-    R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";
+    R"(^[\t\ ]*[@#]?[\t\ ]*(import|include)[^"<]*[\t\n\ \\]*("[^"]+"|<[^>]+>|[^"<>;]+;))";
 
----------------
I don't see why this is needed. It should match with the previous stuff.


================
Comment at: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp:177
 const char IncludeRegexPattern[] =
-    R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";
+    R"(^[\t\ ]*[@#]?[\t\ ]*(import|include)[^"<]*[\t\n\ \\]*("[^"]+"|<[^>]+>|[^"<>;]+;))";
 
----------------
HazardyKnusperkeks wrote:
> I don't see why this is needed. It should match with the previous stuff.
Do we want that optional?


================
Comment at: clang/unittests/Format/SortIncludesTest.cpp:471
+                 "@import Foundation;\n"
+                 "#import \"a.h\"\n"));
+}
----------------
I've no idea about obj-c, is it feasible to mix `#` and `@`? If so please add a test where the `@` is between `#`s.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121370