[PATCH] D66505: Make add_new_check.py's insertion of registerCheck<> match the sort order

[Daniel Sanders via Phabricator via llvm-commits]

dsanders marked 4 inline comments as done.
dsanders added a comment.

In D66505#1652420 <https://reviews.llvm.org/D66505#1652420>, @alexfh wrote:

> Mostly LG, if you've verified that this works. A couple of comments below.


I verified it using

  ./add_new_check.py llvm prefer-register-over-unsigned
  ./add_new_check.py llvm zzz
  ./add_new_check.py llvm nb
  ./add_new_check.py llvm iz
  ./add_new_check.py llvm a

which is enough to cover every insertion point in the LLVM module



================
Comment at: clang-tools-extra/clang-tidy/add_new_check.py:192
+          else:
+            match = re.search('registerCheck<(.*)> *\( *(?:"([^"]*)")?', line)
+            last_line = None
----------------
alexfh wrote:
> Are there actually any registerCheck calls with spaces between the `>` and the `(`? Should we clang-format these instead of supporting them in the script?
> Are there actually any registerCheck calls with spaces between the > and the (?

No, it was just trivial to be robust against it in case it does somehow happen. Likewise for the whitespace after the '(' but before the '"' or newline.

> Should we clang-format these instead of supporting them in the script?

I think the script should stick to inserting the new code rather than correcting existing mistakes that somehow crept in. I do agree that people should generally clang-format their patches before commit though which will prevent it creeping in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66505