develooper Front page | perl.perl5.porters | Postings from March 2015

Re: testing non-nullness of non-null args

Thread Previous | Thread Next
From:
Dave Mitchell
Date:
March 4, 2015 16:06
Subject:
Re: testing non-nullness of non-null args
Message ID:
20150304160605.GZ28599@iabyn.com
On Wed, Feb 25, 2015 at 01:39:36PM +1100, Tony Cook wrote:
> On Tue, Feb 24, 2015 at 11:28:29AM +0000, Dave Mitchell wrote:
> > 
> > Clang has started throwing a lot of warnings like these:
> > 
> >     op.c:3240:10: warning: nonnull parameter 'o' will evaluate to 'true' on first encounter [-Wpointer-bool-conversion]
> >         if (!o || (PL_parser && PL_parser->error_count))
> > 
> > for code like this:
> > 
> >     Perl_doref(pTHX_ OP *o, I32 type, bool set_op_ref)
> >     {
> >         dVAR;
> >         OP *kid;
> > 
> >         PERL_ARGS_ASSERT_DOREF;
> > 
> >         if (!o || (PL_parser && PL_parser->error_count))
> >             return o;
> > 
> > where the o arg is declared NN in embed.fnc.
> > 
> > So the question is whether for such functions we stop testing vars for
> > nullness within the main body of the function?
> 
> There are existing commits that remove such checks:
> 
> 6c4650b32e1142e017440b6cef647077dfed2d1a
> 492254706dd070a67dab2799fba515e6e4c3a3bb
> 5805b5855c54176df1831b2fbbb0fae5dc2d52ad
> 1061065f7a09399eefb50e9a035502621722bcc0


I have pushes the branch davem/nonnull. Unless anyone objects, I'll
merge it in a few days.

commit d8b8b7268243f21e9600c2ba53a74710c665698c
Author:     David Mitchell <davem@iabyn.com>
AuthorDate: Wed Mar 4 15:30:00 2015 +0000
Commit:     David Mitchell <davem@iabyn.com>
CommitDate: Wed Mar 4 15:30:00 2015 +0000

    don't test non-null args
    
    For lots of core functions:
    
    if a function parameter has been declared NN in embed.fnc, don't test for
    nullness at the start of the function, i.e. eliminate code like
    
        if (!foo) ...
    
    On debugging builds the test is redundant, as the PERL_ARGS_ASSERT_FOO
    at the start of the function will already have croaked.
    
    On optimised builds, it will skip the check (and so be slightly faster),
    but if actually passed a null arg, will now crash with a null-deref SEGV
    rather than doing whatever the check used to do (e.g. croak, or silently
    return and let the caller's code logic to go awry). But hopefully  this
    should never happen as such instances will already have been detected on
    debugging builds.
    
    It also has the advantage of shutting up recent clangs which spew forth
    lots of stuff like:
    
        sv.c:6308:10: warning: nonnull parameter 'bigstr' will evaluate to
        'true' on first encounter [-Wpointer-bool-conversion]
            if (!bigstr)
    
    The only exception was in dump.c, where rather than skipping the null
    test, I instead changed the function def in embed.fnc to allow a null arg,
    on the basis that dump functions are often used for debugging (where
    pointers may unexpectedly become NULL) and it's better there to display
    that this item is null than to SEGV.
    
    See the p5p thread starting at 20150224112829.GG28599@iabyn.com.

-- 
"Procrastination grows to fill the available time"
    -- Mitchell's corollary to Parkinson's Law

Thread Previous | Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About