1472126 - Remove nullptr JSContext* check from NativeIterator::allocateSentinel() because it's never called with a nulllptr

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P3)

Description

[André Bargull [:anba]]

These lines [1] can be simplied to:
---
if (!ni)
    ReportOutOfMemory(maybecx);
---

because the |JSContext*| parameter is never a nullptr. And then the parameter name should be changed from |maybecx| to just |cx|.


[1] https://searchfox.org/mozilla-central/rev/d2966246905102b36ef5221b0e3cbccf7ea15a86/js/src/vm/Iteration.cpp#650-653

Comment 1

[Ashley Hauck [:khyperia]]

Attachment: require non-null context in NativeIterator::allocateSentinel

Comment 2

[Ashley Hauck [:khyperia]]

I believe that the only reference to NativeIterator::allocateSentinel is in ObjectRealm::init [1]. (It's non-null.)


[1] https://searchfox.org/mozilla-central/rev/2466b82b729765fb0a3ab62f812c1a96a7362478/js/src/vm/Realm.cpp#92

Comment 3

[Jan de Mooij [:jandem]]

Comment on attachment 9000083 [details] [diff] [review]
require non-null context in NativeIterator::allocateSentinel

Review of attachment 9000083 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thanks.

If you post a new patch with r=jandem at the end of the commit message and the style nit below addressed, you can set the r+ flag yourself and add the checkin-needed keyword to the bug so sheriffs will land it. Or request r? if you want another review :)

::: js/src/vm/Iteration.cpp
@@ +660,4 @@
>  {
>      NativeIterator* ni = js_new<NativeIterator>();
>      if (!ni) {
> +        ReportOutOfMemory(cx);

Nit: you can remove the {} braces per our coding style [0] (or, maybe better, keep them and add |return nullptr;| on the next line) and we can also rename maybecx to cx in the Iteration.h header file.

https://wiki.mozilla.org/JavaScript:SpiderMonkey:Coding_Style

Comment 4

[Ashley Hauck [:khyperia]]

Attachment: Fix style nit

Comment 5

[Ashley Hauck [:khyperia]]

Attachment: Require non-null context in NativeIterator::allocateSentinel

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bdea655c3190
Require non-null context in NativeIterator::allocateSentinel. r=jandem

Comment 7

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/bdea655c3190