Page MenuHomePhabricator

[clang-tidy] Add readability-redundant-using check
Needs ReviewPublic

Authored by nullptr.cpp on Feb 23 2021, 11:28 PM.

Details

Summary

Finds redundant using declarations and directives.

Example:

namespace n {
void func();
}

namespace n {
using n::func; // redundant using declaration, already in namespace 'n'.
}
namespace n {
using namespace n; // redundant using directive, already in namespace 'n'.
}

Diff Detail

Event Timeline

nullptr.cpp created this revision.Feb 23 2021, 11:28 PM
nullptr.cpp requested review of this revision.Feb 23 2021, 11:28 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 23 2021, 11:28 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript

I think this check belongs to readability module.

clang-tools-extra/clang-tidy/misc/RedundantUsingCheck.cpp
1 ↗(On Diff #325995)

Please make single line from two. See other checks as example.

njames93 added inline comments.Feb 24 2021, 8:31 AM
clang-tools-extra/clang-tidy/misc/RedundantUsingCheck.cpp
23 ↗(On Diff #325995)

Why restrict these to main file only?

30–33 ↗(On Diff #325995)

nit: Elide braces of single line if statements.

35–38 ↗(On Diff #325995)
45 ↗(On Diff #325995)

nit: don't use auto as type isn't explicitly spelled in the initialization.

50 ↗(On Diff #325995)

Shouldn't there be a break after we have diagnosed this?

78 ↗(On Diff #325995)

nit: The canonical way to do diagnostics like this is with select. Then pass 0 for declaration and 1 for directive.

nullptr.cpp retitled this revision from [clang-tidy] Add misc-redundant-using check to [clang-tidy] Add readability-redundant-using check.

Modify according to suggestions

balazske added inline comments.Feb 25 2021, 5:57 AM
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-using.cpp
115

Add more tests?

namespace outer {
namespace inner {
void func();
} // namespace inner
using inner::func();
} // namespace outer

using namespace outer;
using namespace inner;

Can you override the isLanguageVersionSupported method to ensure this check only runs over c++ code.
Can you add some macro tests, typically we err on the side of caution and don't emit any fixes for macro code. You can use SourceLocation::IsMacroID() to detect macro expansions.
The description looks to have an error in the 2nd example, guess you are messing 'n' after the namespace.
How will this handle inline namespaces.

namespace N {
inline namespace M {
void foo();
void bar();
} // namespace M
inline namespace O {
void bar();
}
using M::foo; // This is redundant.
using M::bar; // This is gonna give you a hard time.
} // namespace N
clang-tools-extra/clang-tidy/readability/RedundantUsingCheck.cpp
63

nit: New line between methods.

You can look into misc/UnusedUsingDeclsCheck.cpp (if not done yet), that check handles using too. And that check is related to this check, the module should be the same too (misc)?

There should be already a name lookup implemented somewhere in clang (DeclContext? or Sema) that maybe usable here (check if the name of the "used" entity is visible in the declaration context of the using statement). Probably it is not as simple to do because the using is already there and it may depend on the order.

You can look into misc/UnusedUsingDeclsCheck.cpp (if not done yet), that check handles using too. And that check is related to this check, the module should be the same too (misc)?

Majority of redundant checks are in readability. Awhile ago I suggested separate group for unused checks or move them to Clang.

  • Restrict to only run on C++ code
  • Ignore using defined in macro
  • Support inline namespace
  • Support global namespace
nullptr.cpp edited the summary of this revision. (Show Details)Mar 6 2021, 11:27 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 16 2021, 5:48 AM
nullptr.cpp retitled this revision from [clang-tidy] Add readability-redundant-using check to [clang-tidy] Add cppcoreguidelines-comparison-operator.
nullptr.cpp edited the summary of this revision. (Show Details)

Update

nullptr.cpp retitled this revision from [clang-tidy] Add cppcoreguidelines-comparison-operator to [clang-tidy] Add readability-redundant-using check.
nullptr.cpp edited the summary of this revision. (Show Details)

Rebase

  • Restrict to only run on C++ code
  • Ignore using defined in macro
  • Support inline namespace
  • Support global namespace

Ping.