whisperity added a comment.
LGTM. Some typos inline. Also I think this is a significant enough bug-fix that
it warrants an entry in the Release Notes under `Changes to existing checks`.
Also, the tests are failing, but it fails on the formatting of the test file
(???), and not the actual test itself. (Usually such issues would be sent back
to Phab to appear an a machine-made inline comment, I am not sure what happened
to that logic.)
changed files:
clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-extern-c.cpp
================
Comment at:
clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp:79-82
+ auto Begin = IncludesToBeProcessed.begin();
+ auto End = IncludesToBeProcessed.end();
+ auto Low = std::lower_bound(Begin, End, ExternCBlockBegin);
+ auto Up = std::upper_bound(Low, End, ExternCBlockEnd);
----------------
(`llvm/ADT/STLExtras.h` define a range-based `lower_bound` and `upper_bound`)
================
Comment at: clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp:98
+ // to act on a "TranslationUnit" to acquire the AST where we can walk each
+ // decl and look for `extern "C"` blocks where we wil lsuppress the report we
+ // collected during the preprocessing phase.
----------------
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125209/new/
https://reviews.llvm.org/D125209
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
