steakhal marked 3 inline comments as done. steakhal added a comment. In D125209#3508573 <https://reviews.llvm.org/D125209#3508573>, @whisperity wrote:
> 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 Thanks. In D125209#3508576 <https://reviews.llvm.org/D125209#3508576>, @whisperity wrote: > (Also: this is a fix to an issue of your own finding and not something that > was reported on Bugzilla? Just to make sure we cross-reference and close > tickets.) It was an internal ticket. ================ 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); ---------------- whisperity wrote: > (`llvm/ADT/STLExtras.h` define a range-based `lower_bound` and `upper_bound`) Yup, but I'm using a different range. The first call could be replaced by the `llvm` ranged version, but the second starts with `Low` instead of `Begin`. At that point, I decided to keep the symmetry and use the `std` versions in both cases. Well, I could `llvm::make_range()` and use the `llvm::upper_bound` but yea, no. 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
