1450085 - Use default arguments for ReportValueError instead of macros and more cleanup

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P3)

Description

[André Bargull [:anba]]

The |ReportValueError| macros [1] could be changed to proper C++ functions with defaults arguments. For example like so:

---
extern void
ReportValueError(JSContext* cx, unsigned flags, const unsigned errorNumber,
                 int spindex, HandleValue v, HandleString fallback = nullptr,
                 const char* arg1 = nullptr, const char* arg2 = nullptr)
{
    mozilla::Unused << ReportValueErrorFlags(cx, JSREPORT_ERROR, errorNumber, spindex, v, fallback, arg1, arg2);
}
---

After that
- Callers to ReportValueErrorFlags which use |flags = JSREPORT_ERROR|, like [2], could then be changed to call ReportValueError.
- An explicit |nullptr| argument for the |fallback| parameter can then be removed, for example in [3].
- And completely unused functions can be deleted, like [4].


[1] https://searchfox.org/mozilla-central/rev/7e663b9fa578d425684ce2560e5fa2464f504b34/js/src/vm/JSContext.h#1091-1101
[2] https://searchfox.org/mozilla-central/rev/7e663b9fa578d425684ce2560e5fa2464f504b34/js/src/builtin/ReflectParse.cpp#291-292
[3] https://searchfox.org/mozilla-central/rev/7e663b9fa578d425684ce2560e5fa2464f504b34/js/src/builtin/Promise.cpp#900
[4] https://searchfox.org/mozilla-central/rev/7e663b9fa578d425684ce2560e5fa2464f504b34/js/src/vm/JSObject.cpp#2584-2609

Comment 1

[André Bargull [:anba]]

Attachment: bug1450085-part1-report-value-error-function.patch

Replaces the |ReportValueError| macros with a proper function and removes the remaining C89-bits from js::ReportValueErrorFlags(...).

Comment 2

[André Bargull [:anba]]

Attachment: bug1450085-part2-report-value-error-flags.patch

Replaces calls to ReportValueErrorFlags(...) with ReportValueError(...) when the error-flags arguments is JSREPORT_ERROR. This matches how the JSAPI error API "normally" works. (Except when it doesn't -> Part 3!)


JSObject::report{ReadOnly,NotConfigurable,NotExtensible} are no longer used, so instead of updating them to use ReportValueError(...), we can instead remove them completely.

Comment 3

[André Bargull [:anba]]

Attachment: bug1450085-part3-report-error-flag.patch

Replaces call to JS_ReportErrorFlagsAndNumber{ASCII,Latin1,UTF8,UC}(...) with JS_ReportErrorNumber{ASCII,Latin1,UTF8,UC}(...) when the error-flags argument is JSREPORT_ERROR.

I've also removed the unused |fallback| argument from js::ReportIsNullOrUndefined(...) and changed the function to use a void-return to match the standard error API pattern.

Comment 4

[André Bargull [:anba]]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7e1e6b704e20decb2bdad486f5b79058ce42b422

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3e478d973ae2
Part 1: Change ReportValueError to a proper function instead of a macro. r=tcampbell
https://hg.mozilla.org/integration/mozilla-inbound/rev/9529d02de634
Part 2: Replace ReportValueErrorFlags with ReportValueError where applicable. r=tcampbell
https://hg.mozilla.org/integration/mozilla-inbound/rev/a23ae192f340
Part 3: Call error-functions instead of passing JSREPORT_ERROR flag. r=tcampbell

Comment 6

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/3e478d973ae2
https://hg.mozilla.org/mozilla-central/rev/9529d02de634
https://hg.mozilla.org/mozilla-central/rev/a23ae192f340