Closed Bug 1515170 Opened 5 years ago Closed 5 years ago

Don't use `is` for comparing strings

Categories

(Core :: IPC, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox66 --- fixed

People

(Reporter: Alex_Gaynor, Assigned: Alex_Gaynor)

Details

Attachments

(1 file)

|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").
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.
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.
Keywords: checkin-needed
Pushed by rvandermeulen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0e2f7d9f5628
in the IPDL compiler, don't use 'is' for comparing strings; r=mccr8
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/0e2f7d9f5628
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: