On 06/29/2017 11:57 AM, Jeff Law wrote: > On 05/23/2017 09:58 AM, Martin Sebor wrote: >> On 05/18/2017 12:55 PM, Prathamesh Kulkarni wrote: >>> Hi, >>> The attached patch tries to fix PR80806 by warning when a variable is >>> set using memset (and friends) but not used. I chose to warn in dse >>> pass since dse would detect if the variable passed as 1st argument is >>> a dead store. Does this approach look OK ? >> >> Detecting -Wunused-but-set-variable in the optimizer means that >> the warning will not be issued without optimization. It also >> means that the warning will trigger in cases where the variable >> is used conditionally and the condition is subject to constant >> propagation. For instance: > Yea. There's definitely tradeoffs for implementing warnings early vs > late. There's little doubt we could construct testcases where an early > warning would miss cases that could be caught by a late warning. > > >> >> void sink (void*); >> >> void test (int i) >> { >> char buf[10]; // -Wunused-but-set-variable >> memset (buf, 0, sizeof(buf)); >> >> if (i) >> sink (buf); >> } >> >> void f (void) >> { >> test (0); >> } >> >> I suspect this would be considered a false positive by most users. >> In my view, it would be more in line with the design of the warning >> to enhance the front end to detect this case, and it would avoid >> these issues. > Given no knowledge of sink() here, don't we have to assume that buf is > used? So, yea, I'd probably consider that a false positive. Oh, wait, I missed the constant propagation. That makes this one less clear cut in my mind -- it means its context sensitive. I could easily argue either way on this one.
Jeff