1515170 - Don't use `is` for comparing strings

Status: RESOLVED FIXED

(Core :: IPC, defect, P1)

Description

[Alex Gaynor [:Alex_Gaynor]]

|is| is a pointer identity comparison, and the guarantees around it aren't as strong as the code that's using it assumes. Specifically lower.py currently assumes that various values will be pointer equal to constants in source (e.g. |obj.attr is "string"|). Right now this happens to work because all of the strings are literals in the same module, and they're all interned, but various refactors could breaks this quite easily -- namely if any of these strings were produced by concat/format/slicing they would not be interned, and these comparisons would be confusingly false.

(This message brought to you by "thinking about refactorings and seeing a footgun").

Comment 1

[Alex Gaynor [:Alex_Gaynor]]

Attachment: Bug 1515170 - in the IPDL compiler, don't use 'is' for comparing strings; r?nika

Comment 2

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Thanks for fixing this; that pattern confused/scared me the first time I saw it (and led to a Learning Experience™ about Python), but I don't think I ever got around to filing a bug.

Comment 3

[Alex Gaynor [:Alex_Gaynor]]

Yup, this is a pattern that will work, right up until it doesn't. There's similarly using |is| with integers, which will work if they're in [-5, 256] (or something like that), and fail otherwise, because there's a cache for small integers.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0e2f7d9f5628
in the IPDL compiler, don't use 'is' for comparing strings; r=mccr8

Comment 5

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/0e2f7d9f5628