LegalizeAdulthood marked 9 inline comments as done.
LegalizeAdulthood added inline comments.
================
Comment at: test/clang-tidy/check_clang_tidy.py:112
+ process_output = e.output.decode()
+ print('%s failed:\n%s' % (' '.join(args), process_output))
+ if raise_error:
----------------
JonasToth wrote:
> Its better to use `.format()` instead of `%` syntax
That can be done in a later commit; it is not the point of this review.
================
Comment at: test/clang-tidy/check_clang_tidy.py:141
+ '-check-prefixes=' + ','.join(check_notes_prefixes),
+ '-implicit-check-not={{note|warning|error}}:'])
+ return
----------------
serge-sans-paille wrote:
> These three `check_*` function all use the same `'FileCheck', '-....']`
> pattern. Maybe it's worth syndicating that to a
> try_run_filecheck(input_file0, input_file1, *extra_args)` function.
That can be done in a later commit; it is not the point of this review.
================
Comment at: test/clang-tidy/check_clang_tidy.py:178
+ check_fixes_prefixes, check_messages_prefixes, check_notes_prefixes = \
+ get_prefixes(args.check_suffix, input_text)
+
----------------
serge-sans-paille wrote:
> This is the verbose call site I was pointing to above.
Addressed by Extract Class instead of Extract Function
================
Comment at: test/clang-tidy/check_clang_tidy.py:206
- if has_check_fixes:
- try:
- subprocess.check_output(
- ['FileCheck', '-input-file=' + temp_file_name, input_file_name,
- '-check-prefixes=' + ','.join(check_fixes_prefixes),
- '-strict-whitespace'],
- stderr=subprocess.STDOUT)
- except subprocess.CalledProcessError as e:
- print('FileCheck failed:\n' + e.output.decode())
- raise
-
- if has_check_messages:
- messages_file = temp_file_name + '.msg'
- write_file(messages_file, clang_tidy_output)
- try:
- subprocess.check_output(
- ['FileCheck', '-input-file=' + messages_file, input_file_name,
- '-check-prefixes=' + ','.join(check_messages_prefixes),
- '-implicit-check-not={{warning|error}}:'],
- stderr=subprocess.STDOUT)
- except subprocess.CalledProcessError as e:
- print('FileCheck failed:\n' + e.output.decode())
- raise
-
- if has_check_notes:
- notes_file = temp_file_name + '.notes'
- filtered_output = [line for line in clang_tidy_output.splitlines()
- if not "note: FIX-IT applied" in line]
- write_file(notes_file, '\n'.join(filtered_output))
- try:
- subprocess.check_output(
- ['FileCheck', '-input-file=' + notes_file, input_file_name,
- '-check-prefixes=' + ','.join(check_notes_prefixes),
- '-implicit-check-not={{note|warning|error}}:'],
- stderr=subprocess.STDOUT)
- except subprocess.CalledProcessError as e:
- print('FileCheck failed:\n' + e.output.decode())
- raise
+ check_fixes(check_fixes_prefixes, has_check_fixes, input_file_name,
temp_file_name)
+ check_messages(check_messages_prefixes, has_check_messages,
clang_tidy_output, input_file_name, temp_file_name)
----------------
JonasToth wrote:
> If would prefer keeping the `if check_notes` outside of the function call and
> remove that one argument. Same for the other `check_...` functions
It's the wrong level of abstraction for the `if` check to be inside `run()`
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56343/new/
https://reviews.llvm.org/D56343
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
