[PATCH] D105734: [clang-tidy] performance-unnecessary-copy-initialization: Do not remove comments on new lines.
Felix Berger via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 12 11:01:01 PDT 2021
flx marked 2 inline comments as done.
flx added a comment.
Thanks for the review!
================
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);
----------------
ymandel wrote:
> 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.
I added a check to not go past the end of the string.
================
Comment at: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp:56
+ // ever comes first.
+ auto End = std::min(PastNewLine, BeforeFirstTokenAfterComment);
+ Diagnostic << FixItHint::CreateRemoval(
----------------
ymandel wrote:
> 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`.
We don't apply any fixes when the decl statement is inside of a macro, but switched to using SourceManager::isBeforeInTranslationUnit.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105734/new/
https://reviews.llvm.org/D105734
More information about the cfe-commits
mailing list