linking with MOZ_GLUE_PROGRAM_LDFLAGS should use the C++ compiler if necessary

RESOLVED FIXED in mozilla14

Status

()

Core
Build Config
RESOLVED FIXED
5 years ago
a year ago

People

(Reporter: froydnj, Assigned: froydnj)

Tracking

(Depends on: 1 bug)

unspecified
mozilla14
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
rules.mk for PROGRAMS and SIMPLE_PROGRAMS says:

ifeq ($(CPP_PROG_LINK),1)
    $(EXPAND_CCC) ... $(MOZ_GLUE_PROGRAM_LDFLAGS) ...
else
    $(EXPAND_CC) ... $(MOZ_GLUE_PROGRAM_LDFLAGS) ...
endif

If MOZ_GLUE_PROGRAM_LDFLAGS links in C++ code, then we should be using the platform's C++ compiler to link to ensure that we correctly link any necessary C++ runtime code.  (Should we do this unconditionally?  I don't know all the permutations of cases for things that go in or places that use MOZ_GLUE_PROGRAM_LDFLAGS.)

This doesn't cause problems now, but it will as more code is added to places like MFBT, for instance.
I'd say just link everything as C++ and if anything blows up, fix it.
(Assignee)

Comment 2

5 years ago
Created attachment 613683 [details] [diff] [review]
patch

This patch looks like it at least builds on try.

I didn't nuke CPP_PROG_LINK, as it's still used for purify/quantify targets (which could probably be nuked in a separate bug).
Attachment #613683 - Flags: review?(ted.mielczarek)
Comment on attachment 613683 [details] [diff] [review]
patch

Review of attachment 613683 [details] [diff] [review]:
-----------------------------------------------------------------

If you're not going to rip out CPP_PROG_LINK, can you at least file a followup on doing so?

::: config/rules.mk
@@ +868,5 @@
>  # 2-second granularity
>  	touch -t `date +%Y%m%d%H%M.%S -d "now+5seconds"` pgo.relink
>  endif
>  else # !WINNT || GNU_CC
> +# mozglue contains C++ code, so we need to link with the C++ compiler.

I wouldn't bother with this comment.
(Assignee)

Updated

5 years ago
Blocks: 744444
Comment on attachment 613683 [details] [diff] [review]
patch

Review of attachment 613683 [details] [diff] [review]:
-----------------------------------------------------------------

::: config/rules.mk
@@ +868,5 @@
>  # 2-second granularity
>  	touch -t `date +%Y%m%d%H%M.%S -d "now+5seconds"` pgo.relink
>  endif
>  else # !WINNT || GNU_CC
> +# mozglue contains C++ code, so we need to link with the C++ compiler.

I wouldn't bother with this comment.
Attachment #613683 - Flags: review?(ted.mielczarek) → review+
Assignee: nobody → nfroyd
(Assignee)

Comment 5

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/e31de91d13cc
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla14
https://hg.mozilla.org/mozilla-central/rev/e31de91d13cc
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Blocks: 747348
Funnily, 4 years later, this makes us link nspr with libstdc++...
Depends on: 1272626
You need to log in before you can comment on or make changes to this bug.