1054334 - Misc code cleanups

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Dan Gohman [:sunfish]]

Attachment: unneeded-semicolons.patch

Attached is a series of miscellaneous code-cleanup patches.

Comment 1

[Dan Gohman [:sunfish]]

Attachment: void-parameters.patch

Comment 2

[Dan Gohman [:sunfish]]

Attachment: return-parens.patch

Comment 3

[Dan Gohman [:sunfish]]

Attachment: static.patch

Comment 4

[Dan Gohman [:sunfish]]

Attachment: const.patch

Comment 5

[Dan Gohman [:sunfish]]

Attachment: const-gnostring.patch

Comment 6

[Nicolas B. Pierron [:nbp]]

Comment on attachment 8473722 [details] [diff] [review]
unneeded-semicolons.patch

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

::: js/src/builtin/SIMD.cpp
@@ -689,5 @@
>      if (args.length() == 2) {
>          if (!IsVectorObject<V>(args[0]) || !args[1].isInt32())
>              return ErrorBadArgs(cx);
>  
> -        Elem *val = TypedObjectMemory<Elem *>(args[0]);;

Hum … I wonder where are the "case" and "esac".

::: js/src/jsapi.h
@@ +1325,1 @@
>  #endif

luke: Any reason to keep this code around inside a "#if 0" ?

Comment 7

[Nicolas B. Pierron [:nbp]]

Comment on attachment 8473722 [details] [diff] [review]
unneeded-semicolons.patch

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

::: js/src/jsapi.h
@@ +1325,1 @@
>  #endif

luke> nbp: heh, no, it looks like some a mistake to leave that in
luke> nbp: actually, it seems better to use MOZ_STACK_CLASS

Comment 8

[Nicolas B. Pierron [:nbp]]

Comment on attachment 8473727 [details] [diff] [review]
const-gnostring.patch

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

::: js/xpconnect/src/XPCJSID.cpp
@@ +25,5 @@
>  
>  nsJSID::nsJSID()
> +    : mID(GetInvalidIID()),
> +      mNumber(const_cast<char *>(gNoString)),
> +      mName(const_cast<char *>(gNoString))

I am not a big fan of the const_cast, as it often hide mistakes.  But I did not spot any location where the content of mNumber / mName is mutated, and if ever it is, this will cause a SEGV as the gNoString would be mapped as read-only.

It might be better to change nsJSID to reflect that even if the pointer are mutable their target is not.

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/e682602319be
https://hg.mozilla.org/mozilla-central/rev/80db0e307d6d
https://hg.mozilla.org/mozilla-central/rev/c9b218c047fa
https://hg.mozilla.org/mozilla-central/rev/b01844af31c9
https://hg.mozilla.org/mozilla-central/rev/3074b8e53bc6
https://hg.mozilla.org/mozilla-central/rev/a14312d5203e