Closed Bug 59195 Opened 24 years ago Closed 15 years ago

Engine changes to compile with CW & Borland for Win32

Categories

(Core :: JavaScript Engine, defect, P3)

x86
Windows 98
defect

Tracking

()

RESOLVED INCOMPLETE
Future

People

(Reporter: shantirao, Assigned: drobec)

References

Details

(Keywords: js1.5)

Attachments

(4 files)

A few problems with compilers that can be fixed:

strdup() and fileno() are not defined or are improperly handled by the 
compiler. So, add some code to js.c:

#if defined( __MWERKS__) && defined(WIN32)

char *strdup(const char *str)
{
    char *copy = (char *) malloc(strlen(str)+1);
    if (copy)
        strcpy(copy, str);
    return copy;
}
int fileno(FILE *stream)
{
 if (stream == stdin) return 0;
 if (stream == stdout) return 1;
 if (stream == stderr) return 2;
 return -1;
}
#endif

Both Borland's free compiler and CodeWarrior use __declspec instead of 
_declspec. This shows up in jstypes.h's definition of JS_EXTERN_API. Add, at 
the beginning of the WIN32 section,

#if defined(__BORLANDC__) || defined(__MWERKS__)
#define _declspec __declspec
#endif

I notice that _WIN32, WIN32, and _WINDOWS are used interchangably in js. This 
only works with Visual C. Borland defines _Windows and __WIN32__, and 
MetroWerks defines __MWERKS__ and __INTEL__ (which is ambiguous with Linux). I 
haven't tested VisualAge yet, but it appears to be discontinued anyway. 

A big problem with the Borland compiler is that it doesn't like syntax of the 
form

a = b << c | d << e

It really needs parentheses, because it binds << as tightly as |.

a = (b << c) | (d << e)

This occurrs a couple of times in jsdtoa.c, jsdhash.c, and jslong.c.
Confirming; cc'ing Brendan and mccabe-
Status: UNCONFIRMED → NEW
Ever confirmed: true
"This occurrs a couple of times in jsdtoa.c, jsdhash.c, and jslong.c."

I see no such expressions in jsdhash.c or jslong.c, only in jsdtoa.c.

I thought someone already did a Borland-C port a while ago.  Maybe their "free 
compiler" is not quite as good as the non-free one?  McCabe, do you remember any 
Borland porting changes?

/be
Adding myself to CC.
I would really like to see Mozilla being able to compile with Borland C++Builder
compiler.
Roger, are you going to fix this bug?  If not, please find someone to reassign
it to, who will.

/be
Phil, can you see if we can get access to a Borland compiler to try builds? I'm 
happy to do the check-in if somebody wants to provide sr=, but I'd rather have 
proof that we're making the right changes.
For the impatient, source for JS that works on CodeWarrior and Borland is at 
http://www.its.caltech.edu/~shanti/js.zip. For CW, include "cw.h" as a 
precompiled header. For Borland, be sure to turn on 2-byte alignment.
I've downloaded the Borland compiler to my WinNT box, but I'm having trouble
using it to build the JS Engine. Here is what I am doing; I get the error


           "Incorrect command line option: -FoWINNT4.0_DBG.OBJ/"


Any suggestions? 


make -f Makefile.ref CC=/d/BORLAND/BCC55/bin/bcc32.exe
cat: ../../dist/WINNT4.0_DBG.OBJ/nspr/Version: No such file or directory
cd fdlibm; make -f Makefile.ref all
whoami: cannot find username for UID 500
/d/BORLAND/BCC55/bin/bcc32.exe -FoWINNT4.0_DBG.OBJ/ -c -DXP_UNIX /MD /Od /Z7  
-DXP_PC -DWIN32 -D_WINDOWS -D_WIN32 /nologo /W3
 /FpWINNT4.0_DBG.OBJ/js.pch  -DDEBUG -DDEBUG_  -DJSFILE  -D_IEEE_LIBM 
k_standard.c
Borland C++ 5.5.1 for Win32 Copyright (c) 1993, 2000 Borland
Error E2075: Incorrect command line option: -FoWINNT4.0_DBG.OBJ/
make[1]: *** [WINNT4.0_DBG.OBJ/k_standard.obj] Error 1
make: *** [all] Error 2
[/d/BORLAND/mozilla/js/src]
I wouldn't expect the existing makefile.ref to work with that compiler without 
some changes. See the jseng newsgroup thread on a new Borland specific makefile 
for use with the borland make utility and compiler...
news://news.mozilla.org/91cr92%24ova10%40secnews.netscape.com
...especially look at the followups for improvements on the original makefile.

I did make this work (just barely) on my machine.

I don't know if it is best to go with that strategy or to adapt makefile.ref and 
use the gnu make. I expect that people using the engine with borland tools might  
not agree either.

FWIW, I don't consider spending a lot of time fooling with the make system for a 
compiler that Netscape doesn't need to support a very good use of your time. I'd 
try to get the folks interested in supporting the Borland compiler to do the 
work to either whip the Borland make specific makefile into shape or to do the 
makefile.ref changes for you.
If you're compiling with CBuilder, just drag all the files into the project. If
you _really_ want to try it with the free compiler, then let me know and I'll
generate a makefile for you. Most of us in the Borland community are willing to
fork over $90 not to have to write makefiles. Just look at the differences in
the code I uploaded, and consider merging them into the standard distribution.
Shanti, if it wouldn't be too much trouble to generate a makefile for the 
free Borland compiler, that would help me out a lot! I'm new to it...


If it is a lot of work, however, never mind - I don't want to waste your time!


Thanks -
Phil
Makefile and cfg file, generated by the Borland IDE. It assumes your source 
files are in the JS\ directory, and that a OBJ\ directory exists.
http://www.its.caltech.edu/~shanti/makefile.zip 

A sample program, that embeds and extends JS to format JSAPI.XML. Sorry, 
documentation isn't complete.
http://www.its.caltech.edu/~shanti/jsdb.zip
Shanti, thanks! But here are the errors I'm getting now: 


[/d/Borland/mozilla/js] /d/BORLAND/BCC55/bin/make -f jslib.mak

MAKE Version 5.2  Copyright (c) 1987, 2000 Borland
        Bcc32 +BccW32.cfg -P- -c @MAKE0001.@@@
Borland C++ 5.5.1 for Win32 Copyright (c) 1993, 2000 Borland
js\prmjtime.c:
Error E2209 js\jsstddef.h 76: Unable to open include file 'stddef.h'
Error E2209 js\prmjtime.c 45: Unable to open include file 'string.h'
Error E2209 js\prmjtime.c 46: Unable to open include file 'time.h'
Error E2209 js\jstypes.h 63: Unable to open include file 'stddef.h'
Error E2257 js\jstypes.h 351: , expected
Error E2257 js\jstypes.h 359: , expected
Error E2209 js\jsprf.h 55: Unable to open include file 'stdio.h'
Error E2209 js\jsprf.h 56: Unable to open include file 'stdarg.h'
Error E2303 js\jsprf.h 102: Type name expected
Error E2303 js\jsprf.h 103: Type name expected
Error E2303 js\jsprf.h 104: Type name expected
Error E2303 js\jsprf.h 105: Type name expected
Error E2209 js\prmjtime.h 41: Unable to open include file 'time.h'
Error E2209 js\prmjtime.c 57: Unable to open include file 'sys\timeb.h'
Error E2450 js\prmjtime.c 169: Undefined structure 'tm' in function 
PRMJ_LocalGMTDifference
Error E2449 js\prmjtime.c 169: Size of 'ltime' is unknown or zero in function 
PRMJ_LocalGMTDifference
Error E2450 js\prmjtime.c 169: Undefined structure 'tm' in function 
PRMJ_LocalGMTDifference
Error E2450 js\prmjtime.c 169: Undefined structure 'tm' in function 
PRMJ_LocalGMTDifference
Error E2449 js\prmjtime.c 169: Size of 'ltime' is unknown or zero in function 
PRMJ_LocalGMTDifference
Error E2450 js\prmjtime.c 172: Undefined structure 'tm' in function 
PRMJ_LocalGMTDifference
Error E2109 js\prmjtime.c 172: Not an allowed type in function 
PRMJ_LocalGMTDifference
Warning W8065 js\prmjtime.c 172: Call to function 'memset' with no prototype in 
function PRMJ_LocalGMTDifference
Error E2451 js\prmjtime.c 173: Undefined symbol 'tm_mday' in function 
PRMJ_LocalGMTDifference
Error E2451 js\prmjtime.c 174: Undefined symbol 'tm_year' in function 
PRMJ_LocalGMTDifference
Warning W8065 js\prmjtime.c 180: Call to function 'mktime' with no prototype in 
function PRMJ_LocalGMTDifference
Error E2450 js\prmjtime.c 256: Undefined structure 'timeb' in function PRMJ_Now
Error E2449 js\prmjtime.c 256: Size of 'b' is unknown or zero in function 
PRMJ_Now
Error E2228 js\prmjtime.c 256: Too many error or warning messages in function 
PRMJ_Now

*** 26 errors in Compile ***

** error 1 ** deleting OBJ\prmjtime.obj

[/d/Borland/mozilla/js]




The first error above is occuring on line 76 of jsstddef.h, which is:


                     #include <stddef.h>



Is there some adjustment I need to make to the makefile to handle this? 
On my WinNT box, the included files (stddef.h, string.h, etc.) all seem 
to be available in this directory: 


                     /d/BORLAND/BCC55/Include
See also bug 62773, which is about compiling all of Mozilla on the Borland free
compiler. It has links to all the tools, and links to additional documentation
about the free compiler (including stuff that is not in the manuals supplied by
Borland).
Please submit these changes in the form of a UNIFIED CVS diff. This makes 
integrating much easier. If the changes compile on all platforms, we'll get this 
in.
Target Milestone: --- → Future
Reassigning to Kenton -
Assignee: rogerl → khanson
> Please submit these changes in the form of a UNIFIED CVS diff.
> This makes integrating much easier. If the changes compile
> on all platforms, we'll get this in.

Any chance we can get a diff for this? That would help us; thanks -
Phil will have the changes necessary for JS 1.5 shortly. With this version, it
seems that 4-byte alignment is necessary. Either 8-byte or 2-byte alignment
leads to crashes in garbage collection.
The diff is against RC3a because Shanti is working from the RC3a tarball. 
We're currently working on real-time access for Shanti via anonymous CVS login.

In the meantime, I wanted to get his patch up for review - 
Brendan, could you review the patch here?
Comment on attachment 59968 [details] [diff] [review]
Shanti's patch: diff -wu against JS1.5 RC3a source, May 2001

>+++ jsdtoa.c	2001/11/29 00:51:54
>@@ -752,7 +752,8 @@
>         k1 = 32 - k;
>         z = 0;
>         do {
>-            *x1++ = *x << k | z;
>+        /* Added parentheses for BCC and CW on Windows */
>+            *x1++ = (*x << k) | z;

This is fine, better style (it avoids a gcc warning too, IIRC), but it doesn't
need a comment.  Just overparenthesize bitwise and shift ops, always.  That's
JS house style, but jsdtoa.c was old David M. Gay from Guy Steele spec code.

This comment above, which again should be removed, is underindented -- FYI.

>@@ -836,13 +837,15 @@
> #ifdef ULLong
>     do {
>         y = (ULLong)*xa++ - *xb++ - borrow;
>-        borrow = y >> 32 & 1UL;
>+        /* Added parentheses for BCC and CW on Windows */
>+        borrow = (y >> 32) & 1UL;

Ditto -- I won't repeat for the remaining over-commented parenthesization
fixes, but some have misindented (over- as well as under-indented) comments.

>+++ js62Gjkjklibmath.h	2001/11/29 00:51:54
>@@ -62,7 +62,10 @@
>  * by default since there can be problems with endian-ness and such.
>  */
> 
>-#if defined(_WIN32) && !defined(__MWERKS__)
>+#if defined(__WIN32__) && defined(__BORLANDC__)
>+#define JS_USE_FDLIBM_MATH 0
>+
>+#elif defined(_WIN32) && !defined(__MWERKS__)
> #define JS_USE_FDLIBM_MATH 1
> 
> #elif defined(SUNOS4)
>@@ -99,7 +102,6 @@
> /*
>  * Use system provided math routines.
>  */
>-
> #define fd_acos acos
> #define fd_asin asin
> #define fd_atan atan
>@@ -145,10 +147,6 @@
> #define fd_fabs fabs
> #define fd_floor floor
> #define fd_fmod fmod
>-
>-extern double fd_atan2 __P((double, double));
>-extern double fd_copysign __P((double, double));
>-extern double fd_pow __P((double, double));

Aren't these being removed from the SUNOS4 sectio? How come (I hope I'm not
misreading the diff)?

>+++ jsmath.c	2001/11/29 00:51:54
>@@ -244,7 +244,11 @@
>             *rval = DOUBLE_TO_JSVAL(cx->runtime->jsNaN);
>             return JS_TRUE;
>         }
>+#ifdef __BORLANDC__
>+        if ((x==0)&&(x==z)&&(z<0))
>+#else
>         if ((x==0)&&(x==z)&&(fd_copysign(1.0,z)==-1))
>+#endif

I realize someone left out spaces, and overparenthesized == against &&, but
that's not JS house style, or necessary for any warning avoidance.  Please
deparenthesize and add some spaces around binary operators.  Ditto below.

>@@ -269,7 +273,11 @@
>             *rval = DOUBLE_TO_JSVAL(cx->runtime->jsNaN);
>             return JS_TRUE;
>         }
>+#ifdef __BORLANDC__
>+        if ((x==0)&&(x==z)&&(x<0))
>+#else
>         if ((x==0)&&(x==z)&&(fd_copysign(1.0,x)==-1))
>+#endif
>             z = x;
>         else
>             z = (x < z) ? x : z;
>@@ -384,7 +392,12 @@
> 
>     if (!js_ValueToNumber(cx, argv[0], &x))
>         return JS_FALSE;
>+#ifdef __BORLANDC__
>+    z = fd_floor(x + 0.5);
>+    if (x < 0 && z > 0) z = -z;

Then statement on its  own line for better readability and (more imp.)
debugability.

>+#else
>     z = fd_copysign(fd_floor(x + 0.5), x);
>+#endif
>     return js_NewNumberValue(cx, z, rval);
> }
> 
>Index: jstypes.h
>===================================================================
>RCS file: /cvsroot/mozilla/js/src/jstypes.h,v
>retrieving revision 3.19
>diff -w -u -r3.19 jstypes.h
>--- jstypes.h	2001/04/11 23:07:26	3.19
>+++ jstypes.h	2001/11/29 00:51:56
>@@ -82,6 +82,14 @@
> **
> **
> ***********************************************************************/
>+
>+/* Added to compile with BCC */
>+#if defined(__BORLANDC__) || defined(__MWERKS__)
>+#define _declspec __declspec
>+#endif

Why || defined(__MWERKS__) in this #if condition? Judging by what was there
before, this isn't necessary, is it?

>+++ jssh.c	2001/11/29 00:52:00

Isn't js/sh dead code?	Maybe we ought to cvs remove it.

>@@ -75,6 +75,26 @@
> #else
> #   error "Platform not supported."
> #endif
>+
>+/* Added to compile with CodeWarrior on Win32 */
>+#if defined( __MWERKS__) && defined(WIN32)
>+
>+char *strdup(const char *str)
>+{
>+    char *copy = (char *) malloc(strlen(str)+1);
>+    if (copy)
>+        strcpy(copy, str);
>+    return copy;
>+}
>+int fileno(FILE *stream)
>+{
>+ if (stream == stdin) return 0;
>+ if (stream == stdout) return 1;
>+ if (stream == stderr) return 2;
>+ return -1;
>+}

Indentation all wacky here, but I don't mind the then-return on same line as if
here.

/be
Attachment #59968 - Flags: needs-work+
Remarking on Brendan's comments...

+++ js62Gjkjklibmath.h	  2001/11/29 00:51:54

>Aren't these being removed from the SUNOS4 sectio? How come (I hope I'm not
>misreading the diff)?

I think this is an accident. No changes are required to jslibmath.h

Index: jstypes.h
===================================================================
RCS file: /cvsroot/mozilla/js/src/jstypes.h,v
retrieving revision 3.19
diff -w -u -r3.19 jstypes.h
--- jstypes.h	 2001/04/11 23:07:26	3.19
+++ jstypes.h	 2001/11/29 00:51:56
@@ -82,6 +82,14 @@
**
**
***********************************************************************/
+
+/* Added to compile with BCC */
+#if defined(__BORLANDC__) || defined(__MWERKS__)
+#define _declspec __declspec
+#endif

>Why || defined(__MWERKS__) in this #if condition? Judging by what was there
>before, this isn't necessary, is it?

MSVC uses _declspec, and most other compilers use __declspec. CW accepts both.

>Isn't js/sh dead code?    Maybe we ought to cvs remove it.

Judging from n.p.m.jseng, anybody who makes a console-mode JS interpreter
writes their own front end and their own File object. But js/sh is a good
tutorial to teach you how to write such a thing. You could put it in the
documentation, and remove it from the source tree. It would have taken a while
to figure out JS_IsBufferCompilableUnit() or how to write an error reporter
otherwise.

Also, since you mentioned it, jsmath.c uses (fd_copysign(1.0,x)==-1) when (x <
0) would have worked. I found the latter to be more portable. Any reason for
doing it the other way?

I've attached a diff that looks better and still compiles for Borland C++.

Shanti
>Also, since you mentioned it, jsmath.c uses (fd_copysign(1.0,x)==-1) when (x < 
>0) would have worked. I found the latter to be more portable. Any reason for 
>doing it the other way?

Yes, to handle negative 0 properly.  This is important, e.g., for atan2.

Good idea on turning js/sh into a tutorial.  As soon as 1.0 lets up, I'm going
to do some constructive violence to the SpiderMonkey docs.  Patch review in a
minute or two.

/be
Comment on attachment 74630 [details] [diff] [review]
Revised diff, with corrections

>+++ jsmath.c	2001/11/29 00:51:54
>@@ -244,7 +244,11 @@
>             *rval = DOUBLE_TO_JSVAL(cx->runtime->jsNaN);
>             return JS_TRUE;
>         }
>+#ifdef __BORLANDC__
>+        if (x==0 && x==z && z<0)
>+#else
>         if (x==0 && x==z && fd_copysign(1.0,z)==-1)
>+#endif

Thanks for fixing the space-less overparenthesized style there, good of you
since that was existing code -- if you're still game, how about a space on
either side of == and other binary ops?

Also, per my last coment, I think you need fd_copysign here to handle -0
properly.

>             z = x;
>         else
>             z = (x > z) ? x : z;
>@@ -269,7 +273,11 @@
>             *rval = DOUBLE_TO_JSVAL(cx->runtime->jsNaN);
>             return JS_TRUE;
>         }
>+#ifdef __BORLANDC__
>+        if (x==0 && x==z && x<0)
>+#else
>         if (x==0 && x==z && fd_copysign(1.0,x)==-1)
>+#endif
>             z = x;
>         else
>             z = (x < z) ? x : z;
>@@ -384,7 +392,12 @@
> 
>     if (!js_ValueToNumber(cx, argv[0], &x))
>         return JS_FALSE;
>+#ifdef __BORLANDC__
>+    z = fd_floor(x + 0.5);
>+    if (x < 0 && z > 0) z = -z;
>+#else
>     z = fd_copysign(fd_floor(x + 0.5), x);
>+#endif
>     return js_NewNumberValue(cx, z, rval);
> }
> 

Looks good otherwise, sr=brendan@mozilla.org if you fix the above, and assuming
the merge-up to the trunk from 1.3 is straightforward.

/be
Attachment #74630 - Flags: superreview+
I took the substantive changes out of jsmath.c (except to make it look better),
and add a separate patch to jsmath.c to implement copysign(). See the end of
the attachment. A good place to insert the code would be at the top.
Shanti, do you have CVS access yet? I notice that the versions you're
diffing against are not all current. For example, the latest patch says

Index: jscpucfg.c
===================================================================
RCS file: /cvsroot/mozilla/js/src/jscpucfg.c,v
retrieving revision 3.12
diff -w -u -r3.12 jscpucfg.c


But this file is at a higher version now:


[//d/JS_TRUNK/mozilla/js/src] cvs stat jscpucfg.c
===================================================================
File: jscpucfg.c        Status: Up-to-date

   Working revision:    3.15
   Repository revision: 3.15    /cvsroot/mozilla/js/src/jscpucfg.c,v
Attachment #74674 - Flags: needs-work+
Comment on attachment 74674 [details]
add fd_copysign implementation

>diff -w -u -r1.4 jssh.c
>--- jssh.c	1999/09/28 23:11:50	1.4
>+++ jssh.c	2001/11/29 00:52:00
>@@ -75,6 +75,26 @@
> #else
> #   error "Platform not supported."
> #endif
>+
>+/* Added to compile with CodeWarrior on Win32 */
>+#if defined( __MWERKS__) && defined(WIN32)
>+
>+char *strdup(const char *str)
>+{
>+    char *copy = (char *) malloc(strlen(str)+1);
>+    if (copy)
>+        strcpy(copy, str);
>+    return copy;
>+}

Nit: extra newline here.

>+int fileno(FILE *stream)
>+{
>+    if (stream == stdin) return 0;
>+    if (stream == stdout) return 1;
>+    if (stream == stderr) return 2;
>+    return -1;
>+}

And here.

>+#endif
>+
> 
> 
> /* globals */
>Index: jsmath.c
>===================================================================
>RCS file: /cvsroot/mozilla/js/src/jsmath.c,v
>retrieving revision 3.15
>diff -w -u -r3.15 jsmath.c
>--- jsmath.c	2000/08/19 08:37:04	3.15
>+++ jsmath.c	2001/11/29 00:51:54
>@@ +97,9
>+
>+#ifdef __BORLANDC__
>+double fd_copysign(double a,double b)
>+{
>+ if ((a > 0 && b < 0) || (a < 0 && b > 0)) return -b;
>+ return b;
>+}
>+#endif
>+

Hmm, shouldn't this come from fdlibm?  fd_copysign is supposed to take b's sign
bit and set it in a.

You'll want to compile with fdlibm anyway, unless Borland's libm is
exceptional.

/be
See also Bug 134113 (which is about making it work with gcc and mingw).
Metrowerks C++ (info applies to 7.x):
To get the strdup from the MSL (it's not standard):
#include <extras.h>

To get fileno from the MSL:
#include <unix.h>
#include <stdio.h>

Behavior of the MSL fileno is return 0, 1 or 2 for stdin, stdout, stderr
(respectively), else return file handle.

strdup calls __msl_strdup, which has same behavior as the fragment here, except
for some macros to handle namespaces in the library appropriately.

I'd suggest you probably want these includes vs. re-defining the routines.
Also, change jslong.h:#84 from

#define JSLL_INIT(hi, lo) ((hi ## i64 << 32) + lo ## i64)

to

#define JSLL_INIT(hi, lo) (((__int64)hi << 32) + lo)
Assignee: khanson → general
Keywords: js1.5
jstypes.h: if the inclusion of jsstddef.h is unwanted this can be replaced with
a conditional #define _WINDOWS, WIN32
jscpucfg.c: the error here might actually be the _int64 define, added the
#ifdef to make sure I don't break anythin
prmjtime.c: including <windows.h> sure looks like cannons for sparrows but it
doesn't change either build time or output size, so I kept it.
Attachment #179933 - Flags: review?(brendan)
-> Klaus

It would be good to get this in for an expected spidermonkey pre release when ff
1.1/Gecko 1.8 ship.
Assignee: general → drobec
QA Contact: pschwartau → general
(In reply to comment #32)

-> Bob
brendan has the diff for review and told me he wants to get it into 1.8final
This appears to have fallen off the radar. Blocking the js1.6 release for now.
Klaus, can you attach a fresh patch for review?
Blocks: js1.6rc1
Flags: testcase-
(In reply to comment #34)
> This appears to have fallen off the radar. Blocking the js1.6 release for now.
> Klaus, can you attach a fresh patch for review?

Obviously it's been several months since I last worked with a checkout. Less
obviously Your update just reached me 4 am Friday morning. I have to ask for
three days continuance - weekend, you see. If the source is stable and clean on
your side Monday, I'll be happy to give it another go.
Comment on attachment 179933 [details]
unified diff of js/src, js/src/fdlibm for bcc / JS_BUILD_STATIC

Clearing old request.

/be
Attachment #179933 - Flags: review?(brendan)
closing. we are way past this, reopen if you disagree.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: