1487136 - Make OwningCompileOptions::setFile(JSContext*, const char*) MOZ_MUST_USE

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Comment 1

[Jeff Walden [:Waldo]]

Attachment: Patch

You seem to be the most vaguely recent person touching ChromeScriptLoader.cpp who isn't just a SpiderMonkey person rearranging deckchairs.  :-)

Comment 2

[Kris Maglione [:kmag]]

Comment on attachment 9004935 [details] [diff] [review]
Patch

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

Mandatory Init() methods make me nervous. I generally prefer classes that need them to have private constructors and static Create() methods that return a Result or take an ErrorResult argument. In this case, though, just having Start() setup the compile options might be a better option, though, since it's already fallible.

::: js/xpconnect/loader/ChromeScriptLoader.cpp
@@ +52,5 @@
>        , mScriptLength(0)
> +    {}
> +
> +    MOZ_MUST_USE nsresult Init(JSContext* aCx,
> +                          const nsACString& aURL,

Nit: Please align arguments after opening paren.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0e1dd163d614
Make OwningCompileOptions::setFile(JSContext*, const char*) MOZ_MUST_USE.  r=kmag on fixing the one user requiring any changes, r=me on the obviously-justified attribute-addition

Comment 4

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/0e1dd163d614