whisperity added a comment.
`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.
================
Comment at: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp:29
+ Finder->addMatcher(typedefDecl(unless(isInstantiated()),
+ hasParent(decl().bind("parent-decl")))
+ .bind("typedef"),
----------------
(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.)
================
Comment at: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp:50-52
+ if (!ParentDecl) {
+ return;
+ }
----------------
(Style: Elid braces.)
================
Comment at: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp:132-137
bool Invalid;
- Type = std::string(
- Lexer::getSourceText(CharSourceRange::getTokenRange(LastTagDeclRange),
- *Result.SourceManager, getLangOpts(), &Invalid));
+ Type = std::string(Lexer::getSourceText(
+ CharSourceRange::getTokenRange(LastTagDeclRange->second),
+ *Result.SourceManager, getLangOpts(), &Invalid));
if (Invalid)
return;
----------------
(Nit: if the source ranges are invalid, a default constructed `StringRef` is
returned, which is empty and converts to a default-constructed `std::string`.)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113804/new/
https://reviews.llvm.org/D113804
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
