This revision was automatically updated to reflect the committed changes.
Closed by commit rL346835: [clang-tidy] Avoid C arrays check (authored by
lebedevri, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D53771?vs=173514=173996#toc
JonasToth added a comment.
In https://reviews.llvm.org/D53771#1294244, @lebedev.ri wrote:
> *Maybe* we should be actually diagnosing the each actual declaration, not
> just the `typeloc`.
> Else if you use one `typedef`, and `// NOLINT` it, you will silence it for
> all the decls that use it.
lebedev.ri added a comment.
*Maybe* we should be actually diagnosing the each actual declaration, not just
the `typeloc`.
Else if you use one `typedef`, and `// NOLINT` it, you will silence it for all
the decls that use it.
In https://reviews.llvm.org/D53771#1294241, @JonasToth wrote:
> LGTM.
JonasToth accepted this revision.
JonasToth added a comment.
This revision is now accepted and ready to land.
LGTM.
Please give other reviewers a chance to take a look at it too.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53771
lebedev.ri added inline comments.
Comment at: test/clang-tidy/modernize-avoid-c-arrays.cpp:32
+
+using k = int[4];
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not declare C-style arrays,
use std::array<> instead
JonasToth wrote:
> What happens for a
JonasToth added a comment.
from my side mostly ok. I think the typedef points should be clarified, but I
dont see more issues.
Comment at: test/clang-tidy/modernize-avoid-c-arrays.cpp:32
+
+using k = int[4];
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not declare C-style
lebedev.ri added inline comments.
Comment at: clang-tidy/modernize/AvoidCArraysCheck.cpp:64
+ diag(ArrayType->getBeginLoc(),
+ isa(ArrayType->getTypePtr()) ? UseVector : UseArray);
+}
JonasToth wrote:
> Why `isa<>` and not `isVariableArrayType()` (does it
lebedev.ri updated this revision to Diff 173514.
lebedev.ri marked 2 inline comments as done.
lebedev.ri added a comment.
Use `isVariableArrayType()`
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53771
Files:
clang-tidy/cppcoreguidelines/CMakeLists.txt
JonasToth added inline comments.
Comment at: clang-tidy/modernize/AvoidCArraysCheck.cpp:26
+ const clang::Type *TypeNode = Node.getTypePtr();
+ return (TypeNode != nullptr &&
+ InnerMatcher.matches(*TypeNode, Finder, Builder));
Nit: these parens are
lebedev.ri added a comment.
Comments? :)
https://reviews.llvm.org/D53771
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
JonasToth added inline comments.
Comment at: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp:84
"cppcoreguidelines-c-copy-assignment-signature");
+CheckFactories.registerCheck(
+"cppcoreguidelines-avoid-c-arrays");
lebedev.ri
lebedev.ri updated this revision to Diff 172520.
lebedev.ri marked 6 inline comments as done.
lebedev.ri added a comment.
Better diag for VLA arrays.
https://reviews.llvm.org/D53771
Files:
clang-tidy/cppcoreguidelines/CMakeLists.txt
lebedev.ri added inline comments.
Comment at: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp:84
"cppcoreguidelines-c-copy-assignment-signature");
+CheckFactories.registerCheck(
+"cppcoreguidelines-avoid-c-arrays");
lebedev.ri
lebedev.ri marked 4 inline comments as done.
lebedev.ri added a comment.
(only comments, patch to follow)
Comment at: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp:84
"cppcoreguidelines-c-copy-assignment-signature");
+CheckFactories.registerCheck(
+
JonasToth added inline comments.
Comment at: clang-tidy/modernize/AvoidCArraysCheck.cpp:44
+ unless(anyOf(hasParent(varDecl(isExternC())),
+ hasAncestor(functionDecl(isExternC())
+ .bind("typeloc"),
JonasToth
JonasToth added inline comments.
Comment at: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp:84
"cppcoreguidelines-c-copy-assignment-signature");
+CheckFactories.registerCheck(
+"cppcoreguidelines-avoid-c-arrays");
lebedev.ri
lebedev.ri updated this revision to Diff 171747.
lebedev.ri marked 4 inline comments as done.
lebedev.ri added a comment.
Address review notes.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53771
Files:
clang-tidy/cppcoreguidelines/CMakeLists.txt
lebedev.ri added inline comments.
Comment at: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp:84
"cppcoreguidelines-c-copy-assignment-signature");
+CheckFactories.registerCheck(
+"cppcoreguidelines-avoid-c-arrays");
JonasToth
JonasToth added inline comments.
Comment at: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp:84
"cppcoreguidelines-c-copy-assignment-signature");
+CheckFactories.registerCheck(
+"cppcoreguidelines-avoid-c-arrays");
please
lebedev.ri added inline comments.
Comment at: docs/clang-tidy/checks/cppcoreguidelines-avoid-c-arrays.rst:6
+cppcoreguidelines-avoid-c-arrays
+=
+
Eugene.Zelenko wrote:
> Please make same length as header.
lebedev.ri updated this revision to Diff 171425.
lebedev.ri marked 3 inline comments as done.
lebedev.ri added a comment.
Docs fixes.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53771
Files:
clang-tidy/cppcoreguidelines/CMakeLists.txt
Eugene.Zelenko added inline comments.
Comment at: docs/clang-tidy/checks/cppcoreguidelines-avoid-c-arrays.rst:6
+cppcoreguidelines-avoid-c-arrays
+=
+
Please make same length as header.
lebedev.ri updated this revision to Diff 171423.
lebedev.ri added a comment.
Herald added subscribers: kbarton, nemanjai.
In https://reviews.llvm.org/D53771#1278177, @Eugene.Zelenko wrote:
> Will be good idea to add aliases at least for existing modules.
Added CPPCG, HICPP aliases.
Eugene.Zelenko added a comment.
Will be good idea to add aliases at least for existing modules.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53771
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
lebedev.ri updated this revision to Diff 171393.
lebedev.ri marked 9 inline comments as done.
lebedev.ri edited the summary of this revision.
lebedev.ri added a comment.
- Moved into `modernize`
- Detect based on `TypeLoc`, which means we now catch *everything*, including
`using`-declarations.
JonasToth added inline comments.
Comment at: test/clang-tidy/misc-avoid-c-arrays.cpp:14
+
+template
+class array {
lebedev.ri wrote:
> JonasToth wrote:
> > Please add a case with templates, where `T` itself is the array type, maybe
> > there are other fancy
lebedev.ri added inline comments.
Comment at: test/clang-tidy/misc-avoid-c-arrays.cpp:14
+
+template
+class array {
JonasToth wrote:
> Please add a case with templates, where `T` itself is the array type, maybe
> there are other fancy template tricks that
Eugene.Zelenko added a comment.
May be this check belongs to modernize?
Comment at: docs/clang-tidy/checks/misc-avoid-c-arrays.rst:6
+
+Find C-style array delarations and recommend to use ``std::array<>``.
+All types of C arrays are diagnosed.
Finds.
JonasToth added inline comments.
Comment at: docs/clang-tidy/checks/list.rst:13
abseil-redundant-strcat-calls
- abseil-string-find-startswith
abseil-str-cat-append
spurious change
Comment at: docs/clang-tidy/checks/list.rst:154
JonasToth added a comment.
`modernize` would be a fitting module, wouldn't it?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53771
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
lebedev.ri created this revision.
lebedev.ri added reviewers: aaron.ballman, JonasToth, alexfh, hokein, xazax.hun.
lebedev.ri added a project: clang-tools-extra.
Herald added subscribers: rnkovacs, mgorny.
lebedev.ri edited the summary of this revision.
PR39224
31 matches
Mail list logo
