Closed Bug 451881 Opened 11 years ago Closed 11 years ago

nanojit won't compile with VC7.1 or mingw

Categories

(Core :: JavaScript Engine, defect)

x86
All
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: neil, Assigned: neil)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
Attached patch Proposed patch (obsolete) — Splinter Review
* Removed __msvc_only macro and its usage
* Changed #include of <windows.h> guard to XP_WIN
Assignee: general → neil
Status: NEW → ASSIGNED
Attachment #335202 - Flags: superreview?(brendan)
Attachment #335202 - Flags: review?(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
Attachment #335202 - Flags: superreview?(brendan)
Attachment #335202 - Flags: review?(danderson)
Attachment #335202 - Flags: review?(brendan)
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).
Attachment #335202 - Flags: review?(danderson) → review-
(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!
I meant that nanojit is now using variadic macros.  Perhaps errors will only be triggered if you're compiling debug builds.
If you mean vprof then that's only included by Makefile.ref not Makefile.in
Attached patch Revised patchSplinter Review
* Switched windows.h guard from XP_WIN to WIN32
* Restored : unsigned keyword on LOpcode enum but for VC8 or later only
Attachment #335202 - Attachment is obsolete: true
Attachment #336234 - Flags: review?(danderson)
Attachment #336234 - Flags: review?(danderson) → review+
Ack, my apologies on the WIN32 thing -- avmplus.h is ours not Adobe's.  Pushed to the tracemonkey repo as changeset f5ae2fbafae0, thanks!
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Flags: in-testsuite-
Flags: in-litmus-
You need to log in before you can comment on or make changes to this bug.