Closed Bug 394447 Opened 17 years ago Closed 17 years ago

Tamarin's __DEBUGGING__ macro conflicts with Mac system headers

Categories

(Tamarin Graveyard :: Virtual Machine, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: jorendorff, Assigned: jorendorff)

Details

Attachments

(2 files)

(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.
Assignee: nobody → jorendorff
Status: NEW → ASSIGNED
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.
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?
Not that I can think of.
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).
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.)
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...
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?
Attachment #279104 - Flags: review?(dansmith)
Attachment #279114 - Flags: review?(dansmith)
Attachment #279114 - Flags: review?(dansmith) → review+
Attachment #279104 - Flags: review?(dansmith) → review+
pushed changesets d81eb3457771 and e80d62e610e4.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: