ziqingluo-90 added inline comments.
================
Comment at:
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-multi-parm-span.cpp:13-15
+ expected-note{{change type of 'p' to 'std::span' to preserve bounds
information, and change 'q' and 'a' to 'std::span' to propagate bounds
information between them}} \
+ expected-note{{change type of 'q' to 'std::span' to preserve bounds
information, and change 'p' and 'a' to 'std::span' to propagate bounds
information between them}} \
+ expected-note{{change type of 'a' to 'std::span' to preserve bounds
information, and change 'p' and 'q' to 'std::span' to propagate bounds
information between them}}
----------------
ziqingluo-90 wrote:
> NoQ wrote:
> > QoL thing: //"to propagate bounds information between them"// isn't the
> > correct reason in this case. Bounds information doesn't propagate between
> > `p` and `q` and `a`. Each of them carries its own, completely unrelated
> > bounds information.
> >
> > We need to come up with a different reason. Or it might be better to
> > combine the warnings into one warning here, so that we didn't need a reason:
> > ```
> > warning: 'p', 'q' and 'a' are unsafe pointers used for buffer access
> > note: change type of 'p', 'q' and 'a' to 'std::span' to preserve bounds
> > information
> > ```
> >
> > This gets a bit worse when parameters are implicated indirectly, but it
> > still mostly works fine, for example:
> > ```
> > void foo(int *p, int *q) {
> > int *p2 = p;
> > p2[5] = 7;
> > int *q2 = q;
> > q2[5] = 7;
> > }
> > ```
> > would produce:
> > ```
> > warning: 'p2' and 'q2' are unsafe pointers used for buffer access
> > note: change type of 'p2' and 'q2' to 'std::span' to preserve bounds
> > information, and
> > change 'p' and 'q' to 'std::span' to propagate bounds information
> > between them
> > ```
> >
> > ---
> >
> > In any case, this shows that the relationship between parameters (grouped
> > together simply for being parameters) is very different from the
> > relationship within groups of variables implicated by assignments (even if
> > two or more of them are parameters). All the way down to the warning/note
> > printer, we probably need to continue giving the parameter relationship a
> > special treatment.
> This is a good argument! An edge from `p` to `q` in an assignment graph
> stands for not only that fixing `p` implicates fixing `q` but also that the
> bounds information is propagated from `q` to `p`. In this sense, the way I
> connect parameters is inappropriate.
>
> But my concern is that although treating parameters specially could improve
> the quality of the diagnostic messages, the code may be more complex and less
> re-used.
So for such an example
```
int f(int *p, int *q) {
int * a = p;
int * b = q;
a[5] = 0;
b[5] = 0;
}
```
We will fix all four variables `a, b, p, q` together. But bounds information
propagation only happens between `a` and `p`, and `b` and `q`, respectively.
Then what should the diagnostic message be? How about something like
```
change type of 'a' to 'std::span' to preserve bounds information, and change
'p' to propagate bounds information between them.
To ensure parameters are changed together, also
change type of 'b' to 'std::span' to preserve bounds information, and change
'q' to propagate bounds information between them;
...
```
for variable `a`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153059/new/
https://reviews.llvm.org/D153059
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
