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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
