Closed Bug 1450085 Opened 2 years ago Closed 2 years ago

Use default arguments for ReportValueError instead of macros and more cleanup


(Core :: JavaScript Engine, enhancement, P3)




Tracking Status
firefox61 --- wontfix
firefox62 --- fixed


(Reporter: anba, Assigned: anba)



(3 files)

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].

Priority: -- → P3
Replaces the |ReportValueError| macros with a proper function and removes the remaining C89-bits from js::ReportValueErrorFlags(...).
Assignee: nobody → andrebargull
Attachment #8981400 - Flags: review?(tcampbell)
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.
Attachment #8981401 - Flags: review?(tcampbell)
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.
Attachment #8981402 - Flags: review?(tcampbell)
Attachment #8981400 - Flags: review?(tcampbell) → review+
Attachment #8981401 - Flags: review?(tcampbell) → review+
Attachment #8981402 - Flags: review?(tcampbell) → review+
Pushed by
Part 1: Change ReportValueError to a proper function instead of a macro. r=tcampbell
Part 2: Replace ReportValueErrorFlags with ReportValueError where applicable. r=tcampbell
Part 3: Call error-functions instead of passing JSREPORT_ERROR flag. r=tcampbell
Keywords: checkin-needed
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in before you can comment on or make changes to this bug.