NoQ added inline comments.
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:149-152
+// Matches a `UnaryOperator` whose operator is pre-increment:
+AST_MATCHER(UnaryOperator, isPreInc) {
+ return Node.getOpcode() == UnaryOperator::Opcode::UO_PreInc;
+}
----------------
Oh interesting, so `hasOperatorName` wasn't sufficient, because operators `++`
and `++` have the same "name"?
Yeah, sounds like a universally useful matcher, we should commit it to
`ASTMatchers.h` for everybody to use.
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:663
+ return stmt(isInUnspecifiedPointerContext(expr(
+
ignoringImpCasts(unaryOperator(isPreInc()).bind(UPCPreIncrementTag)))));
+ }
----------------
You're not checking whether the operand is a variable. This isn't incorrect,
given that the fixable will be thrown away if it doesn't claim any variables.
However, for performance it makes sense to never match things that don't act on
variables, so that to never construct the fixable object in the first place.
The check for the variable being an actual `VarDecl` is also useful and can be
made here instead of the `getFixit` method.
I think this is something we should do consistently: //if it's not the right
code, stop doing things with it as early as possible//. Premature optimizations
are evil, but in these cases it's not much of an optimization really, it's just
easier to write code this way from the start.
It also makes the code easier to read: you can easily see which patterns the
gadget covers, by just reading the `matcher()` method.
================
Comment at:
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pre-increment.cpp:25
+
+ // Don't know how to fix the following cases:
+ // CHECK-NOT: fix-it:"{{.*}}":{
----------------
It's probably a good idea to explicitly say `FIXME:` if we think these cases
should eventually be fixed.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144304/new/
https://reviews.llvm.org/D144304
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
