ymandel added a comment.

Given that you're removing a declaration, consider using 
`tooling::getAssociatedRange` 
https://source.corp.google.com/piper///depot/google3/third_party/llvm/llvm-project/clang/include/clang/Tooling/Transformer/SourceCode.h;l=44
  It does have a different semantics than the simpler logic that you have here, 
but, its based on code in a dead-code removal tool, so it may still be what you 
want.



================
Comment at: 
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp:51
+  if (Tok && !Invalid) {
+    size_t Offset = std::strcspn(TextAfter, "\n");
+    auto PastNewLine = Stmt.getEndLoc().getLocWithOffset(Offset + 1);
----------------
what happens for the last line in the file? I think the `Offset + 1` below will 
be sketchy (because PastNewLine will be invalid). It might work, but seems best 
avoided.


================
Comment at: 
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp:56
+    // ever comes first.
+    auto End = std::min(PastNewLine, BeforeFirstTokenAfterComment);
+    Diagnostic << FixItHint::CreateRemoval(
----------------
I suspect this could get tripped up by macros and other expansions. Instead of 
`std::min`, consider using `SourceManager::isBeforeInTranslationUnit`
(clang/include/clang/Basic/SourceManager.h;l=1616)? Alternatively, check that 
both locations are file ids before calling `std::min`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105734/new/

https://reviews.llvm.org/D105734

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to