[PATCH] D125209: [clang-tidy] modernize-deprecated-headers check should respect extern "C" blocks

[Balázs Benics via Phabricator via cfe-commits]

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