Fixes issue #50334.
Details
Diff Detail
Event Timeline
clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h | ||
---|---|---|
27 | Considering the pointer is just a number that hashes well, is there a reason to use the plain std::map? It is generally discouraged. | |
clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp | ||
305–308 | Just for the sake of testing a new logic, it would be beneficial to add a "more complex" nested example. At least one example where there are two sibling nests, and one where there is an additional level of nesting. |
hasParent() is the direct upwards traversal, right? I remember some discussion about matchers like that being potentially costly. But I can't find any description of this, so I guess it's fine, rewriting the matcher to have the opposite logic (decl(hasDescendant(...the-real-matcher-here...)).bind("blah")) would just make everything ugly for no tangible benefit.
clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp | ||
---|---|---|
29 | (Nit: Once we have multiple uses of an identifier, it's better to hoist it to a definition and reuse it that way, to prevent future typos introducing arcane issues. Generally, a static constexpr llvm::StringLiteral in the TU's global scope, is sufficient for that.) | |
50–52 | (Style: Elid braces.) | |
132–137 | (Nit: if the source ranges are invalid, a default constructed StringRef is returned, which is empty and converts to a default-constructed std::string.) |
LGTM aside from a nit.
clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp | ||
---|---|---|
35–44 | I recall that using hasParent() and hasAncestor() tend to be expensive because they walk up the entire AST to the TU level, and so they can also match in surprising places. However, I can't seem to devise a test case that will fail, so I think this is probably okay. | |
clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp | ||
306 | We don't typically use clang-format comments (especially not in tests; those are almost never formatted anyway). Same below. |
Considering the pointer is just a number that hashes well, is there a reason to use the plain std::map? It is generally discouraged.