erichkeane wrote:

> So from the user's perspective we're checking the flag twice: once at the `}` 
> to see if we need to ramp up the analysis, then again at the diagnostic 
> location to see if the diagnostic should be discarded. The warning will only 
> be emitted if it's enabled at both source locations.
> 
> I like this. I think this is a very good practical tradeoff.
> 
> It behaves very counter-intuitively if you try to enable the diagnostic at 
> one line with a pragma but it doesn't show up until you wrap your entire 
> function in a pragma. But this is a very unlikely situation to occur 
> naturally. Even if you do use the pragma to enable the diagnostic, why would 
> you minimize the scope this way? (I also really like @cor3ntin's idea of 
> emitting an explanatory warning if we can detect this case.)
>

The current patch actually WILL work with it ONLY enabled on the line inside of 
the function, which I think is much more intuitive.  The previous one (before 
Aaron implemented my tracking suggestion) did as you mentioned.  My proposal 
was to track throughout the function if it was EVER turned on, so start the 
analysis if it is EVER turned on, then emit it if it is in the source-lcoation 
at the place of diagnostic.
 
> The opposite situation, where the diagnostic is disabled on an individual 
> line but enabled globally, is significantly more likely. And it is also the 
> one that acts in an intuitive manner!
> 
> @ziqingluo-90 @jkorous-apple You might want to take a look at how this 
> interacts with `-Wunsafe-buffer-usage`. If you still need to analyze the 
> entire TU at once, the approach proposed in this patch probably won't work 
> for you.



https://github.com/llvm/llvm-project/pull/136323
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to