Re: [PATCH] D20429: [clang-tidy] Handle using-decls with more than one shadow decl.
hokein added inline comments.
Comment at: clang-tidy/misc/UnusedUsingDeclsCheck.cpp:59
@@ -59,1 +58,3 @@
/*SkipTrailingWhitespaceAndNewLine=*/true));
+for (const auto It : Using->shadows()) {
+ const auto *TargetDecl = It->getTargetDecl()->getCanonicalDecl();
alexfh wrote:
> hokein wrote:
> > alexfh wrote:
> > > It's not iterator, so `It` is a confusing name. Something along the lines
> > > of `Shadow` or `UsingShadow` should be better.
> > Actually, the `Using->shadows()` returns an iterator range, but I'm fine
> > renaming it here.
> Sure, it returns a `llvm::iterator_range`, which is just a
> range adaptor for a pair of iterators. It's not a "collection of iterators",
> it's a "collection, defined by a pair of iterators". If you iterate over it
> using a range-based for loop, you get whatever is pointed by the iterators,
> not iterators.
I see it now. Thanks for the explanations :-).
Repository:
rL LLVM
http://reviews.llvm.org/D20429
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20429: [clang-tidy] Handle using-decls with more than one shadow decl.
alexfh added inline comments.
Comment at: clang-tidy/misc/UnusedUsingDeclsCheck.cpp:59
@@ -59,1 +58,3 @@
/*SkipTrailingWhitespaceAndNewLine=*/true));
+for (const auto It : Using->shadows()) {
+ const auto *TargetDecl = It->getTargetDecl()->getCanonicalDecl();
hokein wrote:
> alexfh wrote:
> > It's not iterator, so `It` is a confusing name. Something along the lines
> > of `Shadow` or `UsingShadow` should be better.
> Actually, the `Using->shadows()` returns an iterator range, but I'm fine
> renaming it here.
Sure, it returns a `llvm::iterator_range`, which is just a
range adaptor for a pair of iterators. It's not a "collection of iterators",
it's a "collection, defined by a pair of iterators". If you iterate over it
using a range-based for loop, you get whatever is pointed by the iterators, not
iterators.
Repository:
rL LLVM
http://reviews.llvm.org/D20429
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20429: [clang-tidy] Handle using-decls with more than one shadow decl.
This revision was automatically updated to reflect the committed changes.
hokein marked an inline comment as done.
Closed by commit rL270191: [clang-tidy] Handle using-decls with more than one
shadow decl. (authored by hokein).
Changed prior to commit:
http://reviews.llvm.org/D20429?vs=57904&id=57906#toc
Repository:
rL LLVM
http://reviews.llvm.org/D20429
Files:
clang-tools-extra/trunk/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/UnusedUsingDeclsCheck.h
clang-tools-extra/trunk/test/clang-tidy/misc-unused-using-decls.cpp
Index: clang-tools-extra/trunk/clang-tidy/misc/UnusedUsingDeclsCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/misc/UnusedUsingDeclsCheck.h
+++ clang-tools-extra/trunk/clang-tidy/misc/UnusedUsingDeclsCheck.h
@@ -11,7 +11,8 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNUSED_USING_DECLS_H
#include "../ClangTidy.h"
-#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallPtrSet.h"
+#include
namespace clang {
namespace tidy {
@@ -32,8 +33,16 @@
private:
void removeFromFoundDecls(const Decl *D);
- llvm::DenseMap FoundDecls;
- llvm::DenseMap FoundRanges;
+ struct UsingDeclContext {
+explicit UsingDeclContext(const UsingDecl *FoundUsingDecl)
+: FoundUsingDecl(FoundUsingDecl), IsUsed(false) {}
+llvm::SmallPtrSet UsingTargetDecls;
+const UsingDecl *FoundUsingDecl;
+CharSourceRange UsingDeclRange;
+bool IsUsed;
+ };
+
+ std::vector Contexts;
};
} // namespace misc
Index: clang-tools-extra/trunk/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
@@ -18,6 +18,25 @@
namespace tidy {
namespace misc {
+// A function that helps to tell whether a TargetDecl will be checked.
+// We only check a TargetDecl if :
+// * The corresponding UsingDecl is not defined in macros or in class
+// definitions.
+// * Only variable, function and class types are considered.
+static bool ShouldCheckDecl(const Decl *TargetDecl) {
+ // Ignores using-declarations defined in macros.
+ if (TargetDecl->getLocation().isMacroID())
+return false;
+
+ // Ignores using-declarations defined in class definition.
+ if (isa(TargetDecl->getDeclContext()))
+return false;
+
+ return isa(TargetDecl) || isa(TargetDecl) ||
+ isa(TargetDecl) || isa(TargetDecl) ||
+ isa(TargetDecl);
+}
+
void UnusedUsingDeclsCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(usingDecl(isExpansionInMainFile()).bind("using"), this);
auto DeclMatcher = hasDeclaration(namedDecl().bind("used"));
@@ -30,33 +49,20 @@
void UnusedUsingDeclsCheck::check(const MatchFinder::MatchResult &Result) {
if (const auto *Using = Result.Nodes.getNodeAs("using")) {
-// FIXME: Implement the correct behavior for using declarations with more
-// than one shadow.
-if (Using->shadow_size() != 1)
- return;
-const auto *TargetDecl =
-Using->shadow_begin()->getTargetDecl()->getCanonicalDecl();
-
-// Ignores using-declarations defined in macros.
-if (TargetDecl->getLocation().isMacroID())
- return;
-
-// Ignores using-declarations defined in class definition.
-if (isa(TargetDecl->getDeclContext()))
- return;
-
-if (!isa(TargetDecl) && !isa(TargetDecl) &&
-!isa(TargetDecl) && !isa(TargetDecl) &&
-!isa(TargetDecl))
- return;
-
-FoundDecls[TargetDecl] = Using;
-FoundRanges[TargetDecl] = CharSourceRange::getCharRange(
+UsingDeclContext Context(Using);
+Context.UsingDeclRange = CharSourceRange::getCharRange(
Using->getLocStart(),
Lexer::findLocationAfterToken(
Using->getLocEnd(), tok::semi, *Result.SourceManager,
Result.Context->getLangOpts(),
/*SkipTrailingWhitespaceAndNewLine=*/true));
+for (const auto *UsingShadow : Using->shadows()) {
+ const auto *TargetDecl = UsingShadow->getTargetDecl()->getCanonicalDecl();
+ if (ShouldCheckDecl(TargetDecl))
+Context.UsingTargetDecls.insert(TargetDecl);
+}
+if (!Context.UsingTargetDecls.empty())
+ Contexts.push_back(Context);
return;
}
@@ -93,20 +99,23 @@
}
void UnusedUsingDeclsCheck::removeFromFoundDecls(const Decl *D) {
- auto I = FoundDecls.find(D->getCanonicalDecl());
- if (I != FoundDecls.end())
-I->second = nullptr;
+ for (auto &Context : Contexts) {
+if (Context.UsingTargetDecls.count(D->getCanonicalDecl()) > 0) {
+ Context.IsUsed = true;
+ break;
+}
+ }
}
void UnusedUsingDeclsCheck::onEndOfTranslationUnit() {
- for (const auto &FoundDecl : FoundDecls) {
-if (FoundDecl.second == nullptr)
- continue;
-diag(FoundDecl.second->getLocation(), "using decl %0 is unused")
-<<
Re: [PATCH] D20429: [clang-tidy] Handle using-decls with more than one shadow decl.
hokein marked 2 inline comments as done.
Comment at: clang-tidy/misc/UnusedUsingDeclsCheck.cpp:59
@@ -59,1 +58,3 @@
/*SkipTrailingWhitespaceAndNewLine=*/true));
+for (const auto It : Using->shadows()) {
+ const auto *TargetDecl = It->getTargetDecl()->getCanonicalDecl();
alexfh wrote:
> It's not iterator, so `It` is a confusing name. Something along the lines of
> `Shadow` or `UsingShadow` should be better.
Actually, the `Using->shadows()` returns an iterator range, but I'm fine
renaming it here.
http://reviews.llvm.org/D20429
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20429: [clang-tidy] Handle using-decls with more than one shadow decl.
hokein updated this revision to Diff 57904.
hokein added a comment.
Address more comments.
http://reviews.llvm.org/D20429
Files:
clang-tidy/misc/UnusedUsingDeclsCheck.cpp
clang-tidy/misc/UnusedUsingDeclsCheck.h
test/clang-tidy/misc-unused-using-decls.cpp
Index: test/clang-tidy/misc-unused-using-decls.cpp
===
--- test/clang-tidy/misc-unused-using-decls.cpp
+++ test/clang-tidy/misc-unused-using-decls.cpp
@@ -31,6 +31,8 @@
template int UsedTemplateFunc() { return 1; }
template int UnusedTemplateFunc() { return 1; }
template int UsedInTemplateFunc() { return 1; }
+void OverloadFunc(int);
+void OverloadFunc(double);
class ostream {
public:
@@ -79,6 +81,10 @@
UsedInTemplateFunc();
}
+using n::OverloadFunc; // OverloadFunc
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'OverloadFunc' is unused
+// CHECK-FIXES: {{^}}// OverloadFunc
+
#define DEFINE_INT(name)\
namespace INT { \
static const int _##name = 1; \
Index: clang-tidy/misc/UnusedUsingDeclsCheck.h
===
--- clang-tidy/misc/UnusedUsingDeclsCheck.h
+++ clang-tidy/misc/UnusedUsingDeclsCheck.h
@@ -11,7 +11,8 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNUSED_USING_DECLS_H
#include "../ClangTidy.h"
-#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallPtrSet.h"
+#include
namespace clang {
namespace tidy {
@@ -32,8 +33,16 @@
private:
void removeFromFoundDecls(const Decl *D);
- llvm::DenseMap FoundDecls;
- llvm::DenseMap FoundRanges;
+ struct UsingDeclContext {
+explicit UsingDeclContext(const UsingDecl *FoundUsingDecl)
+: FoundUsingDecl(FoundUsingDecl), IsUsed(false) {}
+llvm::SmallPtrSet UsingTargetDecls;
+const UsingDecl *FoundUsingDecl;
+CharSourceRange UsingDeclRange;
+bool IsUsed;
+ };
+
+ std::vector Contexts;
};
} // namespace misc
Index: clang-tidy/misc/UnusedUsingDeclsCheck.cpp
===
--- clang-tidy/misc/UnusedUsingDeclsCheck.cpp
+++ clang-tidy/misc/UnusedUsingDeclsCheck.cpp
@@ -18,6 +18,25 @@
namespace tidy {
namespace misc {
+// A function that helps to tell whether a TargetDecl will be checked.
+// We only check a TargetDecl if :
+// * The corresponding UsingDecl is not defined in macros or in class
+// definitions.
+// * Only variable, function and class types are considered.
+static bool ShouldCheckDecl(const Decl *TargetDecl) {
+ // Ignores using-declarations defined in macros.
+ if (TargetDecl->getLocation().isMacroID())
+return false;
+
+ // Ignores using-declarations defined in class definition.
+ if (isa(TargetDecl->getDeclContext()))
+return false;
+
+ return isa(TargetDecl) || isa(TargetDecl) ||
+ isa(TargetDecl) || isa(TargetDecl) ||
+ isa(TargetDecl);
+}
+
void UnusedUsingDeclsCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(usingDecl(isExpansionInMainFile()).bind("using"), this);
auto DeclMatcher = hasDeclaration(namedDecl().bind("used"));
@@ -30,33 +49,20 @@
void UnusedUsingDeclsCheck::check(const MatchFinder::MatchResult &Result) {
if (const auto *Using = Result.Nodes.getNodeAs("using")) {
-// FIXME: Implement the correct behavior for using declarations with more
-// than one shadow.
-if (Using->shadow_size() != 1)
- return;
-const auto *TargetDecl =
-Using->shadow_begin()->getTargetDecl()->getCanonicalDecl();
-
-// Ignores using-declarations defined in macros.
-if (TargetDecl->getLocation().isMacroID())
- return;
-
-// Ignores using-declarations defined in class definition.
-if (isa(TargetDecl->getDeclContext()))
- return;
-
-if (!isa(TargetDecl) && !isa(TargetDecl) &&
-!isa(TargetDecl) && !isa(TargetDecl) &&
-!isa(TargetDecl))
- return;
-
-FoundDecls[TargetDecl] = Using;
-FoundRanges[TargetDecl] = CharSourceRange::getCharRange(
+UsingDeclContext Context(Using);
+Context.UsingDeclRange = CharSourceRange::getCharRange(
Using->getLocStart(),
Lexer::findLocationAfterToken(
Using->getLocEnd(), tok::semi, *Result.SourceManager,
Result.Context->getLangOpts(),
/*SkipTrailingWhitespaceAndNewLine=*/true));
+for (const auto *UsingShadow : Using->shadows()) {
+ const auto *TargetDecl = UsingShadow->getTargetDecl()->getCanonicalDecl();
+ if (ShouldCheckDecl(TargetDecl))
+Context.UsingTargetDecls.insert(TargetDecl);
+}
+if (!Context.UsingTargetDecls.empty())
+ Contexts.push_back(Context);
return;
}
@@ -93,20 +99,23 @@
}
void UnusedUsingDeclsCheck::removeFromFoundDecls(const Decl *D) {
- auto I = FoundDecls.find(D->getCanonicalDecl());
- if (I != FoundDecls.end())
-I->second = nullptr;
+ for (auto &Context : Contexts) {
+if (Context.UsingTargetDecls.count(D->getCa
Re: [PATCH] D20429: [clang-tidy] Handle using-decls with more than one shadow decl.
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
LG with a couple of nits.
Comment at: clang-tidy/misc/UnusedUsingDeclsCheck.cpp:22
@@ +21,3 @@
+// A function that helps to tell whether a TargetDecl will be checked.
+// We only check a TargetDecl if :
+// * The corresponding UsingDecl is not defined in macros or in class
`ShouldCheckDecl` might convey the idea better.
Comment at: clang-tidy/misc/UnusedUsingDeclsCheck.cpp:59
@@ -59,1 +58,3 @@
/*SkipTrailingWhitespaceAndNewLine=*/true));
+for (const auto It : Using->shadows()) {
+ const auto *TargetDecl = It->getTargetDecl()->getCanonicalDecl();
It's not iterator, so `It` is a confusing name. Something along the lines of
`Shadow` or `UsingShadow` should be better.
http://reviews.llvm.org/D20429
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20429: [clang-tidy] Handle using-decls with more than one shadow decl.
hokein marked an inline comment as done.
Comment at: clang-tidy/misc/UnusedUsingDeclsCheck.cpp:22
@@ +21,3 @@
+namespace {
+bool IsValidDecl(const Decl *TargetDecl) {
+ // Ignores using-declarations defined in macros.
alexfh wrote:
> This method assumes a rather non-trivial definition of "valid". Please add a
> comment what exactly this method does.
>
> Also, static is preferred to anonymous namespaces for functions in LLVM style
> (http://llvm.org/docs/CodingStandards.html#anonymous-namespaces):
> | ... make anonymous namespaces as small as possible, and only use them for
> class declarations. ...
I have renamed to `IsCheckable`, but it doesn't make more sense here. Do you
have better suggestion on the function name?
Comment at: test/clang-tidy/misc-unused-using-decls.cpp:86
@@ +85,3 @@
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'OverloadFunc' is
unused
+// CHECK-FIXES: {{^}}// OverloadFunc
+
Will do it in a follow-up.
http://reviews.llvm.org/D20429
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20429: [clang-tidy] Handle using-decls with more than one shadow decl.
hokein updated this revision to Diff 57790.
hokein added a comment.
Forgot a comments.
http://reviews.llvm.org/D20429
Files:
clang-tidy/misc/UnusedUsingDeclsCheck.cpp
clang-tidy/misc/UnusedUsingDeclsCheck.h
test/clang-tidy/misc-unused-using-decls.cpp
Index: test/clang-tidy/misc-unused-using-decls.cpp
===
--- test/clang-tidy/misc-unused-using-decls.cpp
+++ test/clang-tidy/misc-unused-using-decls.cpp
@@ -31,6 +31,8 @@
template int UsedTemplateFunc() { return 1; }
template int UnusedTemplateFunc() { return 1; }
template int UsedInTemplateFunc() { return 1; }
+void OverloadFunc(int);
+void OverloadFunc(double);
class ostream {
public:
@@ -79,6 +81,10 @@
UsedInTemplateFunc();
}
+using n::OverloadFunc; // OverloadFunc
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'OverloadFunc' is unused
+// CHECK-FIXES: {{^}}// OverloadFunc
+
#define DEFINE_INT(name)\
namespace INT { \
static const int _##name = 1; \
Index: clang-tidy/misc/UnusedUsingDeclsCheck.h
===
--- clang-tidy/misc/UnusedUsingDeclsCheck.h
+++ clang-tidy/misc/UnusedUsingDeclsCheck.h
@@ -11,7 +11,8 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNUSED_USING_DECLS_H
#include "../ClangTidy.h"
-#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallPtrSet.h"
+#include
namespace clang {
namespace tidy {
@@ -32,8 +33,16 @@
private:
void removeFromFoundDecls(const Decl *D);
- llvm::DenseMap FoundDecls;
- llvm::DenseMap FoundRanges;
+ struct UsingDeclContext {
+explicit UsingDeclContext(const UsingDecl *FoundUsingDecl)
+: FoundUsingDecl(FoundUsingDecl), IsUsed(false) {}
+llvm::SmallPtrSet UsingTargetDecls;
+const UsingDecl *FoundUsingDecl;
+CharSourceRange UsingDeclRange;
+bool IsUsed;
+ };
+
+ std::vector Contexts;
};
} // namespace misc
Index: clang-tidy/misc/UnusedUsingDeclsCheck.cpp
===
--- clang-tidy/misc/UnusedUsingDeclsCheck.cpp
+++ clang-tidy/misc/UnusedUsingDeclsCheck.cpp
@@ -18,6 +18,25 @@
namespace tidy {
namespace misc {
+// A function that helps to tell whether a TargetDecl will be checked.
+// We only check a TargetDecl if :
+// * The corresponding UsingDecl is not defined in macros or in class
+// definitions.
+// * Only variable, function and class types are considered.
+static bool IsCheckable(const Decl *TargetDecl) {
+ // Ignores using-declarations defined in macros.
+ if (TargetDecl->getLocation().isMacroID())
+return false;
+
+ // Ignores using-declarations defined in class definition.
+ if (isa(TargetDecl->getDeclContext()))
+return false;
+
+ return isa(TargetDecl) || isa(TargetDecl) ||
+ isa(TargetDecl) || isa(TargetDecl) ||
+ isa(TargetDecl);
+}
+
void UnusedUsingDeclsCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(usingDecl(isExpansionInMainFile()).bind("using"), this);
auto DeclMatcher = hasDeclaration(namedDecl().bind("used"));
@@ -30,33 +49,20 @@
void UnusedUsingDeclsCheck::check(const MatchFinder::MatchResult &Result) {
if (const auto *Using = Result.Nodes.getNodeAs("using")) {
-// FIXME: Implement the correct behavior for using declarations with more
-// than one shadow.
-if (Using->shadow_size() != 1)
- return;
-const auto *TargetDecl =
-Using->shadow_begin()->getTargetDecl()->getCanonicalDecl();
-
-// Ignores using-declarations defined in macros.
-if (TargetDecl->getLocation().isMacroID())
- return;
-
-// Ignores using-declarations defined in class definition.
-if (isa(TargetDecl->getDeclContext()))
- return;
-
-if (!isa(TargetDecl) && !isa(TargetDecl) &&
-!isa(TargetDecl) && !isa(TargetDecl) &&
-!isa(TargetDecl))
- return;
-
-FoundDecls[TargetDecl] = Using;
-FoundRanges[TargetDecl] = CharSourceRange::getCharRange(
+UsingDeclContext Context(Using);
+Context.UsingDeclRange = CharSourceRange::getCharRange(
Using->getLocStart(),
Lexer::findLocationAfterToken(
Using->getLocEnd(), tok::semi, *Result.SourceManager,
Result.Context->getLangOpts(),
/*SkipTrailingWhitespaceAndNewLine=*/true));
+for (const auto It : Using->shadows()) {
+ const auto *TargetDecl = It->getTargetDecl()->getCanonicalDecl();
+ if (IsCheckable(TargetDecl))
+Context.UsingTargetDecls.insert(TargetDecl);
+}
+if (!Context.UsingTargetDecls.empty())
+ Contexts.push_back(Context);
return;
}
@@ -93,20 +99,23 @@
}
void UnusedUsingDeclsCheck::removeFromFoundDecls(const Decl *D) {
- auto I = FoundDecls.find(D->getCanonicalDecl());
- if (I != FoundDecls.end())
-I->second = nullptr;
+ for (auto &Context : Contexts) {
+if (Context.UsingTargetDecls.count(D->getCanonicalDecl()) > 0) {
+ Co
Re: [PATCH] D20429: [clang-tidy] Handle using-decls with more than one shadow decl.
hokein updated this revision to Diff 57785.
hokein marked 4 inline comments as done.
hokein added a comment.
Address comments.
http://reviews.llvm.org/D20429
Files:
clang-tidy/misc/UnusedUsingDeclsCheck.cpp
clang-tidy/misc/UnusedUsingDeclsCheck.h
test/clang-tidy/misc-unused-using-decls.cpp
Index: test/clang-tidy/misc-unused-using-decls.cpp
===
--- test/clang-tidy/misc-unused-using-decls.cpp
+++ test/clang-tidy/misc-unused-using-decls.cpp
@@ -31,6 +31,8 @@
template int UsedTemplateFunc() { return 1; }
template int UnusedTemplateFunc() { return 1; }
template int UsedInTemplateFunc() { return 1; }
+void OverloadFunc(int);
+void OverloadFunc(double);
class ostream {
public:
@@ -79,6 +81,10 @@
UsedInTemplateFunc();
}
+using n::OverloadFunc; // OverloadFunc
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'OverloadFunc' is unused
+// CHECK-FIXES: {{^}}// OverloadFunc
+
#define DEFINE_INT(name)\
namespace INT { \
static const int _##name = 1; \
Index: clang-tidy/misc/UnusedUsingDeclsCheck.h
===
--- clang-tidy/misc/UnusedUsingDeclsCheck.h
+++ clang-tidy/misc/UnusedUsingDeclsCheck.h
@@ -11,7 +11,8 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNUSED_USING_DECLS_H
#include "../ClangTidy.h"
-#include "llvm/ADT/DenseMap.h"
+#include
+#include
namespace clang {
namespace tidy {
@@ -32,8 +33,16 @@
private:
void removeFromFoundDecls(const Decl *D);
- llvm::DenseMap FoundDecls;
- llvm::DenseMap FoundRanges;
+ struct UsingDeclContext {
+explicit UsingDeclContext(const UsingDecl *FoundUsingDecl)
+: FoundUsingDecl(FoundUsingDecl), IsUsed(false) {}
+std::set UsingTargetDecls;
+const UsingDecl *FoundUsingDecl;
+CharSourceRange UsingDeclRange;
+bool IsUsed;
+ };
+
+ std::vector Contexts;
};
} // namespace misc
Index: clang-tidy/misc/UnusedUsingDeclsCheck.cpp
===
--- clang-tidy/misc/UnusedUsingDeclsCheck.cpp
+++ clang-tidy/misc/UnusedUsingDeclsCheck.cpp
@@ -18,6 +18,25 @@
namespace tidy {
namespace misc {
+// A function that helps to tell whether a TargetDecl will be checked.
+// We only check a TargetDecl if :
+// * The corresponding UsingDecl is not defined in macros or in class
+// definitions.
+// * Only variable, function and class types are considered.
+static bool IsCheckable(const Decl *TargetDecl) {
+ // Ignores using-declarations defined in macros.
+ if (TargetDecl->getLocation().isMacroID())
+return false;
+
+ // Ignores using-declarations defined in class definition.
+ if (isa(TargetDecl->getDeclContext()))
+return false;
+
+ return isa(TargetDecl) || isa(TargetDecl) ||
+ isa(TargetDecl) || isa(TargetDecl) ||
+ isa(TargetDecl);
+}
+
void UnusedUsingDeclsCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(usingDecl(isExpansionInMainFile()).bind("using"), this);
auto DeclMatcher = hasDeclaration(namedDecl().bind("used"));
@@ -30,33 +49,20 @@
void UnusedUsingDeclsCheck::check(const MatchFinder::MatchResult &Result) {
if (const auto *Using = Result.Nodes.getNodeAs("using")) {
-// FIXME: Implement the correct behavior for using declarations with more
-// than one shadow.
-if (Using->shadow_size() != 1)
- return;
-const auto *TargetDecl =
-Using->shadow_begin()->getTargetDecl()->getCanonicalDecl();
-
-// Ignores using-declarations defined in macros.
-if (TargetDecl->getLocation().isMacroID())
- return;
-
-// Ignores using-declarations defined in class definition.
-if (isa(TargetDecl->getDeclContext()))
- return;
-
-if (!isa(TargetDecl) && !isa(TargetDecl) &&
-!isa(TargetDecl) && !isa(TargetDecl) &&
-!isa(TargetDecl))
- return;
-
-FoundDecls[TargetDecl] = Using;
-FoundRanges[TargetDecl] = CharSourceRange::getCharRange(
+UsingDeclContext Context(Using);
+Context.UsingDeclRange = CharSourceRange::getCharRange(
Using->getLocStart(),
Lexer::findLocationAfterToken(
Using->getLocEnd(), tok::semi, *Result.SourceManager,
Result.Context->getLangOpts(),
/*SkipTrailingWhitespaceAndNewLine=*/true));
+for (const auto It : Using->shadows()) {
+ const auto *TargetDecl = It->getTargetDecl()->getCanonicalDecl();
+ if (IsCheckable(TargetDecl))
+Context.UsingTargetDecls.insert(TargetDecl);
+}
+if (!Context.UsingTargetDecls.empty())
+ Contexts.push_back(Context);
return;
}
@@ -93,20 +99,23 @@
}
void UnusedUsingDeclsCheck::removeFromFoundDecls(const Decl *D) {
- auto I = FoundDecls.find(D->getCanonicalDecl());
- if (I != FoundDecls.end())
-I->second = nullptr;
+ for (auto &Context : Contexts) {
+if (Context.UsingTargetDecls.count(D->getCanonicalDecl()) > 0) {
+
Re: [PATCH] D20429: [clang-tidy] Handle using-decls with more than one shadow decl.
alexfh requested changes to this revision.
This revision now requires changes to proceed.
Comment at: clang-tidy/misc/UnusedUsingDeclsCheck.cpp:22
@@ +21,3 @@
+namespace {
+bool IsValidDecl(const Decl *TargetDecl) {
+ // Ignores using-declarations defined in macros.
This method assumes a rather non-trivial definition of "valid". Please add a
comment what exactly this method does.
Also, static is preferred to anonymous namespaces for functions in LLVM style
(http://llvm.org/docs/CodingStandards.html#anonymous-namespaces):
| ... make anonymous namespaces as small as possible, and only use them for
class declarations. ...
Comment at: clang-tidy/misc/UnusedUsingDeclsCheck.cpp:29
@@ +28,3 @@
+ if (isa(TargetDecl->getDeclContext())) {
+llvm::errs() << "context\n";
+return false;
Debug output?
Comment at: clang-tidy/misc/UnusedUsingDeclsCheck.cpp:102
@@ +101,3 @@
+ for (auto &Context : Contexts) {
+if (Context.UsingTargetDecls.find(D->getCanonicalDecl()) !=
+Context.UsingTargetDecls.end()) {
.count() for sets is as effective, but results in clearer code.
Comment at: clang-tidy/misc/UnusedUsingDeclsCheck.h:39
@@ +38,3 @@
+: FoundUsingDecl(FoundUsingDecl), IsUsed(false) {}
+std::set UsingTargetDecls;
+const UsingDecl *FoundUsingDecl;
`SmallPtrSet` should be more efficient here.
Comment at: test/clang-tidy/misc-unused-using-decls.cpp:86
@@ +85,3 @@
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'OverloadFunc' is
unused
+// CHECK-FIXES: {{^}}// OverloadFunc
+
Not for this patch, but it makes sense to remove trailing comments on deleted
lines.
http://reviews.llvm.org/D20429
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D20429: [clang-tidy] Handle using-decls with more than one shadow decl.
hokein created this revision.
hokein added a reviewer: alexfh.
hokein added subscribers: djasper, cfe-commits.
http://reviews.llvm.org/D20429
Files:
clang-tidy/misc/UnusedUsingDeclsCheck.cpp
clang-tidy/misc/UnusedUsingDeclsCheck.h
test/clang-tidy/misc-unused-using-decls.cpp
Index: test/clang-tidy/misc-unused-using-decls.cpp
===
--- test/clang-tidy/misc-unused-using-decls.cpp
+++ test/clang-tidy/misc-unused-using-decls.cpp
@@ -31,6 +31,8 @@
template int UsedTemplateFunc() { return 1; }
template int UnusedTemplateFunc() { return 1; }
template int UsedInTemplateFunc() { return 1; }
+void OverloadFunc(int);
+void OverloadFunc(double);
class ostream {
public:
@@ -79,6 +81,10 @@
UsedInTemplateFunc();
}
+using n::OverloadFunc; // OverloadFunc
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'OverloadFunc' is unused
+// CHECK-FIXES: {{^}}// OverloadFunc
+
#define DEFINE_INT(name)\
namespace INT { \
static const int _##name = 1; \
Index: clang-tidy/misc/UnusedUsingDeclsCheck.h
===
--- clang-tidy/misc/UnusedUsingDeclsCheck.h
+++ clang-tidy/misc/UnusedUsingDeclsCheck.h
@@ -11,7 +11,8 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNUSED_USING_DECLS_H
#include "../ClangTidy.h"
-#include "llvm/ADT/DenseMap.h"
+#include
+#include
namespace clang {
namespace tidy {
@@ -32,8 +33,16 @@
private:
void removeFromFoundDecls(const Decl *D);
- llvm::DenseMap FoundDecls;
- llvm::DenseMap FoundRanges;
+ struct UsingDeclContext {
+explicit UsingDeclContext(const UsingDecl *FoundUsingDecl)
+: FoundUsingDecl(FoundUsingDecl), IsUsed(false) {}
+std::set UsingTargetDecls;
+const UsingDecl *FoundUsingDecl;
+CharSourceRange UsingDeclRange;
+bool IsUsed;
+ };
+
+ std::vector Contexts;
};
} // namespace misc
Index: clang-tidy/misc/UnusedUsingDeclsCheck.cpp
===
--- clang-tidy/misc/UnusedUsingDeclsCheck.cpp
+++ clang-tidy/misc/UnusedUsingDeclsCheck.cpp
@@ -18,6 +18,24 @@
namespace tidy {
namespace misc {
+namespace {
+bool IsValidDecl(const Decl *TargetDecl) {
+ // Ignores using-declarations defined in macros.
+ if (TargetDecl->getLocation().isMacroID())
+return false;
+
+ // Ignores using-declarations defined in class definition.
+ if (isa(TargetDecl->getDeclContext())) {
+llvm::errs() << "context\n";
+return false;
+ }
+
+ return isa(TargetDecl) || isa(TargetDecl) ||
+ isa(TargetDecl) || isa(TargetDecl) ||
+ isa(TargetDecl);
+}
+} // namespace
+
void UnusedUsingDeclsCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(usingDecl(isExpansionInMainFile()).bind("using"), this);
auto DeclMatcher = hasDeclaration(namedDecl().bind("used"));
@@ -30,33 +48,20 @@
void UnusedUsingDeclsCheck::check(const MatchFinder::MatchResult &Result) {
if (const auto *Using = Result.Nodes.getNodeAs("using")) {
-// FIXME: Implement the correct behavior for using declarations with more
-// than one shadow.
-if (Using->shadow_size() != 1)
- return;
-const auto *TargetDecl =
-Using->shadow_begin()->getTargetDecl()->getCanonicalDecl();
-
-// Ignores using-declarations defined in macros.
-if (TargetDecl->getLocation().isMacroID())
- return;
-
-// Ignores using-declarations defined in class definition.
-if (isa(TargetDecl->getDeclContext()))
- return;
-
-if (!isa(TargetDecl) && !isa(TargetDecl) &&
-!isa(TargetDecl) && !isa(TargetDecl) &&
-!isa(TargetDecl))
- return;
-
-FoundDecls[TargetDecl] = Using;
-FoundRanges[TargetDecl] = CharSourceRange::getCharRange(
+UsingDeclContext Context(Using);
+Context.UsingDeclRange = CharSourceRange::getCharRange(
Using->getLocStart(),
Lexer::findLocationAfterToken(
Using->getLocEnd(), tok::semi, *Result.SourceManager,
Result.Context->getLangOpts(),
/*SkipTrailingWhitespaceAndNewLine=*/true));
+for (const auto It : Using->shadows()) {
+ const auto *TargetDecl = It->getTargetDecl()->getCanonicalDecl();
+ if (IsValidDecl(TargetDecl))
+Context.UsingTargetDecls.insert(TargetDecl);
+}
+if (!Context.UsingTargetDecls.empty())
+ Contexts.push_back(Context);
return;
}
@@ -93,20 +98,24 @@
}
void UnusedUsingDeclsCheck::removeFromFoundDecls(const Decl *D) {
- auto I = FoundDecls.find(D->getCanonicalDecl());
- if (I != FoundDecls.end())
-I->second = nullptr;
+ for (auto &Context : Contexts) {
+if (Context.UsingTargetDecls.find(D->getCanonicalDecl()) !=
+Context.UsingTargetDecls.end()) {
+ Context.IsUsed = true;
+ break;
+}
+ }
}
void UnusedUsingDeclsCheck::onEndOfTranslationUnit() {
- for (const auto &FoundDecl : FoundDecls) {
-if (FoundDecl.sec
