1511401 - Make some improvements to our reporting of insufficient arguments

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P3)

Description

[Boris Zbarsky [:bzbarsky]]

Our current error for not enough arguments for JS stuff looks like this:

  TypeError: FUNC requires more than N arguments

and for DOM stuff:

  TypeError: Not enough arguments to FUNC.

I haven't found a way to trigger the "JS stuff" case in Chrome so far, but their DOM case is:

  TypeError: Failed to execute FUNC on OBJTYPE: N arguments required, but only M present.

I'd like to do two things: add reporting of how many args were actually passed to our "JS stuff" and make the DOM use the "JS stuff" mechanism to get the nicer reporting.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Use requireAtLeast more in the JS engine

Looks like these uses might predate requireAtLeast existing.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Have JSMGS_MORE_ARGS_NEEDED say how many args it actually got

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: part 3. Inline the fast (no error) path of requireAtLeast

We don't want to pay the cost of a function call here in DOM bindings.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: part 4. Use CallArgs::requireAtLeast in the DOM

Comment 5

[Nicolas B. Pierron [:nbp] {away}]

Comment on attachment 9029104 [details] [diff] [review]
part 1.  Use requireAtLeast more in the JS engine

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

I had doubts about converting the  non-zero "if (!=)" cases, but they all appears in js.cpp which does not matter spec-wise.

Comment 6

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 9029107 [details] [diff] [review]
part 4.  Use CallArgs::requireAtLeast in the DOM

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

Sorry this got stuck in my queue.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e37a2cdffa1a
part 1.  Use requireAtLeast more in the JS engine.  r=nbp
https://hg.mozilla.org/integration/mozilla-inbound/rev/e381de28cdca
part 2.  Have JSMGS_MORE_ARGS_NEEDED say how many args it actually got.  r=nbp
https://hg.mozilla.org/integration/mozilla-inbound/rev/696e7b4f7535
part 3.  Inline the fast (no error) path of requireAtLeast.  r=nbp
https://hg.mozilla.org/integration/mozilla-inbound/rev/39b04fe4eae8
part 4.  Use CallArgs::requireAtLeast in the DOM.  r=nbp,qdot

Comment 8

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/e37a2cdffa1a
https://hg.mozilla.org/mozilla-central/rev/e381de28cdca
https://hg.mozilla.org/mozilla-central/rev/696e7b4f7535
https://hg.mozilla.org/mozilla-central/rev/39b04fe4eae8

Comment 9

[Chris Mills [:cmills]]

Looks like Boris already updated the relevant MDN error page. Cheers!

I haven't added a note to the Fx66 rel notes, as I don't think it's really worth it for error message changes.