#23 Check for mutable default arguments

open
nobody
None
5
2005-07-14
2005-07-14
No

I got bitten by mutable default arguments, and wanted
pychecker to find those for me.

This is sort of a pseudo patch, since I don't know the
pychecker codebase (and I didn't seem to find a decent
diff utility at the moment)

Patch probably need to be reworked a bit (and probably
add a switch to turn the check on/off) to fit nicely
into pychecker. But I think that it would be a
beneficial check to add.

Discussion

  • Simon Dahlbacka

    Simon Dahlbacka - 2005-07-15

    Logged In: YES
    user_id=750513

    Got myself together and prodused a somewhat more usable
    patch. This one adds a --mutabledefaults option that
    currently defaults to on.

    The patch tests all the function arguments against
    MUTABLE_TYPES, and issues a warning is a mutable argument is
    found. Currently it does not bother to check the rest of the
    arguments if one that is mutable was found.

     
  • Neal Norwitz

    Neal Norwitz - 2005-08-31

    Logged In: YES
    user_id=33168

    This patch seems to implement similar functionality to the
    following option: -j, --moddefvalue check if modify (call
    method) on a parameter that has a default value [on]

    I understand this goes through all arguments where the
    current code tries to check default argument accesses. It
    does this to try to eliminate spurious warnings. Can you
    give an example where the current code doesn't work?

    Thanks.

     
  • Simon Dahlbacka

    Simon Dahlbacka - 2005-08-31

    Logged In: YES
    user_id=750513

    I can understand the need to avoid spurious warnings, (I
    also currently have problems with too many spurious warnings
    that leads to not using pychecker as often as I should)

    That aside, the following testcase shows two cases where
    current code do not find the modification of the default
    value. In my experience, a mutable default value is more
    often than not a bug (that also can be hard to pinpoint)

     
  • Neal Norwitz

    Neal Norwitz - 2005-08-31

    Logged In: YES
    user_id=33168

    Ok, I buy that argument (ie, the code). Though, I'm not
    sure how to marry the existing option with this one or if
    they should remain separate.

    What problems are you having with spurious warnings? Are
    you using the last release 0.8.14 or the CVS version? The
    CVS version does have a lot of bug fixes. I hope to release
    it real soon now, but there's been so many changes over a
    long period, I think there will be some problems with it.

     

Log in to post a comment.