Closed Bug 958803 Opened 6 years ago Closed 6 years ago

non-ion build failure: ForkJoin.cpp:58:54: error: too few arguments

Categories

(Core :: JavaScript Engine, defect)

PowerPC
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla29

People

(Reporter: stevensn, Assigned: stevensn)

References

Details

Attachments

(1 file, 2 obsolete files)

Bug 91963 seems to have broken non-ion builds 

/js/src/vm/ForkJoin.cpp: In function ‘bool js::ForkJoin(JSContext*, JS::CallArgs&)’:
js/src/vm/ForkJoin.cpp:58:54: error: too few arguments to function ‘bool ExecuteSequentially(JSContext*, JS::HandleValue, bool*, uint16_t, uint16_t)’
js/src/vm/ForkJoin.cpp:49:1: note: declared here
js/src/vm/ForkJoin.cpp:59:1: error: control reaches end of non-void function [-Werror=return-type]

An new argument was added to ExecuteSequentially but the non-ion non-thread path wasn't updated
Correction/typo,  Bug 919638 seems to have caused this
Blocks: 919638
No longer blocks: 91953
Attached patch 958803.diff (obsolete) — Splinter Review
Attachment #8358766 - Flags: review?(shu)
Comment on attachment 8358766 [details] [diff] [review]
958803.diff

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

Thanks for catching this. See comment below.

::: js/src/vm/ForkJoin.cpp
@@ +54,5 @@
>  js::ForkJoin(JSContext *cx, CallArgs &args)
>  {
>      RootedValue argZero(cx, args[0]);
>      bool complete = false; // since warmup is false, will always complete
> +    return ExecuteSequentially(cx, argZero, &complete,0,1);

Passing 0, 1 as the slice bounds is actually not correct.

You need to pass args[2] as numSlices, so something like:

> uint32_t numSlices = args[2].toInt32();
> return ExecuteSequentially(cx, argZero, &complete, 0, numSlices);
Attachment #8358766 - Flags: review?(shu)
Attached patch 958803.diff (obsolete) — Splinter Review
Patch updated to extract the number of slices from args2
Attachment #8358766 - Attachment is obsolete: true
Attachment #8358824 - Flags: review?(shu)
Comment on attachment 8358824 [details] [diff] [review]
958803.diff

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

r=me with nit addressed.

::: js/src/vm/ForkJoin.cpp
@@ +55,5 @@
>  {
>      RootedValue argZero(cx, args[0]);
>      bool complete = false; // since warmup is false, will always complete
> +    uint32_t numSlices = args[2].toInt32();
> +    return ExecuteSequentially(cx, argZero, &complete,0,numSlices);

Nit: spaces after , in the argument list.
Attachment #8358824 - Flags: review?(shu) → review+
Attached patch 958803.diffSplinter Review
Patch updated to fix spacing
Attachment #8358824 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/9ce81e47d4ab
Assignee: nobody → steve
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in before you can comment on or make changes to this bug.