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

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

steakhal created this revision.
steakhal added reviewers: aaron.ballman, whisperity, LegalizeAdulthood, alexfh.
Herald added subscribers: carlosgalvezp, martong, mgrang, rnkovacs, xazax.hun.
Herald added a project: All.
steakhal requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

The check should not report includes wrapped by `extern "C" { ... }` blocks,
such as:

  #ifdef __cplusplus
  extern "C" {
  #endif
  
  #include "assert.h" // <-- actual include
  
  #ifdef __cplusplus
  }
  #endif

This pattern comes up sometimes in header files designed to be consumed
by both C and C++ source files.
The check now reports false reports when the header file is consumed by
a C++ translation unit.

In this change, I'm not emitting the reports immediately from the
`PPCallback`, rather aggregating them for further processing.
After all preprocessing is done, the matcher will be called on the
`TranslationUnitDecl`, ensuring that the check callback is called only
once.

Within that callback, I'm recursively visiting each decls, looking for
`LinkageSpecDecls` which represent the `extern "C"` specifier.
After this, I'm dropping all the reports coming from inside of it.
After the visitation is done, I'm emitting the reports I'm left with.

For performance reasons, I'm sorting the `IncludeMarkers` by their
corresponding locations.
This makes the scan `O(log(N)` when looking up the `IncludeMarkers`
affected by the given `extern "C"` block. For this, I'm using
`lower_bound()` and `upper_bound()`.

---

I've run this on some real code, and I've not seen a measurable impact on performance.
Out of these projects `llvm-project,contour,codechecker,qtbase,protobuf,bitcoin,xerces,libwebm,tinyxml2,postgres,ffmpeg,sqlite,openssl,vim,twin,curl,tmux,memcached`, it suppressed 5 reports:

- `bitcoin/src/leveldb/include/leveldb/c.h` L47, L48, L49  (`v0.20.1`)
- `bitcoin/src/secp256k1/include/secp256k1.h` L8  (`v0.20.1`)
- `qtbase/src/gui/opengl/qopenglext.h` L1427  (`v6.2.0`)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125209

Files:
  clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
  clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-deprecated-headers/mylib.h
  clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-extern-c.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D125209.427988.patch
Type: text/x-patch
Size: 10715 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220509/8de3954a/attachment-0001.bin>