1302582 - Add explicit JS_ReportError*ASCII variants

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Comment 1

[Tooru Fujisawa [:arai]]

Attachment: Add JS_ReportError*ASCII variants.

Added ASCII variants.
they forward to UTF8 variants.

the forwarding part is simple, but the validation part is somewhat tricky,
as we have multiple parameters, and some of them may be non-string for JS_ReportErrorASCII and JS_ReportWarningASCII cases.

maybe we'd better adding actual ASCII variants, that asserts after formatting in ReportErrorVA or ExpandErrorArgumentsVA.
how do you think?

Comment 2

[Jeff Walden [:Waldo]]

(In reply to Tooru Fujisawa [:arai] from comment #1)
> maybe we'd better adding actual ASCII variants, that asserts after
> formatting in ReportErrorVA or ExpandErrorArgumentsVA.

Hmm, it does seem like it'd be worth pushing the asserting into the when-splatting-each-argument code.  Having an extra allocation/possibility to OOM in order to format the entire string, to verify this, is unideal.  I don't believe *technically* there's any reason the formatter callback can't be impure -- and this would add a reason, and IMO not really a super-great one.

So I guess this really wants a JS::detail::Report8Bit(cx, ..., enum { ArgumentsAreASCII, ArgumentsAreUTF8 }) that both would call, I think.  Shouldn't quite be forwarding from the ASCII function to the UTF-8 one, as I was initially thinking.

Comment 3

[Tooru Fujisawa [:arai]]

Attachment: Add JS_ReportError*ASCII variants.

Thanks :)
Changed it to just a copy of other variants, with ASCII, and added assertion.

Comment 4

[Jeff Walden [:Waldo]]

Comment on attachment 8793057 [details] [diff] [review]
Add JS_ReportError*ASCII variants.

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

::: js/src/jscntxt.cpp
@@ +317,5 @@
>      return false;
>  }
>  
> +#ifdef DEBUG
> +static inline void

We probably don't want inline, if all callers ever die.

Comment 5

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d028babfbb85dcb1ba021c78ea2b60042c6c86d8
Bug 1302582 - Add JS_ReportError*ASCII variants. r=jwalden

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d028babfbb85

Comment 7

[Gerry Chang [:gchang]]

Mark 51 as won't fix. If it's worth uplifting to 51, feel free to nominate it.