1440610 - ArenaLists::placeholder could have a better name

Status: RESOLVED FIXED

(Core :: JavaScript: GC, enhancement, P3)

Description

[Jon Coppeard (:jonco) (PTO until 14th September)]

This is a sentinel object used to indicate an empty free span.  So it should probably be called 'emptySentinel' or something like that.

https://searchfox.org/mozilla-central/search?q=symbol:_ZN2js2gc10ArenaLists11placeholderE&redirect=false

Comment 1

[bobslept]

Attachment: 1440610-rename-placeholder.patch

I hope this patch is correct. I have renamed placeholder to emptySentinel. It builds and runs alright. Do I need to run a specific test suite to be totally sure?

Comment 2

[Eric Rahm [:erahm]]

Comment on attachment 8975282 [details] [diff] [review]
1440610-rename-placeholder.patch

Thanks for the patch! Probably running the jsapi-tests would be good enough, |./mach jsapi-tests|. Lets go ahead and ask Jon for a review.

Comment 3

[Jon Coppeard (:jonco) (PTO until 14th September)]

Comment on attachment 8975282 [details] [diff] [review]
1440610-rename-placeholder.patch

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

Looks good to me!

Comment 4

[bobslept]

Well that is great news! If I'm right, I now need to ask you to push it to the try server for testing?
After successful testing, is it me or the bug owner that needs to add the checkin-needed keywords?

Comment 5

[Eric Rahm [:erahm]]

(In reply to bobslept from comment #4)
> Well that is great news! If I'm right, I now need to ask you to push it to
> the try server for testing?
> After successful testing, is it me or the bug owner that needs to add the
> checkin-needed keywords?

Once we have a good try build you can add 'checkin-needed'. Before that can you update the patch title to include "r=jonco" at the end? Go ahead adn mark the updated patch r+ as Jon already approved it.

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b5f390d6e15af92c2eb1f781c552197712fbd26c

Comment 6

[Emanuel Hoogeveen [:ehoogeveen]]

bobslept might not have the editbug privileges needed to do either of those things. Just let us know and we can do it for you.

Comment 7

[Eric Rahm [:erahm]]

(In reply to Emanuel Hoogeveen [:ehoogeveen] from comment #6)
> bobslept might not have the editbug privileges needed to do either of those
> things. Just let us know and we can do it for you.

Ah good point, try looks good. I'll get this landed. Thank you for your contribution bobslept!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8e2debb29096
Rename ArenaLists::placeholder to ArenaLists::emptySentinel. r=jonco

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/8e2debb29096