Closed Bug 387935 Opened 13 years ago Closed 12 years ago

ActionMonkey: Always build JS as C++

Categories

(Firefox Build System :: General, defect)

Other Branch
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jorendorff, Assigned: benjamin)

References

Details

Attachments

(1 file, 2 obsolete files)

In preparation for merging MMgc.

- Change both Makefile.ref and Makefile.in .
- Rename .c files to .cpp as appropriate.  (Use hg rename.)
Blocks: 387012
Attached patch patch Makefile.in v1 (obsolete) — Splinter Review
Some reason, something /else/ is using HOST_CC, so I'm overwriting HOST_CC with the HOST_CXX. I tried |CPP_PROG_LINK = 1| but that doesn't seem to trigger using HOST_CXX instead of HOST_CC according to the logic in config/rules.mk.

Is there a better way?
HOST_CPPSRCS is what triggers HOST_CXX in the simple case: http://mxr.mozilla.org/mozilla/source/config/rules.mk#988

Which admittedly doesn't make much sense for SIMPLE_PROGRAMS. Add an extra var to the rules.mk conditional to cover that case?
I probably didn't need to change the *_CFILES variable names to *_CPPFILES.. but just keeping things consistent. :) (But I didn't switch CFLAGS..)

Maconly because I only changed the config/Darwin.mk file to use g++ instead of cc when making the shared library (libjs.dylib).

Should editline,etc. be made to compile with c++?
Don't waste time on changing editline etc. to use C++ -- the only reason we need C++ now is MMgc's API being C++. MMgc will be used by core js/src/js* code, and maybe a few XPConnect cheaters.

/be
Attached patch patch v2Splinter Review
There's some checks for IRIX, AIX, ... stuff, and I just replaced the CC with CXX. ??

For the non Darwin config/*.mk, they use LD or libtool for MKSHLIB, so Darwin is the only one that needs the switch from CC to CXX.

The various .c files will need to be renamed, but |hg diff| just shows the original file getting all '-'d and the new file is '+'d, but the history should persist.

ls *.c | awk '{ sub(/\.c$/,""); print "hg rename "$1".c "$1".cpp"}' | sh

Should this be pushed to hg/mozilla-cental in addition to hg/actionmonkey?
Attachment #272095 - Attachment is obsolete: true
Attachment #272101 - Attachment is obsolete: true
Attachment #272244 - Flags: review?(benjamin)
Attachment #272244 - Flags: review?(benjamin) → review+
For hg/actionmonkey,

remote: added 1 changesets with 44 changes to 44 files
changeset 3463:  	bd9c9cbf9ec8

Should I push to hg/mozilla-central?
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
pushed to hg/mozilla-central
changeset 3625:  	0f62afa8267f

hg will correctly merge incoming changes from cvs-trunk-mirror for *.c{,pp}
Summary: Always build JS as C++ → ActionMonkey: Always build JS as C++
This has been backed out of mozilla-central as a suspect in the Linux performance regression. Going to reopen this to track what should happen next.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee: edilee → benjamin
Status: REOPENED → NEW
Oops, I relanded this soon after, as it wasn't the cause of the perf regression.
Status: NEW → RESOLVED
Closed: 13 years ago12 years ago
Resolution: --- → FIXED
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.