682066 - Fix build with clang

Status: RESOLVED FIXED

(Toolkit Graveyard :: Build Config, defect)

Description

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

Attachment: add nostdcall to some methods

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.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 555816 [details] [diff] [review]
add nostdcall to some methods

I don't know enough about this to review.

Comment 2

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

Attachment: alternative patch

This patch also fixes the build. I am OK with both.

Comment 3

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

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.

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 555816 [details] [diff] [review]
add nostdcall to some methods

We discussed a better approach on IRC.

Comment 5

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

Attachment: drop NS_DEFCALL

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.

Comment 6

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

http://tbpl.allizom.org/?tree=Try&usebuildbot=1&rev=1783df1a7824

Comment 7

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

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.

Comment 8

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

Attachment: blame of nscore.h

Comment 9

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/7fa469f47cbb