On Mon, Apr 30, 2012 at 9:05 AM, Douglas Gregor <[email protected]> wrote: > > On Apr 29, 2012, at 9:05 PM, David Blaikie wrote: > >> Just something I've had lying around for a while - I'm not sure >> whether this meets the fixit bar, presumably the real mistakes people >> make when using NULL is that they were calling the wrong function, >> passing arguments in the wrong order, etc - if they're simply passing >> NULL when they meant 0, that's not really the cause of bugs. > > +const char *Sema::getFixItZeroLiteralForType(QualType T) const { > + const char *Str = getFixItZeroInitializerForType(T); > + if (!Str) > + return Str; > + size_t Len = std::strlen(Str); > + assert(Len > 3 && "Should be a non-record type init"); > + assert(strncmp(Str, " = ", 3) == 0 && "Should be non-record type init"); > + return Str + 3; > +} > > This feels very brittle.
Fair point. (repeating from IRC for posterity - I think I'd wanted to avoid string concatenation, but since these will all be small that's probably not worth avoiding. Also, the need to return a "non value" (null in this case) meant string was a bit awkward - but the use of the empty string for the non-value has actually tidied some callers up a bit) I've refactored it out into a common function - though it means getFixItZeroLiteralForType is now a trivial wrapper. Should that just be folded into getFixItZeroLiteralForType, or do you prefer the two to use a common function to make the sharing a bit clearer? - David > At the very least, this needs a comment in getFixItZeroInitializerForType() > so that we know to look here each time we change it. > getFixItZeroLiteralForType() then needs more assertions so that it is obvious > to that future reader what the invariants are. Also, 'Len' isn't needed as a > variable, and will cause a warning in a non-asserts build. [Good points I'll try to keep in mind for future changes - no longer applicable to this change.]
null_fixit.diff
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
