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

Reply via email to