NoQ added inline comments.
================
Comment at: clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h:29
+ /// Invoked when an unsafe operation over raw pointers is found.
+ virtual void handleUnsafeOperation(const Stmt *Operation) = 0;
+};
----------------
xazax.hun wrote:
> What is the purpose of the handler, would this add the fixit hints? In that
> case, is this the right abstraction level? E.g., do we want to group these by
> the declarations so the handler can rewrite the declaration and its uses at
> once?
You're absolutely right, we want to group fixits by declarations when fixits
are actually available.
But we still need to cover the case when fixits *aren't* available, and in this
case it doesn't make sense to group anything, so this interface isn't going
away.
And on top of that, there's layers to grouping. For instance, in this code
```
void foo(int *p, size_t size) {
int *q = p;
for (size_t i = 0; i < size; ++i)
q[i] = 0;
}
```
we'll need to attach the fix to the declaration `p` rather than `q`, as we aim
to provide a single fixit for these two variables, because otherwise we end up
with a low-quality fix that suppresses the warning but doesn't carry the span
all the way from the caller to the use site.
Then if we have two parameters that we want to fix simultaneously this way, the
fix will have to be inevitably grouped by *function* declaration.
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:22
+// Scan the function and return a list of gadgets found with provided kits.
+static GadgetList findGadgets(const Decl *D) {
+
----------------
xazax.hun wrote:
> Is this not a FunctionDecl because of ObjCMethod? At some point I wonder if
> we should have an abstraction that works for both. Would NamedDecl work?
Yupp.
Well, we do have https://clang.llvm.org/doxygen/classclang_1_1AnyCall.html But
it's not like the code actually cares at this point. And once it does (eg. for
the purposes of fixits), it'll have to consider them separately anyway.
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:35-36
+
+ GadgetList G;
+ MatchFinder M;
+
----------------
xazax.hun wrote:
> Move these close to their uses?
The code in between is actually instantly deleted by the follow-up patch :)
(D137348)
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137346/new/
https://reviews.llvm.org/D137346
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
