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.


cfe-commits mailing list

Reply via email to