Last Comment Bug 696232 - Use an enum to pass flag value to InitArrayElements
: Use an enum to pass flag value to InitArrayElements
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- trivial (vote)
: mozilla11
Assigned To: Terrence Cole [:terrence]
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-20 15:09 PDT by Terrence Cole [:terrence]
Modified: 2012-02-01 13:59 PST (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1: Trivial implementation. (3.49 KB, patch)
2011-10-27 09:36 PDT, Terrence Cole [:terrence]
no flags Details | Diff | Splinter Review
v2: Also fix the return type. (4.95 KB, patch)
2011-11-08 14:57 PST, Terrence Cole [:terrence]
jwalden+bmo: review+
Details | Diff | Splinter Review
v3: With review feedback. (4.87 KB, patch)
2011-11-09 18:37 PST, Terrence Cole [:terrence]
no flags Details | Diff | Splinter Review
v4: because I can't read. (4.87 KB, patch)
2011-11-17 17:01 PST, Terrence Cole [:terrence]
no flags Details | Diff | Splinter Review

Description Terrence Cole [:terrence] 2011-10-20 15:09:26 PDT
Right now we pass a bool.  This should be an enum, per modern SM style.
Comment 1 Terrence Cole [:terrence] 2011-10-27 09:36:38 PDT
Created attachment 570007 [details] [diff] [review]
v1: Trivial implementation.

If you have clearer/better/more-canonical names, please let me know.  Otherwise this is absurdly trivial.
Comment 2 Terrence Cole [:terrence] 2011-10-27 17:10:44 PDT
Comment on attachment 570007 [details] [diff] [review]
v1: Trivial implementation.

Scratch that.  I forgot to change the rest of the signature.
Comment 3 Terrence Cole [:terrence] 2011-11-08 14:57:27 PST
Created attachment 573021 [details] [diff] [review]
v2: Also fix the return type.

This should be trivial to review.
Comment 4 Jeff Walden [:Waldo] (remove +bmo to email) 2011-11-09 18:21:35 PST
Comment on attachment 573021 [details] [diff] [review]
v2: Also fix the return type.

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

Slight nits, looks fine otherwise.

::: js/src/jsarray.cpp
@@ +1764,5 @@
>  
> +enum ShouldUpdateTypes
> +{
> +    DoUpdateTypes,
> +    DoNotUpdateTypes

I'd name these UpdateTypes and DontUpdateTypes.  And if you initialized UpdateTypes to true and DontUpdateTypes to false, you probably could get away without changing any of the uses of variables of these types, either -- only the places where the enum is passed.  I've seen Luke do this at least one other place before.
Comment 5 Terrence Cole [:terrence] 2011-11-09 18:37:54 PST
Created attachment 573405 [details] [diff] [review]
v3: With review feedback.

Will push to try once it's unbroken.
Comment 6 Terrence Cole [:terrence] 2011-11-17 17:01:23 PST
Created attachment 575337 [details] [diff] [review]
v4: because I can't read.
Comment 7 Terrence Cole [:terrence] 2011-11-17 17:05:12 PST
https://tbpl.mozilla.org/?tree=Try&rev=3ccdad371c1e
Comment 8 Terrence Cole [:terrence] 2011-11-18 11:28:45 PST
Prior try run looks like it got busted by unrelated problems on macos, and this needed a merge this morning: retrying.

https://tbpl.mozilla.org/?tree=Try&rev=91d9b57fd410
Comment 9 Terrence Cole [:terrence] 2011-11-22 11:06:14 PST
https://tbpl.mozilla.org/?tree=Try&rev=00447b677920
Comment 10 Terrence Cole [:terrence] 2011-11-22 14:28:19 PST
http://hg.mozilla.org/integration/mozilla-inbound/rev/646f4a4d58ea
Comment 11 Ed Morley [:emorley] 2011-11-23 04:25:30 PST
https://hg.mozilla.org/mozilla-central/rev/646f4a4d58ea

Note You need to log in before you can comment on or make changes to this bug.