JonasToth added inline comments.
================ Comment at: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp:84 "cppcoreguidelines-c-copy-assignment-signature"); + CheckFactories.registerCheck<modernize::AvoidCArraysCheck>( + "cppcoreguidelines-avoid-c-arrays"); ---------------- please conserve the alphabetical order here ================ Comment at: clang-tidy/hicpp/HICPPTidyModule.cpp:54 "hicpp-avoid-goto"); + CheckFactories.registerCheck<modernize::AvoidCArraysCheck>( + "hicpp-avoid-c-arrays"); ---------------- i would this one should be before `avoid-goto`. ================ Comment at: clang-tidy/modernize/AvoidCArraysCheck.cpp:44 + unless(anyOf(hasParent(varDecl(isExternC())), + hasAncestor(functionDecl(isExternC()))))) + .bind("typeloc"), ---------------- What about struct-decls that are externC? ================ Comment at: docs/clang-tidy/checks/modernize-avoid-c-arrays.rst:15 +observe *all* the uses of said declaration in order to decide whether it is +safe to transform or not, and that is impossible in case of headers. + ---------------- Not sure if we want the rational of not providing fixits here in this detail. Maybe it's better to formulate it as `fix-it are potentially dangerous in header files and are therefor not emitted right now`? Some places would be save to transform and we might want to provide these in the future. ================ Comment at: test/clang-tidy/modernize-avoid-c-arrays.cpp:48 +// Some header +extern "C" { + ---------------- Please add a test for extern c structs, maybe even in the form `extern "C" struct Foo { ... };`. Additionally to that, maybe the opposite case should be tested too: `extern "C++"` in C-Code. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53771 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits