curdeius added inline comments.

================
Comment at: clang/lib/Format/Format.cpp:2685
 const char CppIncludeRegexPattern[] =
-    R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";
+    R"(^[\t\ ]*[@#]?[\t\ ]*(import|include)[^"<]*[\t\n\ 
\\]*("[^"]+"|<[^>]+>|[^"<>;]+;))";
 
----------------
curdeius wrote:
> I'd rather see handling separately the `#import`/`#include` case on one hand 
> and `@import` on the other.
> IIUC, `@import` is followed by `ns1.ns2.something` without quotes nor `<>`. 
> And it ends with a `;`.
> Trying to put everything together is IMO error-prone and not very readable.
> 
> You could just do (pseudo-code):
> ```
> CppIncludeRegexPattern = (current_regex) | "@\s*import\s+[^;]+;"
> ```
> (`\s` is probably not supported, but we could use character class `[:space:]` 
> for clarity, cf. llvm\lib\Support\regcomp.c)
> 
> Not sure whether whitespace is allowed after `@`.
> Could you please post a link to the specification of `@import` syntax?
> 
> Not sure fixing `<"` / `">` issue is worth it, but in all cases I'd prefer 
> seeing it in a different patch.
`[:space:]` would match newlines and we don't want this in all cases, but 
`[:blank:]` should be a good replacement for `[\t ]`.

On a different note, why there's a new line `\n` and backslash `\\` in `[\t\n\ 
\\]`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121370

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to