AaronBallman wrote: > > > I don't think there is codegen fixes to be done here. Self-assign like > > > this is UB, and it would be an unnecessary amount of work to try to make > > > this a 'no-op' instead. > > > > > > Is it UB, though? I think, at least in C, it's not clear-cut. > > Its an uninitialized read, which further look seems to be 'indeterminate > value'. I think C++ is 'erroneous' now for this. Either way, our > implementation is conforming here, self-init for non-constructor-types is > implemented as effectively a no-op instead of 'no ops'. > > > > That said, as 'UB', self-assign's 'behavior' through LLVM-IR is > > > effectively no effect, even if it is wasted instructions (that is, `mov > > > eax, eax` won't change anything at all except perhaps invalidating > > > speculation/etc). > > > IMO the clang behavior is completely making sense: we diagnose when this > > > COULD be problematic (such as causing copy constructor from a > > > not-yet-life-constructed self). > > > I'm a bit concerned about: `The self-assignment initialization Type var = > > > var; is an idiom in C code.` from the commit message. Is this true? Is > > > this REALLY that common? To what end? > > > > > > I've seen it in C code but don't have a good idea of prevalence. The uses > > I've seen were for silencing "unused variable" warnings in C89 code but I'm > > not certain how crucial they are to support. > > Clang and GCC have some difference in behavior for how we handle > > `-Winit-self` and `-Wuninitialized`, too. It's not clear whether those > > differences are intentional or not. > > My understanding is that Clang's warnings is at least reasonable, so I'd > probably need to see some justification to change that? Carving out this > exception in the static-analyzer is a 'neutral' for me, though I've done SOME > googling and see some suggestions to do self-init to suppress > `-Wuninitialized` on google. The carve-out of `-Winit-self` being separate > seems to be supporting evidence for this... so I guess I'm > slightly-above-neutral?
I think Clang's behavior suggests that *maybe* we don't need to support this idiom any longer. GCC only diagnoses `-Winit-self` when passed with `-Wuninitialized` because they're only warning if the variable is *used* outside of its initialization: https://godbolt.org/z/s1EqnMd6v Clang does not do anything with `-Winit-self` and instead always diagnoses as an uninitialized use: https://godbolt.org/z/qde4c53xs and `-Wuninitialized` is enabled by `-Wall`: https://godbolt.org/z/TWbn93a43 So if this was a common pattern, I would expect our `-Wall` behavior would have caused a fair amount of problems requiring us to actually support `-Winit-self` instead of silently ignoring it. https://github.com/llvm/llvm-project/pull/187530 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
