aaron.ballman added inline comments.
================ Comment at: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp:30 + // They appear in the AST just *prior* to the typedefs. + Finder->addMatcher(cxxRecordDecl().bind("struct"), this); } ---------------- It's unfortunate that we can't restrict this matcher more -- there tend to be a lot of struct declarations, so I am a bit worried about the performance of this matching on so many of them. At a minimum, I think you can restrict it to non-implicit structs here and simplify the code in `check()` a bit. ================ Comment at: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp:55 - // do not fix if there is macro or array - if (MatchedDecl->getUnderlyingType()->isArrayType() || StartLoc.isMacroID()) + // warn at StartLoc but do not fix if there is macro or array + if (MatchedDecl->getUnderlyingType()->isArrayType() || StartLoc.isMacroID()) { ---------------- warn -> Warn Add a full stop to the end of the comment. ================ Comment at: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp:57 + if (MatchedDecl->getUnderlyingType()->isArrayType() || StartLoc.isMacroID()) { + auto Diag = diag(StartLoc, UseUsingWarning); return; ---------------- Is there a purpose to the `Diag` local variable, or can it just be removed? ================ Comment at: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp:85 + ReplaceRange.setBegin(LastReplacementEnd); + Using = "; using "; + ---------------- Should there be a newline here, to avoid putting all the using declarations on the same line? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70270/new/ https://reviews.llvm.org/D70270 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits