Last Comment Bug 725550 - Remove jsdouble
: Remove jsdouble
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86_64 Windows 7
: -- normal (vote)
: mozilla13
Assigned To: David Mandelin [:dmandelin]
:
: Jason Orendorff [:jorendorff]
Mentors:
: 666402 (view as bug list)
Depends on:
Blocks: 725548
  Show dependency treegraph
 
Reported: 2012-02-08 18:41 PST by David Mandelin [:dmandelin]
Modified: 2012-04-28 14:07 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (169.16 KB, patch)
2012-02-08 18:48 PST, David Mandelin [:dmandelin]
no flags Details | Diff | Splinter Review
Python script for s/jsdouble/double (194 bytes, text/plain)
2012-02-08 19:11 PST, David Mandelin [:dmandelin]
no flags Details
Patch v2 (fix codgen.py and qsgen.py also) (171.06 KB, patch)
2012-02-08 19:20 PST, David Mandelin [:dmandelin]
jwalden+bmo: review+
Details | Diff | Splinter Review

Description David Mandelin [:dmandelin] 2012-02-08 18:41:57 PST

    
Comment 1 David Mandelin [:dmandelin] 2012-02-08 18:48:10 PST
Created attachment 595611 [details] [diff] [review]
Patch
Comment 2 David Mandelin [:dmandelin] 2012-02-08 19:11:12 PST
Created attachment 595616 [details]
Python script for s/jsdouble/double

The script depends on the mungefile module attached to the metabug. I ran it like this:

 python tempmunge.py `find . -name '*.h' -o -name '*.cpp' | xargs grep -l jsdouble`
Comment 3 David Mandelin [:dmandelin] 2012-02-08 19:11:51 PST
(In reply to David Mandelin from comment #2)
> Created attachment 595616 [details]
> Python script for s/jsdouble/double
> 
> The script depends on the mungefile module attached to the metabug. I ran it
> like this:
> 
>  python tempmunge.py `find . -name '*.h' -o -name '*.cpp' | xargs grep -l
> jsdouble`

Actually like this:

 python tempmunge.py `find . -name '*.h' -o -name '*.cpp' -o -name '*.c' | xargs grep -l jsdouble`
Comment 4 David Mandelin [:dmandelin] 2012-02-08 19:17:42 PST
Comment on attachment 595611 [details] [diff] [review]
Patch

That patch doesn't work--the quickstub generator still produces some jsdoubles. Guess I'd better check Python files too.
Comment 5 David Mandelin [:dmandelin] 2012-02-08 19:20:13 PST
Created attachment 595617 [details] [diff] [review]
Patch v2 (fix codgen.py and qsgen.py also)
Comment 6 Jeff Walden [:Waldo] (remove +bmo to email) 2012-02-15 15:27:31 PST
Comment on attachment 595617 [details] [diff] [review]
Patch v2 (fix codgen.py and qsgen.py also)

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

::: js/src/ctypes/CTypes.cpp
@@ +1663,5 @@
>    case TYPE_##name: {                                                          \
>      type value = *static_cast<type*>(data);                                    \
>      if (sizeof(type) < 4)                                                      \
>        *result = INT_TO_JSVAL(jsint(value));                                    \
> +    else if (!JS_NewNumberValue(cx, double(value), result))                  \

Correct the alignment here -- probably do a search for all lines changed that end in a \ to make sure you get everything.

::: js/src/frontend/ParseNode.h
@@ +618,5 @@
>          struct {                        /* lexical dependencies + sub-tree */
>              AtomDefnMapPtr   defnMap;
>              ParseNode        *tree;     /* sub-tree containing name uses */
>          } nameset;
> +        double        dval;           /* aligned numeric literal value */

Realign this.

::: js/src/frontend/TokenStream.h
@@ +321,5 @@
>          struct {                        /* pair for <?target data?> XML PI */
>              PropertyName *target;       /* non-empty */
>              JSAtom       *data;         /* maybe empty, never null */
>          } xmlpi;
> +        double        number;         /* floating point number */

Realign.

::: js/src/jsapi.h
@@ +2129,5 @@
>   *   i      int32_t         ECMA int32_t
>   *   u      uint32_t        ECMA uint32_t
>   *   j      int32_t         Rounded int32_t (coordinate)
> + *   d      double        IEEE double
> + *   I      double        Integral IEEE double

Realign these.

@@ +3554,5 @@
>  extern JS_PUBLIC_API(void)
>  JS_FinalizeStub(JSContext *cx, JSObject *obj);
>  
>  struct JSConstDoubleSpec {
> +    double        dval;

Realign.

::: js/src/jstypedarray.cpp
@@ +984,5 @@
>      uint8_clamped(uint32_t x)   { *this = x; }
>      uint8_clamped(int8_t x)     { *this = x; }
>      uint8_clamped(int16_t x)    { *this = x; }
>      uint8_clamped(int32_t x)    { *this = x; }
> +    uint8_clamped(double x) { *this = x; }

Realign.

::: js/xpconnect/src/XPCConvert.cpp
@@ +143,5 @@
>      switch (type.TagPart()) {
>      case nsXPTType::T_I8    : *d = INT_TO_JSVAL(int32_t(*((int8_t*)s)));             break;
>      case nsXPTType::T_I16   : *d = INT_TO_JSVAL(int32_t(*((int16_t*)s)));            break;
>      case nsXPTType::T_I32   : *d = INT_TO_JSVAL(*((int32_t*)s));                     break;
> +    case nsXPTType::T_I64   : *d = DOUBLE_TO_JSVAL(double(*((int64_t*)s)));        break;

Realign.

@@ +148,4 @@
>      case nsXPTType::T_U8    : *d = INT_TO_JSVAL(int32_t(*((uint8_t*)s)));            break;
>      case nsXPTType::T_U16   : *d = INT_TO_JSVAL(int32_t(*((uint16_t*)s)));           break;
>      case nsXPTType::T_U32   : *d = UINT_TO_JSVAL(*((uint32_t*)s));                   break;
> +    case nsXPTType::T_U64   : *d = DOUBLE_TO_JSVAL(double(*((uint64_t*)s)));       break;

Realign.
Comment 7 David Mandelin [:dmandelin] 2012-02-24 14:53:04 PST
Thanks for the review comments!

http://hg.mozilla.org/integration/mozilla-inbound/rev/4fc2f49371a8
Comment 8 Gregory Szorc [:gps] 2012-02-24 15:14:53 PST
This seems to have broken my Jenkins Clang builder: http://jenkins.gregoryszorc.com:9000/job/mozilla-inbound-linux-x64-optimized-llvm-tip/163/console

Hopefully my builder was just being silly. If not, heads up.
Comment 9 Gregory Szorc [:gps] 2012-02-24 15:16:00 PST
Sorry, forgot to paste the actual error:

In the directory  /home/jenkins-slave/workspace/mozilla-inbound-linux-x64-optimized-llvm-tip/obj/js/jsd
The following command failed to execute properly:
/home/jenkins-slave/workspace/llvm/bin/clang -o jsd_scpt.o -c -I../../dist/system_wrappers -include /home/jenkins-slave/workspace/mozilla-inbound-linux-x64-optimized-llvm-tip/config/gcc_hidden.h -DMOZ_GLUE_IN_PROGRAM -DXPCOM_TRANSLATE_NSGM_ENTRY_POINT=1 -DMOZILLA_INTERNAL_API -D_IMPL_NS_COM -DEXPORT_XPT_API -DEXPORT_XPTC_API -D_IMPL_NS_GFX -D_IMPL_NS_WIDGET -DIMPL_XREAPI -DIMPL_NS_NET -DIMPL_THEBES -DSTATIC_EXPORTABLE_JS_API -DOSTYPE="Linux3.0" -DOSARCH=Linux -DEXPORT_JSD_API -I/home/jenkins-slave/workspace/mozilla-inbound-linux-x64-optimized-llvm-tip/js/jsd -I. -I../../dist/include -I../../dist/include/nsprpub -I/home/jenkins-slave/workspace/mozilla-inbound-linux-x64-optimized-llvm-tip/obj/dist/include/nspr -I/home/jenkins-slave/workspace/mozilla-inbound-linux-x64-optimized-llvm-tip/obj/dist/include/nss -fPIC -Qunused-arguments -pedantic -Qunused-arguments -Wall -W -Wno-unused -Wpointer-arith -Wdeclaration-after-statement -W -Wno-long-long -fno-strict-aliasing -pthread -ffunction-sections -fdata-sections -pipe -DNDEBUG -DTRIMMED -g -Os -freorder-blocks -fomit-frame-pointer -Qunused-arguments -include ../../mozilla-config.h -DMOZILLA_CLIENT -MD -MF .deps/jsd_scpt.pp /home/jenkins-slave/workspace/mozilla-inbound-linux-x64-optimized-llvm-tip/js/jsd/jsd_scpt.c
make[5]: *** [jsd_scpt.o] Error 1
1 warning generated.
/home/jenkins-slave/workspace/mozilla-inbound-linux-x64-optimized-llvm-tip/js/jsd/jsd_step.c:190:25: error: use of undeclared identifier 'jsdouble'; did you mean 'double'?
                        jsdouble delta;
                        ^~~~~~~~
                        double
Comment 10 Marco Bonardo [::mak] 2012-02-24 15:18:20 PST
backed out for failing to compile on all platforms

In file included from ../../dist/include/mozilla/Util.h:48:0,
                 from ../../dist/include/jstypes.h:58,
                 from ../../../js/jsd/jsd.h:71,
                 from ../../../js/jsd/jsdebug.c:42:
../../dist/include/mozilla/Assertions.h:192:54: warning: anonymous variadic macros were introduced in C99
../../dist/include/mozilla/Assertions.h:196:33: warning: anonymous variadic macros were introduced in C99
../../dist/include/mozilla/Assertions.h:204:22: warning: anonymous variadic macros were introduced in C99
../../../js/jsd/jsdebug.c:215:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'JSD_GetScriptMinExecutionTime'
../../../js/jsd/jsdebug.c:222:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'JSD_GetScriptMaxExecutionTime'
../../../js/jsd/jsdebug.c:229:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'JSD_GetScriptTotalExecutionTime'
../../../js/jsd/jsdebug.c:236:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'JSD_GetScriptMinOwnExecutionTime'
../../../js/jsd/jsdebug.c:243:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'JSD_GetScriptMaxOwnExecutionTime'
../../../js/jsd/jsdebug.c:250:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'JSD_GetScriptTotalOwnExecutionTime'
../../../js/jsd/jsdebug.c:1114:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'JSD_GetValueDouble'
make[5]: *** [jsdebug.o] Error 1
Comment 11 David Mandelin [:dmandelin] 2012-02-24 15:23:19 PST
Whoops, missed those .c files in my refresh. Relanded:

http://hg.mozilla.org/integration/mozilla-inbound/rev/d4105352a832
Comment 12 :Ms2ger (⌚ UTC+1/+2) 2012-02-25 02:07:15 PST
*** Bug 666402 has been marked as a duplicate of this bug. ***
Comment 13 Marco Bonardo [::mak] 2012-02-25 02:30:20 PST
https://hg.mozilla.org/mozilla-central/rev/d4105352a832
Comment 14 Jeff Walden [:Waldo] (remove +bmo to email) 2012-04-28 14:07:09 PDT
Noted in release notes:

https://developer.mozilla.org/en/SpiderMonkey/1.8.8

Note You need to log in before you can comment on or make changes to this bug.