There are 2 places that defines flag constants. https://dxr.mozilla.org/mozilla-central/rev/39dffbba764210b25bfc1e749b4f16db77fa0d46/js/xpconnect/idl/nsIScriptError.idl#22 > /** pseudo-flag for default case */ > const unsigned long errorFlag = 0x0; > > /** message is warning */ > const unsigned long warningFlag = 0x1; > > /** exception was thrown for this case - exception-aware hosts can ignore */ > const unsigned long exceptionFlag = 0x2; > > // XXX check how strict is implemented these days. > /** error or warning is due to strict option */ > const unsigned long strictFlag = 0x4; https://dxr.mozilla.org/mozilla-central/rev/39dffbba764210b25bfc1e749b4f16db77fa0d46/js/src/jsapi.h#5190 > #define JSREPORT_ERROR 0x0 /* pseudo-flag for default case */ > #define JSREPORT_WARNING 0x1 /* reported via JS_ReportWarning */ > #define JSREPORT_EXCEPTION 0x2 /* exception was thrown */ > #define JSREPORT_STRICT 0x4 /* error or warning due to strict option */ We should at lease assert they're same.

Added assertion for flags. I'm not sure how to handle nsIScriptError.infoFlag there, as it's used only outside of SpiderMonkey. Maybe we could define JSREPORT_USER or something in jsapi.h to avoid future conflict?

Comment on attachment 8767528 [details] [diff] [review] Assert nsIScriptError.*Flags constants match to JSREPORT_*. Review of attachment 8767528 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me, but forwarding review to Ben Turner, who owns this code.

Comment on attachment 8767528 [details] [diff] [review] Assert nsIScriptError.*Flags constants match to JSREPORT_*. Andrea, would you mind reviewing this?

Comment on attachment 8767528 [details] [diff] [review] Assert nsIScriptError.*Flags constants match to JSREPORT_*. Review of attachment 8767528 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/workers/WorkerPrivate.cpp @@ +5920,5 @@ > + static_assert(nsIScriptError::errorFlag == JSREPORT_ERROR && > + nsIScriptError::warningFlag == JSREPORT_WARNING && > + nsIScriptError::exceptionFlag == JSREPORT_EXCEPTION && > + nsIScriptError::strictFlag == JSREPORT_STRICT, > + "flags should be consistent"); This static_assert should be moved into ./js/xpconnect/src/nsScriptError.cpp Plus you should add nsIScriptError::infoFlag == JSREPORT_STRICT_MODE_ERROR

Thank you all :) (In reply to Andrea Marchesini (:baku) from comment #4) > Plus you should add nsIScriptError::infoFlag == JSREPORT_STRICT_MODE_ERROR I don't think they have same meaning. Those 2 constants are just conflicting. There is no place that nsIScriptError::infoFlag flows into the code that checks JSREPORT_STRICT_MODE_ERROR, and I'm about to remove JSREPORT_STRICT_MODE_ERROR constant in bug 1283058 because it's not used anywhere. Maybe we'd better adding another constant in jsapi.h for user-defined flag(s). will prepare a patch for that.

This patch is based on patches in bug 1283058, that removes unused and conflicting JSREPORT_STRICT_MODE_ERROR (0x08). As long as the flag nsIScriptError::infoFlag (0x08) is used only outside of SpiderMonkey, I think we should define it as user-defined flag, to avoid future conflict.

Moved js/xpconnect/src/nsScriptError.cpp and added `nsIScriptError::infoFlag == JSREPORT_USER_1`.

> There is no place that nsIScriptError::infoFlag flows into the code that > checks JSREPORT_STRICT_MODE_ERROR, > and I'm about to remove JSREPORT_STRICT_MODE_ERROR constant in bug 1283058 > because it's not used anywhere. Thanks for doing this!

Comment on attachment 8770351 [details] [diff] [review] Part 1: Add JSREPORT_USER_1 for user-defined flag in JSErrorReport.flags. Review of attachment 8770351 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsapi.h @@ +5189,5 @@ > #define JSREPORT_WARNING 0x1 /* reported via JS_ReportWarning */ > #define JSREPORT_EXCEPTION 0x2 /* exception was thrown */ > #define JSREPORT_STRICT 0x4 /* error or warning due to strict option */ > > +#define JSREPORT_USER_1 0x8 /* user-defined flag */ Guh, this is awful. Let's try to make this thing temporary, hopefully?

Thank you for reviewing :) So, to make this temporary, we could do either following: 1. Make it JSREPORT_INFO and support it in SpiderMonkey side (not sure how it works in SpiderMonkey tho..) 2. Stop using nsIScriptError::infoFlag and store it in different member Will check how nsIScriptError::infoFlag is used

https://hg.mozilla.org/integration/mozilla-inbound/rev/50f6751e3ef22450b0ea457363343d95a331b09f Bug 1284099 - Part 1: Add JSREPORT_USER_1 for user-defined flag in JSErrorReport.flags. r=jwalden https://hg.mozilla.org/integration/mozilla-inbound/rev/50c2b9e5f491bc89186235e52c6c95a1d495df11 Bug 1284099 - Part 2: Assert nsIScriptError.*Flags constants match to JSREPORT_*. r=baku

https://hg.mozilla.org/mozilla-central/rev/50f6751e3ef2 https://hg.mozilla.org/mozilla-central/rev/50c2b9e5f491

(In reply to Tooru Fujisawa [:arai] from comment #10) > So, to make this temporary, we could do either following: > 1. Make it JSREPORT_INFO and support it in SpiderMonkey side (not sure how > it works in SpiderMonkey tho..) > 2. Stop using nsIScriptError::infoFlag and store it in different member Longer-term, I'm thinking more about replacing the entire reporting mechanism with something better, that's more focused on creating and throwing exceptions. And that doesn't have this JSREPORT_* thing at all, because the language itself has nothing like it. Shorter-term, I dunno, maybe there's *some* better way to do things here. But I didn't look closely. This is more of an aspirational complaint/request, than one looking for a definite, concrete, near-term result.