Closed
Bug 714728
Opened 13 years ago
Closed 12 years ago
Remove jsword?
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla12
People
(Reporter: Ms2ger, Assigned: Ms2ger)
References
Details
Attachments
(1 file)
96.02 KB,
patch
|
Waldo
:
review+
|
Details | Diff | Splinter Review |
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•13 years ago
|
||
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•13 years ago
|
||
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•13 years ago
|
||
Yes!
Assignee | ||
Comment 4•13 years ago
|
||
Comment 5•13 years ago
|
||
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?
Attachment #585720 -
Flags: review?(jwalden+bmo) → review+
Assignee | ||
Comment 6•12 years ago
|
||
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.
Assignee | ||
Comment 7•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2f310f456107
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
Comment 8•12 years ago
|
||
Ms2ger: JSSize, JSPtrDiff and JSUptrDiff are some additional easy removals if you're in the mood.
You need to log in
before you can comment on or make changes to this bug.
Description
•