Last Comment Bug 669972 - use jswin.h to clean up from windows.h crapping on STRICT
: use jswin.h to clean up from windows.h crapping on STRICT
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: x86_64 Linux
: -- normal (vote)
: mozilla8
Assigned To: Steve Fink [:sfink] [:s:] (PTO Sep23-28)
:
Mentors:
Depends on:
Blocks: 588537
  Show dependency treegraph
 
Reported: 2011-07-07 12:04 PDT by Steve Fink [:sfink] [:s:] (PTO Sep23-28)
Modified: 2011-07-13 08:43 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Move more windows.h handling to jswin.h (2.06 KB, patch)
2011-07-07 12:09 PDT, Steve Fink [:sfink] [:s:] (PTO Sep23-28)
jwalden+bmo: review+
Details | Diff | Splinter Review

Description Steve Fink [:sfink] [:s:] (PTO Sep23-28) 2011-07-07 12:04:28 PDT
json.h includes jsbuiltins.h includes nanojit/nanojit.h indirectly includes windows.h, which #defines STRICT. json.h uses STRICT in a DecodingMode enum. Because json.h also #undefs STRICT, it's normally ok.

But then, I modify jsprobes.h to include jswin.h which includes windows.h. Now jsonparser.cpp ends up including json.h first, then jsprobes.h. Mysteriously, windows.h seems to redefine STRICT the second time it's included, too, so jsonparse.cpp ends up compiling with STRICT #defined to something else. (Which happens to compile, but produces the wrong results.)

This seems bizarre, since I would expect windows.h to have protection from multiple inclusion, but that's the observed behavior. And I've fled my Windows dev environment purgatory for now, so I can't dig into it further at the moment. Plus, I don't really care why it was happening.
Comment 1 Steve Fink [:sfink] [:s:] (PTO Sep23-28) 2011-07-07 12:09:47 PDT
Created attachment 544576 [details] [diff] [review]
Move more windows.h handling to jswin.h

Use jswin.h to deal with windows.h. It'd be nice to replace nanojit's windows.h include with jswin.h to catch it at the source, but I don't understand the rules of the nanojit subtree.

Also, this patch could be expanded to handle jstracer.cpp's include of nanojit.h, but that looked like a slippery slope -- to do it right, it seems like jswin.h should also take over the alloca stuff from jstracer.cpp, but that involves

  #ifdef SOLARIS

so it would make jswin.h broader than just Windows, and that starts to feel messy.
Comment 2 Jeff Walden [:Waldo] (remove +bmo to email) 2011-07-08 15:03:22 PDT
Comment on attachment 544576 [details] [diff] [review]
Move more windows.h handling to jswin.h

Review of attachment 544576 [details] [diff] [review]:
-----------------------------------------------------------------
Comment 3 Steve Fink [:sfink] [:s:] (PTO Sep23-28) 2011-07-12 16:39:55 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/4d7f75359cc4

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