[PATCH] D146669: [-Wunsafe-buffer-usage] Hide fixits/suggestions behind an extra flag, -fsafe-buffer-usage-suggestions.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

So in a nutshell, this is the intended behavior:

| code                 | diagnostic                                                                          | EmitFixits && EmitSuggestions | !EmitFixits && EmitSuggestions | !EmitFixits && !EmitSuggestions |
| -------------------- | ----------------------------------------------------------------------------------- | ----------------------------- | ------------------------------ | ------------------------------- |
| `void foo(int *p)` { | note: 'p' declared here                                                             | yes                           | yes                            | no                              |
| `  int *q;`          | warning: 'q' is unsafe pointer to raw buffer                                        | yes                           | yes                            | no                              |
|                      | note with fixit: change type of 'q' and 'p' to span to propagate bounds information | yes                           | yes (no fixit)                 | no                              |
| `  q = p;`           | note: 'bounds information propagates from 'p' to 'q' here                           | yes                           | yes                            | no                              |
| `  q[5] = 10;`       | note: unsafe raw buffer operation here                                              | yes                           | yes                            | yes (warning)                   |
|                      | note: pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions    | no                            | no                             | yes                             |
| `}`                  |                                                                                     |                               |                                |                                 |
|

The first mode, "**EmitFixits && EmitSuggestions**", is the default behavior before this patch, and the behavior under `-fsafe-buffer-usage-suggestions` after this patch. The third mode, "**!EmitFixits && !EmitSuggestions**", is the default behavior after the patch.

The second mode, "**!EmitFixits && EmitSuggestions**" (the special behavior under `-fno-diagnostics-fixit-info`) was implemented for two purposes:

- Recover some performance when `-fno-diagnostics-fixit-info` is passed (don't run the sophisticated fixit machinery if fixits are going to be discarded);
- As a possible workaround for potential crashes in the fixit machinery.

We initially hoped that this mode will make "**!EmitSuggestions**" unnecessary, but as the table above demonstrates, a very large portion of the fixit machine's logic is still being invoked for the purposes of building notes, even when fixits aren't attached. And we cannot really disable this logic under that compiler flag, given that  `-fno-diagnostics-fixit-info` already has a well-defined meaning which doesn't allow us to change the shape of the diagnostics in response. So we needed a new, more powerful flag, so here we are. I suspect that explicit support for `!EmitFixits` is no longer necessary. It is now useful for performance purposes only, and we don't any concrete data to support the claim that it's a valuable optimization, nor do we believe that this flag is ever intentionally passed in practice, so I guess I'll go ahead and remove it.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146669/new/

https://reviews.llvm.org/D146669