394447 - Tamarin's __DEBUGGING__ macro conflicts with Mac system headers

Status: VERIFIED FIXED

(Tamarin Graveyard :: Virtual Machine, defect)

Description

[Jason Orendorff [:jorendorff]]

(filing this bug against Tamarin VM, but it's really against the Tamarin build system)

The Tamarin build system defines __DEBUGGING__ via a command-line option to the compiler.  It doesn't appear to be used anywhere, though.

We should remove it; there's a Mac OS X system header, CarbonCore/Debugging.h, which contains this code:

  #ifndef __DEBUGGING__
  #define __DEBUGGING__
  ...
  #endif /* __DEBUGGING__ */

If __DEBUGGING__ is defined on the command line, you basically can't include this header.  This actually tripped me up doing build integration for ActionMonkey.

Comment 1

[Jason Orendorff [:jorendorff]]

Attachment: remove __DEBUGGING__ from both the cross-platform build system and the Mac xcode project files

Comment 2

[Jason Orendorff [:jorendorff]]

It appears __DEBUGGING__ was added intentionally to disable that header, due to a naming conflict between pcre and Carbon.  So the attached patch won't work.  Maybe we can fix this some other way.

Comment 3

[Jason Orendorff [:jorendorff]]

This conflict between pcre and Carbon is fixed in the latest release of pcre, version 7.3.  We're using version 6.0.  Any reason not to upgrade?

Comment 4

[Edwin Smith]

Not that I can think of.

Comment 5

[Dan Smith]

Jason, the PCRE in Tamarin is a modified version to make it compatible with ECMAScript. Here's a note from Chris Nuuja who made the changes to the Tamarin PCRE:

I've made lots of small mods and one or two significant mods.  ECMAScript RegExp differs from the pcre standard in a few significant ways.  There are lots of expressions which will evaluate differently between the two.  

If the new changes don't affect the code I moded, then it should be simple.  Look at the change history for pcre_compile.cpp and pcre_exec.cpp  (look in mainline flashPlayer/avmplus/pcre.  For some reason /as/avmplus/pcre is missing a lot of change history).

Comment 6

[Jason Orendorff [:jorendorff]]

In that case-- Since it is a modified version of PCRE anwyay, there's no harm manually patching in the few lines I need.

+It turns out that the Mac Debugging.h header also defines the macro DPRINTF, so
+be absolutely sure we get our version. */
+
+#undef DPRINTF

I'll post a new patch with that change.  (If you want to upgrade to PCRE 7.3 anyway, let me know.)

Comment 7

[Steven Johnson]

Sounds like we should submit our changes (with suitable ifdef's) back to the PCRE maintainers, so we can track the current version and not have to re-make them...

Comment 8

[Jason Orendorff [:jorendorff]]

Attachment: fix conflict between pcre and Carbon headers

Here's the fix.  (Apply both patches.)

For some reason Bugzilla is not showing me controls for requesting a code review.  So, this is me requesting a review.  :-)  Anyone?

Comment 9

[Jason Orendorff [:jorendorff]]

pushed changesets d81eb3457771 and e80d62e610e4.