On Tue, Aug 14, 2012 at 4:01 PM, Richard Trieu <[email protected]> wrote:
> On Tue, Aug 14, 2012 at 3:31 PM, Richard Smith <[email protected]>wrote: > >> On Mon, Aug 6, 2012 at 6:47 PM, Richard Trieu <[email protected]> wrote: >> >>> On Mon, Aug 6, 2012 at 4:09 PM, Richard Smith <[email protected]>wrote: >>> >>>> On Mon, Aug 6, 2012 at 3:29 PM, Richard Trieu <[email protected]>wrote: >>>> >>>>> Modify the self-reference visitor to mark pass-by-reference to >>>>> function calls as uninitialized use. This will warn on code such as: >>>>> >>>>> const string Foo = "prefix" + Foo + "suffix"; >>>>> >>>> >>>> Can we put this under a more specific -W flag? -Wuninitialized is >>>> intended to be essentially free of false positives. >>>> >>> Would this be more suited for a different uninitialized group or a new >>> flag altogether? >>> >> >> I think a different warning flag would probably be best; neither >> -Wuninitialized nor -Wconditional-uninitialized really fits well. >> >> >>> I'm a little concerned that this uninitialized-variables warning is >>>> deviating from the behavior of the CFG-based warning, which explicitly >>>> excludes variables passed by reference to functions from its list of uses, >>>> so if this is effective at finding bugs, perhaps we should add this check >>>> there too. >>>> >>> At least for classes, this seems to be a reasonable check. I'm not sure >>> if it would be useful for other cases. >>> >>>> >>>> --- lib/Sema/SemaDecl.cpp (revision 161345) >>>> +++ lib/Sema/SemaDecl.cpp (working copy) >>>> @@ -6223,7 +6223,7 @@ >>>> >>>> void VisitImplicitCastExpr(ImplicitCastExpr *E) { >>>> if ((!isRecordType && E->getCastKind() == CK_LValueToRValue) || >>>> - (isRecordType && E->getCastKind() == CK_NoOp)) >>>> + (E->getCastKind() == CK_NoOp)) >>>> >>>> Drop these parens... But I'm not really clear on what this is checking >>>> for. Presumably qualification conversions, but why are those a good place >>>> to look for self-initialization? >>>> >>> T -> const T conversion when passed by reference. >> >> >> OK, but does that mean we don't catch: >> >> const string s = "Foo " + s + " bar"; >> >> ? It would make more sense to me to check const >> glvalue arguments to function call (including overloaded operator calls >> etc). >> > const string s = "Foo " + s + " bar"; is the motivating case for this > change, which is uncaught by current -Wuninitialized. It's caught by > VisitCallExpr() where it any argument that is just a DeclRefExpr is > checked. OK, then it'd make more sense to me for that check to look through casts (at least lvalue bitcasts, no-op conversions and lvalue-to-rvalue conversions, but possibly others) than to start a search from no-op conversions.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
