aaron.ballman added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:316-317
return nullptr;
+ if (!Member->getMemberDecl()->getDeclName().isIdentifier())
+ return nullptr;
StringRef Name = Member->getMemberDecl()->getName();
----------------
It's really strange to me that we're even getting to this point in the check --
the only way for the assertion to fail is for the member call expression to be
on something without a name. The cases I can think of for that would be
something like `foo.operator+(RHS)` or something similarly nonsensical within
this context (we're looking for things named `begin` or `end`). I think it'd
make more sense to handle this at the matcher level (or early in the call
chain) so that we never get here.
I think having a test case would be really useful to trying to understand what
changes are appropriate. I don't think these changes are wrong so much as I
wonder if we're in the wrong place to make them (and we'll hit other confused
code elsewhere).
================
Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:549
const auto *AliasVar = cast<VarDecl>(AliasDecl->getSingleDecl());
+ assert(AliasVar->getDeclName().isIdentifier());
VarName = AliasVar->getName().str();
----------------
This doesn't seem necessary? Calling `NamedDecl::getName()` already asserts
that the name is an identifier.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97889/new/
https://reviews.llvm.org/D97889
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
