On Tue, Aug 5, 2014 at 3:43 PM, Chandler Carruth <[email protected]> wrote:
> > On Tue, Aug 5, 2014 at 3:36 PM, Nico Weber <[email protected]> wrote: > >> This is a very useful warning, thanks. >> >> One issue I have with it is that it's disabled >> by Wno-tautological-compare. From the warning name that makes sense – >> but Wtautological-compare used to be a fairly harmless warning, so we >> (chromium) disabled it for a bunch of third-party libraries. When >> Wtautological-undefined-compare arrived, we fixed all instances of this in >> our code and updated our compiler to a clang that optimizes away >> comparisons of references with NULL (since we had fixed all of these >> comparisons, we thought!) >> >> I recently realized that we didn't see Wtautological-undefined-compare >> warnings for all the third-party libraries that we're building with >> Wno-tautological-compare. >> >> Do you think Wtautological-undefined-compare should be in its own warning >> group (and maybe have a different name to make that clear), so that folks >> who disabled Wtautological-compare still get this warning? The reasoning is >> that this warning is much more serious that what Wtautological-compare used >> to warn about. >> > > I don't really understand why. There is no actual undefined behavior here, > just a comparison that can never, ever be go the other way. It seems almost > exactly as severe as comparing an unsigned number for >= 0? > The undefined behavior isn't the check, but the code that preceded it and that made someone put in the fix: int* a = nullptr; int& ra = *a; // Undefined! But also harmless in practice. if (a == 5) {} // Crash! Better "fix" this by instead writing: if (&a && a == 5) {} // No crash! Except if the compiler rightfully optimizes away the &a check. If you don't have an ubsan bot, then it's possible that your code has references to null. That's undefined, but used to be safe in practice – but now that clang marks addresses of references as non-null, it isn't anymore. Wtautological-undefined-compare catches some of these places.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
