[PATCH] D91302: Handle template instantiations better in clang-tidy check
Stephen Kelly via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 22 07:33:31 PST 2020
steveire marked 2 inline comments as done.
steveire added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp:26-28
+ // The first argument of an overloaded member operator is the implicit object
+ // argument of the method which should not be matched against a parameter, so
+ // we skip over it here.
----------------
aaron.ballman wrote:
> Isn't this true for any non-static member function? e.g., should the matcher be looking at `cxxOperatorCallExpr()` at all?
I think it's deliberate - https://godbolt.org/z/EYYndv - it's just that we don't consider the `aa` an argument because the op is a member func.
================
Comment at: clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp:67
+ hasParent(forStmt(hasCondition(expr(equalsBoundNode(exprName))))),
+ hasParent(varDecl(hasType(booleanType()))),
+ hasParent(
----------------
aaron.ballman wrote:
> If this matters for var decls, what about field decls in a structure?
Not sure. What would this look like in a test?
================
Comment at: clang-tools-extra/test/clang-tidy/checkers/readability-container-size-empty.cpp:493
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
+ // CHECK-MESSAGES: :9:8: note: method 'vector'::empty() defined here
+ // CHECK-FIXES: {{^ }}if (!v.empty()){{$}}
----------------
aaron.ballman wrote:
> The quoting here is unfortunate. It's not part of your patch, but it could be solved by calling `Container->getName()` and manually quoting the note text if you wanted to hit it in a follow-up (I'd consider that an NFC change, no need to review). You could also address it as part of this patch if you felt like it.
That will be a follow-up.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91302/new/
https://reviews.llvm.org/D91302
More information about the cfe-commits
mailing list