This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10014

debugcommands: prevent using `is False`
ClosedPublic

Authored by pulkit on Feb 17 2021, 2:43 PM.

Details

Reviewers
Alphare
marmoute
Group Reviewers
hg-reviewers
Commits
rHG9306a16ca964: debugcommands: prevent using `is False`
Summary

I was touching this code in a future patch and marmoute warned about usage of
is False here.

Quoting marmoute:

"is False" is going to check if the object you have the very same object in
memory than the one Python allocated for False (in practice 0)
This will "mostly work" on cpython because of implementation details, but
is semantically wrong and can start breaking unexpectedly

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

pulkit created this revision.Feb 17 2021, 2:43 PM
mharbison72 added a subscriber: mharbison72.Feb 17 2021, 11:29 PM

I was touching this code in a future patch and marmoute warned about usage of is False here.
Quoting marmoute:

"is False" is going to check if the object you have the very same object in
memory than the one Python allocated for False (in practice 0)
This will "mostly work" on cpython because of implementation details, but
is semantically wrong and can start breaking unexpectedly

Is if foo == False valid in this case? I probably got my wires crossed with is None. (I'm fine with the code change here, just wondering for future reference.)

Alphare accepted this revision.Feb 23 2021, 4:20 AM
Alphare added a subscriber: Alphare.
In D10014#151711, @mharbison72 wrote:

Is if foo == False valid in this case? I probably got my wires crossed with is None. (I'm fine with the code change here, just wondering for future reference.)

You will probably always want to use if not foo instead. if is None checks against the None singleton, so it's fine, but there is no singleton (aside from implementation details as marmoute said) for booleans in Python.

marmoute accepted this revision.Mar 1 2021, 3:51 AM
marmoute added a subscriber: marmoute.

☺

baymax updated this revision to Diff 26109.Mar 5 2021, 1:17 PM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

pulkit added a commit: rHG9306a16ca964: debugcommands: prevent using `is False`.Mar 8 2021, 3:25 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG9306a16ca964: debugcommands: prevent using `is False` (authored by pulkit). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/debugcommands.py (6 lines)

Diff 26142

mercurial/debugcommands.py

def debugtagscache(ui, repo): def debugtagscache(ui, repo):
"""display the contents of .hg/cache/hgtagsfnodes1""" """display the contents of .hg/cache/hgtagsfnodes1"""
cache = tagsmod.hgtagsfnodescache(repo.unfiltered()) cache = tagsmod.hgtagsfnodescache(repo.unfiltered())
for r in repo: for r in repo:
node = repo[r].node() node = repo[r].node()
tagsnode = cache.getfnode(node, computemissing=False) tagsnode = cache.getfnode(node, computemissing=False)
if tagsnode: if tagsnode:
tagsnodedisplay = hex(tagsnode) tagsnodedisplay = hex(tagsnode)
elif tagsnode is False: elif tagsnode is None:
tagsnodedisplay = b'invalid'
else:
tagsnodedisplay = b'missing' tagsnodedisplay = b'missing'
else:
tagsnodedisplay = b'invalid'
ui.write(b'%d %s %s\n' % (r, hex(node), tagsnodedisplay)) ui.write(b'%d %s %s\n' % (r, hex(node), tagsnodedisplay))
@command( @command(
b'debugtemplate', b'debugtemplate',
[ [
(b'r', b'rev', [], _(b'apply template on changesets'), _(b'REV')), (b'r', b'rev', [], _(b'apply template on changesets'), _(b'REV')),