198276 - RFE: Make Sun Workshop SHUT UP about |Distance(const nsReadingIterator<unsigned short>&,const nsReadingIterator<unsigned short>&)|

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Roland Mainz]

Nearly every 2nd week we have problems like the lines below:
-- snip --
Undefined                       first referenced
 symbol                             in file
unsigned Distance(const nsReadingIterator<unsigned short>&,const
nsReadingIterator<unsigned short>&) ../../dist/bin/libxpcom.so
unsigned Distance(const nsReadingIterator<char>&,const nsReadingIterator<char>&)
../../dist/bin/libxpcom.so
-- snip --

This usually breaks the Sun Workshop/Forte StaticBuild for which we do not have
a tinderbox and the (well known) workaround is to stick lots of |#include
"nsReadableUtils.h"| in all sources which use the string iterators or include
something which uses the string iterators or includes something which includes
something else which includes something which uses the string iterators.

Finally I am sick of it to fix that manually all the time which involves r= and
sr=. I'll file a patch which should make the compiler shut up forver!

Comment 1

[Roland Mainz]

Suffering is mine...

Comment 2

[Roland Mainz]

Attachment: Patch for 2003-03-19-08-trunk

Comment 3

[Roland Mainz]

Comment on attachment 117757 [details] [diff] [review]
Patch for 2003-03-19-08-trunk

Requesting r=/sr= ...

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 117757 [details] [diff] [review]
Patch for 2003-03-19-08-trunk

sr=bzbarsky, but this must get review from the module owner (jag).  Please do
not check this in with _anyone_ else's review without checking with me first.

Comment 5

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

Is there any chance that changing the extra foward declarations in
nsStringIteratorUtils.h from saying:

NS_COM size_t Distance( const nsReadingIterator<PRUnichar>&, const
nsReadingIterator<PRUnichar>& );
NS_COM size_t Distance( const nsReadingIterator<char>&, const
nsReadingIterator<char>& );

to saying:

NS_COM size_t Distance( const nsAString::const_iterator&, const
nsAString::const_iterator& );
NS_COM size_t Distance( const nsACString::const_iterator&, const
nsACString::const_iterator& );

would help?

Comment 6

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

(Actually, that might not compile, so what about making the reverse change, in
both nsReadableUtils.h and .cpp?)

Comment 7

[hacker formerly known as [e-mail]]

I made the reverse change on nebiros and all of xpcom built.  I'll let the
tinderbox catch up and see if the rest of the tree builds.  I don't know if that
reverse change will work everywhere else (OS/2 in particular).

Comment 8

[hacker formerly known as [e-mail]]

Attachment: dbaron's suggested changes

Comment 9

[jag (Peter Annema)]

Comment on attachment 117770 [details] [diff] [review]
dbaron's suggested changes

r=jag

silly compilers.

Comment 10

[jag (Peter Annema)]

Though I wonder... Do we still need this declaration in nsStringIteratorUtils.h?

This is the patch I had mkaply test a long time ago, and it worked fine on OS/2:

Index: string/public/nsStringIterator.h
===================================================================
RCS file: /cvsroot/mozilla/string/public/nsStringIterator.h,v
retrieving revision 1.3
diff -u -r1.3 nsStringIterator.h
--- string/public/nsStringIterator.h    13 Oct 2001 15:01:17 -0000      1.3
+++ string/public/nsStringIterator.h    15 Feb 2002 10:05:13 -0000
@@ -423,5 +423,8 @@
  {
    return lhs.get() != rhs.get();
  }
+  
+NS_COM size_t Distance( const nsReadingIterator<PRUnichar>&, const
nsReadingIterator<PRUnichar>& );
+NS_COM size_t Distance( const nsReadingIterator<char>&, const
nsReadingIterator<char>& );
 
#endif /* !defined(nsStringIterator_h___) */
Index: string/public/nsStringIteratorUtils.h
===================================================================
RCS file: /cvsroot/mozilla/string/public/nsStringIteratorUtils.h,v
retrieving revision 1.3
diff -u -r1.3 nsStringIteratorUtils.h
--- string/public/nsStringIteratorUtils.h       29 Jun 2001 12:47:07
-0000      1.3
+++ string/public/nsStringIteratorUtils.h       15 Feb 2002 10:05:13 -0000
@@ -36,13 +36,4 @@
    return lhs.fragment().mStart == rhs.fragment().mStart;
  }

-template <class CharT> class nsReadingIterator;
-
-  // NOTE: need to break iterators out into their own file (as with
many classes here), need
-  //  these routines, but can't currently |#include
"nsReadableUtils.h"|, this hack is bad
-  //  but we need it to get OS2 building again.  Fix by splitting
things into different files.
-NS_COM size_t Distance( const nsReadingIterator<PRUnichar>&, const
nsReadingIterator<PRUnichar>& );
-NS_COM size_t Distance( const nsReadingIterator<char>&, const
nsReadingIterator<char>& );
-
-
#endif /* !defined(nsStringIteratorUtils_h___) */


cls, could you see if that fixes the build problem on nebiros?

Comment 11

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

I think the build problem on nebiros is that it's using the typedef name in the
name mangling, so that would only work if the place where you're adding it has
access to nsAString and thus could use nsAString::const_iterator instead of
nsReadingIterator<PRUnichar>, etc.

Comment 12

[hacker formerly known as [e-mail]]

jag, your inline proposed patch breaks on nebiros.

Comment 13

[hacker formerly known as [e-mail]]

dbaron's patch has been checked in.

Comment 14

[Roland Mainz]

The dbaron's patch did not work:
-- snip --
/opt/SUNWspro/bin/CC  -I/usr/openwin/include -xbuiltin=%all -mt  -DDEBUG
-D_DEBUG -DDEBUG_mozilla -DTRACING -g -g -o nsTestSample ns
TestSample.o -xildoff -zlazyload -zcombreloc           -L../../dist/bin
-L../../dist/lib ../../dist/lib/libxpcomglue.a ../../dist/li
b/libembedstring.a
-L/shared/bigtmp2/mozilla/2003-03-20-08-trunk_cvs/objdir_ws7_gtk/dist/lib
-lplds4 -lplc4 -lnspr4 -lpthread -ldl -
lrt   -lsocket -ldl -lm     
Undefined                       first referenced
 symbol                             in file
unsigned Distance(const nsReadingIterator<unsigned short>&,const
nsReadingIterator<unsigned short>&) ../../dist/lib/libembedstring.a
(nsDependentSubstring.o)
unsigned Distance(const nsReadingIterator<char>&,const nsReadingIterator<char>&)
../../dist/lib/libembedstring.a(nsDependentSubstring.o)
ld: fatal: Symbol referencing errors. No output written to nsTestSample
gmake[3]: *** [nsTestSample] Error 1
gmake[3]: Leaving directory
`/shared/bigtmp2/mozilla/2003-03-20-08-trunk_cvs/objdir_ws7_gtk/xpcom/sample'
gmake[2]: *** [libs] Error 2
gmake[2]: Leaving directory
`/shared/bigtmp2/mozilla/2003-03-20-08-trunk_cvs/objdir_ws7_gtk/xpcom'
gmake[1]: *** [tier_2] Error 2
gmake[1]: Leaving directory
`/shared/bigtmp2/mozilla/2003-03-20-08-trunk_cvs/objdir_ws7_gtk'
gmake: *** [default] Error 2
-- snip --
and the resulting Zilla binaries crash due missing symbols.

Reopening bug...

Comment 15

[Roland Mainz]

Attachment: Patch for 2003-03-20-08-trunk to get Mozilla compiling and running again

Comment 16

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

Did you clobber your build?  I don't see why that include should help unless
it's covering up some other problem.

Comment 17

[Roland Mainz]

David Baron wrote:
> Did you clobber your build? 

That was a clean, fresh source pull and clean objdir. 
And the same issue occurs for the automated Phoenix+Mozilla nightly build system
which runs on another box and on another, seperate account.

Comment 18

[Roland Mainz]

Comment on attachment 118120 [details] [diff] [review]
Patch for 2003-03-20-08-trunk to get Mozilla compiling and running again

Requesting r=/sr= for this patch to get the build and runtime bustage fixed for
now (bug should be kept open to go after the real issue once this patch has
been checked-in) ...

Comment 19

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 118120 [details] [diff] [review]
Patch for 2003-03-20-08-trunk to get Mozilla compiling and running again

jag and dbaron are much more qualified reviewers for this than I am.

Comment 20

[hacker formerly known as [e-mail]]

Nebiros tripped over this as well when timeless caused that makefile to be rebuilt.

I think the problem in this case is that embedstring.a has a dependency upon
|Distance| via nsDependentSubstring.o but we're not actually linking against
libxpcom.so to resolve the dependency.  Adding nsReadableUtils.cpp to
embedstring.a would probably resolve that issue.

Comment 21

[hacker formerly known as [e-mail]]

Attachment: Change embedstring standalone Distance signatures

Comment 22

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

Comment on attachment 118343 [details] [diff] [review]
Change embedstring standalone Distance signatures

sr=dbaron, although it might be nice to leave the space between the "&" and the
variable names.

Comment 23

[jag (Peter Annema)]

Comment on attachment 118343 [details] [diff] [review]
Change embedstring standalone Distance signatures

r=jag, see dbaron's comment.

Comment 24

[hacker formerly known as [e-mail]]

Patch checked in with extra spaces.