1185104 - jsfriendapi.h misuses MOZ_WARN_UNUSED_RESULT

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Daniel Holbert [:dholbert]]

Quoting jsfriendapi.h:
> 1309     bool init(JSContext* cx, JSString* s) MOZ_WARN_UNUSED_RESULT;
[...]
> 1312     bool initTwoByte(JSContext* cx, JSString* s) MOZ_WARN_UNUSED_RESULT;
http://mxr.mozilla.org/mozilla-central/source/js/src/jsfriendapi.h?rev=198bf962586d#1309

The MOZ_WARN_UNUSED_RESULT annotation should be at the beginning of the function decl, not at the end, according to its documentation in MFBT:
> 319  * Place this attribute at the very beginning of a function definition. For
> 320  * example, write
> 321  *
> 322  *   MOZ_WARN_UNUSED_RESULT int foo();

http://mxr.mozilla.org/mozilla-central/source/mfbt/Attributes.h?rev=6f25cf876490#319

(I don't actually know if this recommendation is purely stylistic, or if it has functional implications -- i.e. not sure if the annotation has zero effect or weird interactions when incorrectly placed at the end of a decl, or if it still works just fine. Regardless, we should follow the documentation & be consistent with other usages.)

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: fix v1

Tagging tromey for review since he added these decls (in bug 1168593).

Comment 2

[Tom Tromey :tromey]

(In reply to Daniel Holbert [:dholbert] from comment #0)

> (I don't actually know if this recommendation is purely stylistic, or if it
> has functional implications -- i.e. not sure if the annotation has zero
> effect or weird interactions when incorrectly placed at the end of a decl,
> or if it still works just fine. Regardless, we should follow the
> documentation & be consistent with other usages.)

Totally agreed.  Sorry I missed this first time around.
IIRC I did make sure the error triggered before I added the fix.

Comment 3

[Tom Tromey :tromey]

Comment on attachment 8635526 [details] [diff] [review]
fix v1

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

Thanks.

Comment 4

[Daniel Holbert [:dholbert]]

No worries, & thanks for the quick review turnaround! Landed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0a1f292ba8cc

Comment 5

[Carsten Book [:Tomcat]]

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