Closed
Bug 1383174
Opened 7 years ago
Closed 7 years ago
js::GetNonSyntacticGlobalThis() has an unused return at the end
Categories
(Core :: JavaScript Engine, enhancement)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: mccr8, Assigned: mccr8)
References
Details
Attachments
(1 file)
This method iterates over the environment chain, looking for whatever. However, if it fails to find anything, it does not set the return value, but ends up returning true. That seems bad.
Assignee | ||
Comment 1•7 years ago
|
||
Jan, it looks like you wrote this code. Am I right, or am I missing something? Thanks.
Flags: needinfo?(jdemooij)
Comment 2•7 years ago
|
||
Note the while (true) {}, the loop is guaranteed to find something - we should never reach the |return true;| after it. Not sure why this returns bool instead of void. Probably because it's called directly from JIT code and the machinery for that doesn't support void yet.
Flags: needinfo?(jdemooij)
Assignee | ||
Comment 3•7 years ago
|
||
Ah, great. I figured that might be the case. I'll add a comment or crash or something in that case.
Assignee: nobody → continuation
Assignee | ||
Comment 4•7 years ago
|
||
Oh, right, there's no break in the while(true) loop!
Assignee | ||
Updated•7 years ago
|
Summary: js::GetNonSyntacticGlobalThis() should return false on failure → js::GetNonSyntacticGlobalThis() has an unused return at the end
Comment hidden (mozreview-request) |
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8889594 [details] Bug 1383174 - Make GetNonSyntacticGlobalThis return void. https://reviewboard.mozilla.org/r/160622/#review166144 Nice
Attachment #8889594 -
Flags: review?(jdemooij) → review+
Pushed by amccreight@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a3a843f6334d Make GetNonSyntacticGlobalThis return void. r=jandem
Comment 8•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/a3a843f6334d
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in
before you can comment on or make changes to this bug.
Description
•