[PATCH] D19259: Initial version of misc-unused-using-decl check

Alexander Kornienko via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 19 06:02:03 PDT 2016

alexfh added a comment.

Awesome! Thank you for tackling this! A few comments.

Comment at: clang-tidy/misc/CMakeLists.txt:38
@@ -37,2 +37,3 @@
+  UnusedUsingDeclsCheck.cpp
Please fix file sorting around the new file.

Comment at: clang-tidy/misc/MiscTidyModule.cpp:123
@@ -121,1 +122,3 @@
+    CheckFactories.registerCheck<UnusedUsingDeclsCheck>(
+        "misc-unused-using-decls");
I think, all "misc-unused-" checks except for the "misc-unused-raii" (which finds actual bugs) mostly relate to readability. I suggest placing the new check to readability/ and we'll then move the other two "misc-unused-" checks there as well.

Comment at: clang-tidy/misc/UnusedUsingDeclsCheck.cpp:23
@@ +22,3 @@
+  Finder->addMatcher(usingDecl(isExpansionInMainFile()).bind("using"), this);
+  Finder->addMatcher(recordType(hasDeclaration(namedDecl().bind("used"))),
+                     this);
Why is it necessarily a `recordType`? How about typedefs and enums?

Comment at: clang-tidy/misc/UnusedUsingDeclsCheck.h:32
@@ +31,3 @@
+  llvm::DenseMap<const NamedDecl*, const UsingDecl*> FoundDecls;
+  llvm::DenseMap<const NamedDecl*, CharSourceRange> FoundRanges;
I'm not sure how the llvm/ADT/DenseMap.h header is pulled here, but DenseMap is not even used in ClangTidy.h, so this transitive inclusion is rather brittle. Please include llvm/ADT/DenseMap.h directly.

Comment at: docs/clang-tidy/checks/misc-unused-using-decls.rst:6
@@ +5,2 @@
+Finds unused using declarations.
Add an example, please.


More information about the cfe-commits mailing list