451881 - nanojit won't compile with VC7.1 or mingw

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[[e-mail]]

js/nanojit won't compile with VC7.1 because it tries to make LOpcode unsigned which VC7.1 doesn't support; none of the enumerated values lies outside the range 0 to 127 and as far as I can tell signedness doesn't seem to matter.

js/nanojit won't compile with mingw because the windows.h include uses an _MSC_VER guard rather than an XP_WIN guard.

Comment 1

[[e-mail]]

Attachment: Proposed patch

* Removed __msvc_only macro and its usage
* Changed #include of <windows.h> guard to XP_WIN

Comment 2

[Brendan Eich [:brendan]]

Comment on attachment 335202 [details] [diff] [review]
Proposed patch

We don't do SR in js/src, and David is best reviewer for nanojit changes. Thanks,

/be

Comment 3

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 335202 [details] [diff] [review]
Proposed patch

Adobe doesn't use XP_WIN so I'd use WIN32 instead.  Perhaps it'd also be better to make a new macro (like __msvc8_or_higher) that does a version check on _MSC_VER, and to keep Adobe's unsigned keyword.

Neither Mozilla nor Adobe use VC 7.1 anymore so you may find compatibility being regressed.  For example variadic macros got committed a day or two ago and those won't work on VC 7.1.  You may want to turn those into fixed-argument macros, like Blah5(a,b,c,d,e).

Comment 4

[[e-mail]]

(In reply to comment #3)
> Neither Mozilla nor Adobe use VC 7.1 anymore so you may find compatibility
> being regressed.  For example variadic macros got committed a day or two ago
> and those won't work on VC 7.1.
If you're referring to the ones in libtheora then I did check that VC 7.1 was still supported before filing (and patching) bug 450265. If you're not then I'm wondering why my builds haven't tripped over them yet!

Comment 5

[David Anderson [:dvander] - inactive, e-mail if emergency]

I meant that nanojit is now using variadic macros.  Perhaps errors will only be triggered if you're compiling debug builds.

Comment 6

[[e-mail]]

If you mean vprof then that's only included by Makefile.ref not Makefile.in

Comment 7

[[e-mail]]

Attachment: Revised patch

* Switched windows.h guard from XP_WIN to WIN32
* Restored : unsigned keyword on LOpcode enum but for VC8 or later only

Comment 8

[David Anderson [:dvander] - inactive, e-mail if emergency]

Ack, my apologies on the WIN32 thing -- avmplus.h is ours not Adobe's.  Pushed to the tracemonkey repo as changeset f5ae2fbafae0, thanks!