[PATCH] D125770: [clang-tidy] modernize-deprecated-headers should ignore system headers

Richard via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed May 18 08:21:41 PDT 2022


LegalizeAdulthood added inline comments.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-extern-c.cpp:37
 
-#include <mysystemlib.h> // FIXME: We should have no warning into system headers.
-// CHECK-MESSAGES-WARN-INTO-HEADERS: mysystemlib.h:1:10: warning: inclusion of deprecated C++ header 'assert.h'; consider using 'cassert' instead [modernize-deprecated-headers]
+#include <mysystemlib.h> // no-warning: Don't warn into system headers.
 
----------------
steakhal wrote:
> LegalizeAdulthood wrote:
> > Where is this file?  I can't find it in my tree.
> > 
> > ```
> > D:\legalize\llvm\llvm-project\clang-tools-extra
> > > dir/s/b mysystemlib.h
> > File Not Found
> > ```
> > 
> > Does this patch depend on some other unsubmitted patch?
> D125209 should have included this file at `clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-deprecated-headers/mysystemlib.h`.
OK, so it depends on D125209 .... why not just include these small changes in that?

Supposedly there is some way in phabricator to make one review dependent on another, but instead of being an expert in phabricator this change seems small enough to just fold into the other review, but perhaps opinions differ on that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125770/new/

https://reviews.llvm.org/D125770



More information about the cfe-commits mailing list