Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang-tidy]fix misc-unused-using-decls false positive false for using in elaborated type #70230

Merged
merged 2 commits into from Oct 26, 2023
Merged

[clang-tidy]fix misc-unused-using-decls false positive false for using in elaborated type #70230

merged 2 commits into from Oct 26, 2023

Conversation

HerrCai0907
Copy link
Contributor

ElaboratedType including tag keywords and any nested-name-specifiers.
We should ignore nested-name-specifiers case but consider tag keywords case for misc-unused-using-decls check
Fixes: #69714

@llvmbot
Copy link
Collaborator

llvmbot commented Oct 25, 2023

@llvm/pr-subscribers-clang-tidy

Author: Congcong Cai (HerrCai0907)

Changes

ElaboratedType including tag keywords and any nested-name-specifiers.
We should ignore nested-name-specifiers case but consider tag keywords case for misc-unused-using-decls check
Fixes: #69714


Full diff: https://github.com/llvm/llvm-project/pull/70230.diff

3 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp (+11)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4)
  • (modified) clang-tools-extra/test/clang-tidy/checkers/misc/unused-using-decls.cpp (+9)
diff --git a/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp b/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
index 14b6aca6f3b8fac..14f822c1c086ab9 100644
--- a/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
@@ -8,6 +8,7 @@
 
 #include "UnusedUsingDeclsCheck.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
 
@@ -71,6 +72,10 @@ void UnusedUsingDeclsCheck::registerMatchers(MatchFinder *Finder) {
                          templateArgument().bind("used")))),
                      this);
   Finder->addMatcher(userDefinedLiteral().bind("used"), this);
+  Finder->addMatcher(
+      elaboratedType(unless(hasQualifier(nestedNameSpecifier())),
+                     hasUnqualifiedDesugaredType(type().bind("usedType"))),
+      this);
   // Cases where we can identify the UsingShadowDecl directly, rather than
   // just its target.
   // FIXME: cover more cases in this way, as the AST supports it.
@@ -145,6 +150,12 @@ void UnusedUsingDeclsCheck::check(const MatchFinder::MatchResult &Result) {
     return;
   }
 
+  if (const auto *T = Result.Nodes.getNodeAs<Type>("usedType")) {
+    if (const auto *ND = T->getAsTagDecl())
+      RemoveNamedDecl(ND);
+    return;
+  }
+
   if (const auto *UsedShadow =
           Result.Nodes.getNodeAs<UsingShadowDecl>("usedShadow")) {
     removeFromFoundDecls(UsedShadow->getTargetDecl());
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index b5348384e965ab5..3227e2ef9d81c83 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -290,6 +290,10 @@ Changes in existing checks
   <clang-tidy/checks/misc/redundant-expression>` check to ignore
   false-positives in unevaluated context (e.g., ``decltype``).
 
+- Improved :doc:`misc-unused-using-decls
+  <clang-tidy/checks/misc/unused-using-decls>` check to avoid false positive when
+  using in elaborated type.
+
 - Improved :doc:`modernize-avoid-bind
   <clang-tidy/checks/modernize/avoid-bind>` check to
   not emit a ``return`` for fixes when the function returns ``void``.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/unused-using-decls.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/unused-using-decls.cpp
index 7d02aa4d2e2bf90..12fc18f340f2130 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/misc/unused-using-decls.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/unused-using-decls.cpp
@@ -213,3 +213,12 @@ template <typename T, template <typename> class U> class Bar {};
 // We used to report Q unsued, because we only checked the first template
 // argument.
 Bar<int, Q> *bar;
+
+namespace gh69714 {
+struct StructGH69714_1 {};
+struct StructGH69714_2 {};
+} // namespace gh69714
+using gh69714::StructGH69714_1;
+using gh69714::StructGH69714_2;
+struct StructGH69714_1 a;
+struct StructGH69714_2 *b;

Comment on lines 76 to 77
elaboratedType(unless(hasQualifier(nestedNameSpecifier())),
hasUnqualifiedDesugaredType(type().bind("usedType"))),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
elaboratedType(unless(hasQualifier(nestedNameSpecifier())),
hasUnqualifiedDesugaredType(type().bind("usedType"))),
loc(elaboratedType(unless(hasQualifier(nestedNameSpecifier())),
hasUnqualifiedDesugaredType(type().bind("usedType")))),

To avoid matching too much.

@HerrCai0907 HerrCai0907 merged commit 04ca1b6 into llvm:main Oct 26, 2023
3 of 4 checks passed
@HerrCai0907 HerrCai0907 deleted the fix/UnusedUsingDeclsCheck branch October 26, 2023 01:24
zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Oct 26, 2023
…g in elaborated type (llvm#70230)

`ElaboratedType` including tag keywords and any nested-name-specifiers.
We should ignore nested-name-specifiers case but consider tag keywords
case for `misc-unused-using-decls` check
Fixes: llvm#69714
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

misc-unused-using-decls incorrectly reports using as unused when it is used in elaborated-type-specifier
3 participants