[PATCH] D113804: [clang-tidy] Fix behavior of `modernize-use-using` with nested structs/unions

[Whisperity via Phabricator via cfe-commits]

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