714728 - Remove jsword?

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

jsword and jsuword are just typedefs for intptr_t and uintptr_t, which are already used in js/src. Should we get rid of the indirection?

Comment 1

[Jeff Walden [:Waldo]]

I'm in favor.  I can never remember if jsword is size_t or intptr_t anyway, and I can't be the only one, and every mistake there just makes for more pain for those crazy S/390 people.  :-)

Comment 2

[Luke Wagner [:luke]]

While you're on the warpath, can we remove jsint, jsdouble, and every typedef in jstypes.h (except JSBool) and fold the remainder into public/Utility.h?

Comment 3

[David Mandelin [:dmandelin]]

Yes!

Comment 4

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Patch v1

Comment 5

[Jeff Walden [:Waldo]]

Comment on attachment 585720 [details] [diff] [review]
Patch v1

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

::: js/src/jsapi.cpp
@@ +3008,4 @@
>  {
>  #if JS_STACK_GROWTH_DIRECTION > 0
>      if (limitAddr == 0)
> +        limitAddr = uintptr_t(-1);

Make this UINTPTR_MAX.

@@ +3021,5 @@
>  #endif
>  
>  #if JS_STACK_GROWTH_DIRECTION > 0
>      if (stackSize == 0) {
> +        cx->stackLimit = uintptr_t(-1);

Ditto.

::: js/src/jscntxt.cpp
@@ +1423,5 @@
>      localeCallbacks(NULL),
>      resolvingList(NULL),
>      generatingError(false),
>  #if JS_STACK_GROWTH_DIRECTION > 0
> +    stackLimit(uintptr_t(-1)),

Ditto.

::: js/src/jsgc.cpp
@@ +1118,5 @@
>       * With 64-bit jsvals on 32-bit systems, we can optimize a bit by
>       * scanning only the payloads.
>       */
>      JS_ASSERT(begin <= end);
> +    for (const uintptr_t *i = begin; i != end; i += sizeof(Value)/sizeof(uintptr_t))

Add spaces around / while you're changing this.

And if I'm pointing out something to change in this line, I might as well request that you change |i != end| to |i < end|.  Trying to avoid potential bad consequences (should something go awry) by making the loop-exit condition still-correct yet broader may be spitting in the wind, when the code in question is part of a garbage collector.  Even still, at the worst it sets a good example.

::: js/src/jslock.cpp
@@ +76,3 @@
>  
>  #if defined(_MSC_VER) && defined(_M_IX86)
>  #pragma warning( disable : 4035 )

I see in passing that this pragma's never undone, so any function in this file that doesn't return a value won't be warned about.  Could you file a bug to use this pragma, and then revert it, in more narrowly-scoped locations?

Comment 6

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 585720 [details] [diff] [review]
Patch v1

>--- a/js/src/jsgc.cpp
>+++ b/js/src/jsgc.cpp
>-MarkRangeConservatively(JSTracer *trc, const jsuword *begin, const jsuword *end)
>+MarkRangeConservatively(JSTracer *trc, const uintptr_t *begin, const uintptr_t *end)
> {
>     JS_ASSERT(begin <= end);
>-    for (const jsuword *i = begin; i != end; ++i)
>+    for (const uintptr_t *i = begin; i != end; ++i)

And if you're requesting that, I might as well make this one consistent.

Comment 7

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/2f310f456107

Comment 8

[Nicholas Nethercote [inactive]]

Ms2ger:  JSSize, JSPtrDiff and JSUptrDiff are some additional easy removals if you're in the mood.