alexfh added inline comments.
Comment at:
clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.cpp:11
+extern int A, B;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: redundant 'A' declaration
+// CHECK-FIXES: {{^}}extern int A, B;{{$}}
Just
This revision was automatically updated to reflect the committed changes.
Closed by commit rL285689: [clang-tidy] Add check
readability-redundant-declaration (authored by danielmarjamaki).
Changed prior to commit:
https://reviews.llvm.org/D24656?vs=74478&id=76548#toc
Repository:
rL LLVM
htt
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
Sorry for the delay. I'm on vacation, returning on Monday.
The patch looks good when the comments are fixed.
Comment at: clang-tidy/readability/RedundantDeclarationCheck.cpp
danielmarjamaki added a comment.
Ping.
Repository:
rL LLVM
https://reviews.llvm.org/D24656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
danielmarjamaki set the repository for this revision to rL LLVM.
danielmarjamaki updated this revision to Diff 74478.
danielmarjamaki added a comment.
Herald added a subscriber: modocache.
changed cast(D)->getName() to cast(D)
Repository:
rL LLVM
https://reviews.llvm.org/D24656
Files:
clan
aaron.ballman added inline comments.
Comment at: clang-tidy/readability/RedundantDeclarationCheck.cpp:60
+auto Diag = diag(D->getLocation(), "redundant '%0' declaration")
+<< cast(D)->getName();
+if (!MultiVar && !DifferentHeaders)
danielmarjamaki
danielmarjamaki added inline comments.
Comment at: clang-tidy/readability/RedundantDeclarationCheck.cpp:60
+auto Diag = diag(D->getLocation(), "redundant '%0' declaration")
+<< cast(D)->getName();
+if (!MultiVar && !DifferentHeaders)
alexfh wrote:
alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.
Comment at: clang-tidy/readability/RedundantDeclarationCheck.cpp:39
+ bool MultiVar = false;
+ if (const auto *VD = dyn_cast(D)) {
+if (VD && VD->getPrevi
danielmarjamaki marked 2 inline comments as done.
danielmarjamaki added a comment.
Ping.
I am not very happy about how I detect multivariable declarations. But I really
don't see any such info in the VarDecl. For instance, the AST dump does not
show this info.
https://reviews.llvm.org/D24656
danielmarjamaki marked 3 inline comments as done.
Comment at: clang-tidy/readability/RedundantDeclarationCheck.cpp:39
@@ +38,3 @@
+ bool MultiVar = false;
+ if (const auto *VD = dyn_cast(D)) {
+if (VD && VD->getPreviousDecl()->getStorageClass() == SC_Extern &&
--
danielmarjamaki updated this revision to Diff 72802.
danielmarjamaki added a comment.
Fix review comments
https://reviews.llvm.org/D24656
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/readability/RedundantDeclarationCheck.cpp
cl
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
If this kind of an error is not too frequent, it might make sense as a clang
diagnostic, indeed. Having it in clang-tidy until then doesn't hurt, though.
Comment at
danielmarjamaki marked an inline comment as done.
danielmarjamaki added a comment.
https://reviews.llvm.org/D24656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
danielmarjamaki removed rL LLVM as the repository for this revision.
danielmarjamaki updated this revision to Diff 71775.
danielmarjamaki added a comment.
run clang-format on test. add release notes.
https://reviews.llvm.org/D24656
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/rea
danielmarjamaki added a comment.
> Will be good idea to detect redundant function prototypes.
Yes.
Should that have the same ID though? Is it better to have one
readability-redundant-declaration or two separate
readability-redundant-variable-declaration and
readability-redundant-function-dec
danielmarjamaki added a comment.
> However, I think this check should be part of Clang diagnostics. GCC has
> -Wredundant-decls for a long time.
I am not against that.
What is the criteria? When should it be in the compiler and when should it be
in clang-tidy?
Personally I think it's natural
Eugene.Zelenko added inline comments.
Comment at: test/clang-tidy/readability-redundant-declaration.cpp:9
@@ +8,3 @@
+extern int A;
+extern int A,B;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: redundant variable A declaration
Please run Clang-format over test.
Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko added a comment.
Please mention this check in docs/ReleaseNotes.rst (in alphabetical order).
Will be good idea to detect redundant function prototypes.
However, I think this check should be part of Clang diagnostics. GCC has
-Wre
danielmarjamaki marked 2 inline comments as done.
danielmarjamaki added a comment.
https://reviews.llvm.org/D24656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
danielmarjamaki updated this revision to Diff 71614.
danielmarjamaki added a comment.
minor fixes
https://reviews.llvm.org/D24656
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/readability/RedundantDeclarationCheck.cpp
clang-tidy
danielmarjamaki added a comment.
For information, I am testing this on debian packages right now. I will see the
results next week.
Comment at: clang-tidy/readability/RedundantDeclarationCheck.cpp:22
@@ +21,3 @@
+void RedundantDeclarationCheck::registerMatchers(MatchFinder *Fin
danielmarjamaki created this revision.
danielmarjamaki added a reviewer: alexfh.
danielmarjamaki added a subscriber: cfe-commits.
Herald added subscribers: mgorny, beanz.
This is a new check that warns about redundant variable declarations.
https://reviews.llvm.org/D24656
Files:
clang-tidy/rea
22 matches
Mail list logo
