695908 - Use bool in js/src/frontend rather than JSBool

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Comment 1

[Jason Orendorff [:jorendorff]]

Attachment: Part 1 - Boolean literals, v1

Comment 2

[Jason Orendorff [:jorendorff]]

Attachment: Part 1 - Boolean literals, v2

Simple search and replace.

Comment 3

[Jason Orendorff [:jorendorff]]

Attachment: Part 2 - Switch from JSBool to bool

Simple search and replace again, separate only because types can be trickier in C++.

Still, I looked at each case at least briefly and found no trickiness.

Comment 4

[Eddy Bruel [:ejpbruel]]

Comment on attachment 638469 [details] [diff] [review]
Part 1 - Boolean literals, v2

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

Looks good to me. r+ provided it builds.

Comment 5

[Eddy Bruel [:ejpbruel]]

Comment on attachment 638470 [details] [diff] [review]
Part 2 - Switch from JSBool to bool

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

Looks fine to me as well.

Comment 6

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

You know jsbool.h isn't about JSBool, right?

Comment 7

[Jason Orendorff [:jorendorff]]

Ms2ger: Yeah, I'm aware of it. What happened was, during my search-and-replace, I came across that line, and it seemed very unlikely that that file actually needed that #include. Sure enough it compiles fine without it.

https://hg.mozilla.org/integration/mozilla-inbound/rev/b23429ce0d81
https://hg.mozilla.org/integration/mozilla-inbound/rev/1e28b1f771c1

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b23429ce0d81
https://hg.mozilla.org/mozilla-central/rev/1e28b1f771c1

Comment 9

[(no longer active)]

This bug, or something which was merged to mozilla-central in the same push, caused a permanent orange on Linux32 mochitest-chrome for builds without frame pointers (example log: <https://tbpl.mozilla.org/php/getParsedLog.php?id=13227357&tree=Profiling&full=1>).  This is important because framepointers are not enabled on Aurora, so this permaorange will appear there on the next uplift.  I backed out this patch as part of the rest of the js and xpconnect patches in the same merge push.  Please test this patch locally (and push to the try server if needed by taking out the --enable-profiling command from the Linux32 mozconfig) and reland when you make sure that it's not the cause behind the perma-orange.  I apologize in advance for the inconvenience in case this patch is not at fault.

Backout changeset:
https://hg.mozilla.org/mozilla-central/rev/77407e03e22f
https://hg.mozilla.org/mozilla-central/rev/de50598e2329

Comment 10

[(no longer active)]

In order to test whether this patch is at fault, push it to try with --enable-profiling removed from browser/config/mozconfigs/linux32/nightly, and if you get a green Linux Moth run, you're good to go!

And in turn you can reproduce the crash on try with your patch, I'd suggest you keep the possibility of having hit a compiler bug in mind.  :-)

Comment 11

[(no longer active)]

Also, note that these backouts indeed made the Linux32 Moth runs on the profiling branch green again, so we can be fairly certain that one of the backed out patches was at fault.

Comment 12

[Jason Orendorff [:jorendorff]]

Ehsan: great work, and thanks for telling me how to proceed.

Green Linux32 Moth runs with --enable-profiling removed:
  https://tbpl.mozilla.org/?tree=Try&rev=5c33be0ba83d

Relanded, therefore:
  https://hg.mozilla.org/integration/mozilla-inbound/rev/936e436201e0
  https://hg.mozilla.org/integration/mozilla-inbound/rev/8361bdf249cb

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/936e436201e0
https://hg.mozilla.org/mozilla-central/rev/8361bdf249cb