[Pull Request] Remove uses of hasattr

Adrian Buehlmann adrian at cadifra.com
Wed Jul 27 13:54:51 CDT 2011


On 2011-07-27 20:25, Augie Fackler wrote:
> 
> On Jul 26, 2011, at 2:54 AM, Adrian Buehlmann wrote:
> 
>> On 2011-07-26 11:40, Adrian Buehlmann wrote:
>>> On 2011-07-26 05:44, Augie Fackler wrote:
>>>> A while ago I asked about removing hasattr because of surprising behavior, and the general consensus seemed to be that I should go ahead and make a run at it. While on a flight today I finally finished the job. It's a really long patch queue, so instead of mailing it I'm submitting it as a pull request. You can see the changesets here:
>>>>
>>>> http://code.google.com/p/durin42-hg-hasattr-ectomy/source/list?num=50
>>>>
>>>> and if you want to pull/incoming, the URL you want is
>>>>
>>>> http://code.google.com/p/durin42-hg-hasattr-ectomy/
>>>>
>>>> It's 38 patches in total. I tried to batch them for easy reviewing. Ones of note are:
>>>>
>>>> Duplicate less code when checking to see if we're in a frozen binary:
>>>> http://code.google.com/p/durin42-hg-hasattr-ectomy/source/detail?r=b6581ed193f374f2d38a4cad5ad25640f304ce1f
>>>>
>>>> These two attack a particular hasattr() use case in the entire codebase since it's mechanically fixable:
>>>> http://code.google.com/p/durin42-hg-hasattr-ectomy/source/detail?r=f4814ebb99f75812a55776ed3a306883432935dd
>>>> http://code.google.com/p/durin42-hg-hasattr-ectomy/source/detail?r=235f15a351866c2c084d4e88b89f7b69fafa66da
>>>>
>>>> and this last one of note adds modules to the blacklist because of those side effects of hasattr() I warned about:
>>>> http://code.google.com/p/durin42-hg-hasattr-ectomy/source/detail?r=57c2982d243bd6651e1d9e3f138f46b27b32828f
>>>>
>>>> Other than that, it's mostly mechanical replacement, with a few migrations to getattr() so we can avoid looking up the attribute twice. I tried to make the patches all tiny so they'd be easy to review.
>>>
>>> Augie asked to pull:
>>>> # HG changeset patch
>>>> # User Augie Fackler <durin42 at gmail.com>
>>>> # Date 1311623995 18000
>>>> # Node ID 9b736a33999292cf7fe5a8e0edbd145c9dd32e19
>>>> # Parent  b6581ed193f374f2d38a4cad5ad25640f304ce1f
>>>> safehasattr: new function to work around hasattr being broken
>>>>
>>>> diff --git a/mercurial/util.py b/mercurial/util.py
>>>> --- a/mercurial/util.py
>>>> +++ b/mercurial/util.py
>>>> @@ -29,6 +29,10 @@
>>>> def sha1(s):
>>>>    return _fastsha1(s)
>>>>
>>>> +_notset = object()
>>>> +def safehasattr(thing, attr):
>>>> +    return getattr(thing, attr, _notset) is not _notset
>>>> +
>>>> def _fastsha1(s):
>>>>    # This function will import sha1 from hashlib or sha (whichever is
>>>>    # available) and overwrite itself with it on the first call.
>>>
>>> IMHO, it would have been nice to write in the change message in what
>>> ways hasattr is broken.
>>>
>>> Or at least put a pointer to an explanation.
>>
>> Augie patched:
>>> # HG changeset patch
>>> # User Augie Fackler <durin42 at gmail.com>
>>> # Date 1311623971 18000
>>> # Node ID e7859521b23b358659ab8a918677ccaf3ed46008
>>> # Parent  27f3169fd8f4affa374201af287698d5f59c920f
>>> check-code: disallow use of hasattr()
>>
>> Or perhaps the explanation why hasattr is bad could be given in this
>> change message (it's the topmost patch)
> 
> 
> http://code.google.com/p/durin42-hg-hasattr-ectomy/source/list?num=50
> 
> repushed.
> 

Excellent explanation there. Vielen Dank.



More information about the Mercurial-devel mailing list