104172 - Remove extraneous exported symbols from XPCOM

Status: RESOLVED INVALID

(Core :: XPCOM, defect)

Description

[Jon Smirl]

Patch changes no behavior, only moves around export definitions to eliminate 
unnecessary exported symbols.

Comment 1

[Jon Smirl]

Attachment: remove extra exported symbols

Comment 2

[Doug Turner (:dougt)]

patch looks okay, but I wonder if we are going about this the wrong way?  seams
to me that we want to have a def file and have a very small list of exported
functions.  but I guess this patch is somewhat orthoganal.  rick, you are the
api export guru after that last patch, thoughts?

Comment 3

[Alec Flett]

hot damn! that's great. so everything still builds and runs as before?

On a side note, I like the idea of having a list of "known exports" via a .def
file, but how hard is it to export entire classes, i.e. such as nsVoidArray or
something?

Comment 4

[Jon Smirl]

This bug is a symptom of a larger problem. Everytime a non-null {}, non-inline 
function is declared in an h file a copy of the function is included in that 
compilation module. This fix removes 100's of needless copies of these 
functions. The NS_EXPORT problem just made these copies visible, they were 
there even without the NS_EXPORT. 

After this fix is in I'll go fix up a similar problem with NS_EXPORT in 
content, layout, parser. I will also go inspect the h files and see if I can 
spot more function declarations in them. 

Comment 5

[Alec Flett]

ok, we'll need some reviewers anyway. I think we should go ahead and get this
checked in, and tackle the larger problem that dougt talks about.
so, sr=alecf on these patches.. just need an r=? dougt?

Comment 6

[Doug Turner (:dougt)]

Maybe I just don't understand enough about this problem.  What is the problem
with nsProxyEvent.*.  This class shouldn't isn't used outside of xpcom.  Why did
it have to be touched?

Comment 7

[Doug Turner (:dougt)]

just read the newsgroup postings.  I would like to have jband or rick review this.  

Comment 8

[Alec Flett]

I just looked at the proxy stuff - for some reason there are lots of libraries
exporting the proxy stuff - addrbook, appcomps, imglib2, to name a few..

Comment 9

[Jon Smirl]

Attachment: Patched idl file instead of generated h file

Comment 10

[Doug Turner (:dougt)]

this does not build on the mac.  There is some problem linker problem when
trying to link oji.

Comment 11

[Jon Smirl]

Did I accidentally get part of this patch mixed into that big merged patch I 
made?

http://bugzilla.mozilla.org/show_bug.cgi?id=104515

Comment 12

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Why the scattering of |inline| in nsIInterfaceRequestorUtils.h and nsCOMPtr.h? 
C++ section 9.3 (clause 2) or section 7.1.2 (clause 3) says they're already inline.

Comment 13

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

(And does an optimized Java plugin still work with a debug build with this patch?)

Comment 14

[Jon Smirl]

Java plug-in still works, but I am recompiling it on my machine. Do we have to 
freeze the OJI stuff forever to allow backwards binary compatibility? All the 
OJI patch did was change the exported symbol name from C++ managled to C. I 
thought there might be a possiblility that the Java support could be compiled 
with a different C++ compiler than Mozilla. For example someone on Solaris 
using a Sun supplied OJI compiled with the SUN compiler and then running a 
later Mozilla compiled with GCC.

As for the inlines, in some cases with the MS compiler I get inline functions 
and some case I get one copy per compilation unit. I just added them to make 
sure. Anyway, the routines involved are going to be reworked into the static 
XPCOM lib when that happens.

Comment 15

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

> I just added them to make sure.

Well, that code was rather clean, and I'd rather keep it that way.  If they
don't do anything, could you not add them?

Comment 16

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

(And no, those nsCOMPtr methods should stay inline, they shouldn't be part of a
static XPCOM library.)

Comment 17

[Jon Smirl]

From looking at the tinderbox logs, it looks to me like there is probably an 
error in the building of OJI. OJI currently does not build correctly on Windows:
http://bugzilla.mozilla.org/show_bug.cgi?id=101410 Nobody notices this becuase 
OJI rarely changes and needs to be rebuilt.

The MAC build process may have a similar bug. This could explain why Doug 
didn't see a problem on his local mac; he could have built in a different 
sequence and hid the OJI build problem.

Also, if these patches didn't work on the Mac in general I would have expected 
to see about 50,000 link errors. Instead the only module that failed to link 
was OJI.

I don't have access to a Mac so I can't track this problem down. I can verify 
that the patch that was checked in worked fine on Windows.

Comment 18

[Benjamin Smedberg]

Life is all different now