Closed Bug 682066 Opened 8 years ago Closed 8 years ago

Fix build with clang

Categories

(Toolkit Graveyard :: Build Config, defect)

x86_64
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla9

People

(Reporter: espindola, Assigned: espindola)

Details

Attachments

(4 files)

One difference from clang to gcc is that clang uses the regparam attribute as part of the type when deciding which template specialization to use. This in intentional (see http://llvm.org/pr10735).

A consequence for us is in nsRunnableMethodTraits. We have a specialization for plain methods and one for __stdcall (I assume because VC behaves like clang?). We are missing one with NS_DEFCALL.

One way to fix this is to just add another specialization with an ifdef for clang. Another option is to avoid using that template with these methods.

The attached patch implements the second option.
Attachment #555816 - Flags: review?(mh+mozilla)
Attachment #555816 - Flags: review?(mh+mozilla) → review?(gavin.sharp)
Comment on attachment 555816 [details] [diff] [review]
add nostdcall to some methods

I don't know enough about this to review.
Attachment #555816 - Flags: review?(gavin.sharp) → review?(benjamin)
This patch also fixes the build. I am OK with both.
Attachment #556060 - Flags: review?(khuey)
Comment on attachment 556060 [details] [diff] [review]
alternative patch

Actually, this bug fails on x86_64:

#if defined(__i386__) && defined(__GNUC__) && \
    (__GNUC__ >= 3) && !defined(XP_OS2)
#define NS_DEFCALL __attribute__ ((regparm (0), cdecl))
#else
#define NS_DEFCALL
#endif

I can be extended to work, but I think the other patch is better now.
Attachment #556060 - Flags: review?(khuey)
Attachment #555816 - Flags: review?(benjamin) → review?(khuey)
Comment on attachment 555816 [details] [diff] [review]
add nostdcall to some methods

We discussed a better approach on IRC.
Attachment #555816 - Flags: review?(khuey)
Attached patch drop NS_DEFCALLSplinter Review
There are more cleanups to be done, but this is sufficient to fix the build, so I will send the rest as follow up patches/bugs.
Assignee: nobody → respindola
Status: NEW → ASSIGNED
Attachment #556588 - Flags: review?(khuey)
Comment on attachment 556588 [details] [diff] [review]
drop NS_DEFCALL

I think this is fine, but I'd like somebody with some more institutional memory to sign off on it.
Attachment #556588 - Flags: review?(khuey)
Attachment #556588 - Flags: review?(benjamin)
Attachment #556588 - Flags: feedback+
Attachment #556588 - Flags: review?(benjamin) → review+
http://hg.mozilla.org/mozilla-central/rev/7fa469f47cbb
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.