52254 - Fix warnings/ambiguities and build issues on AIX

Status: RESOLVED FIXED

(SeaMonkey :: General, defect, P3)

Description

[Jim Dunn]

This is a bug to mark some build issues on aix.
There are 3 problems that I have found.
1) "private" assumed for base class  for the case of
   class blah: public baseclass1, baseclass2 {
   which should be
   class blah: public baseclass1, public baseclass2 {

2)  Return value of type <blah> is expected.
    <blah> someclass::somefunc() {
       for (;;) {
          return (<blah>) 0;
       }
    }
    which should be
    <blah> someclass::somefunc() {
       for (;;) {
         return (<blah>) 0;
       }
       return (<blah>) 0;  /* NEVER Reached */
    }

3) and in config/rules.mk, do not AR_Extract the .o's
   from a <blah>_s.a when linking in <blah>_s.a
   to a .so

Comment 1

[Jim Dunn]

Attachment: List of diffs

Comment 2

[Jim Dunn]

this blocks 18688
making me QA Contact

Comment 3

[Brendan Eich [:brendan]]

How about using break from inside for loops (beware nested switch statements) 
and putting the return at the bottom?

If I were doing this grunt work, I might not minimize change (e.g. I'd be sorely 
tempted to change space = space + 1; in nsBasePrincipal.cpp to space++;).

The explicitly public base class changes seem good to me.

/be

Comment 4

[Scott Collins]

...except that you should prefer pre-increment over post-, except where you 
explicitly require the value before increment.  On most hardware, it's more 
efficient for native types; on all hardware it's more efficient for (correctly 
implemented) user defined types; and it's conceptually simpler. 

Comment 5

[Brendan Eich [:brendan]]

> ...except that you should prefer pre-increment over post-, except where you 
> explicitly require the value before increment.

Except (and for C only, in general -- I am a C luddite, but even in C++ I apply 
this rule to integral and pointer types) where the value is not used at all, the 
compiler is free to select the best increment instruction, and no compiler I 
know of would stupidly waste a useless temporary to hold the pre-incremented 
value.

Yes, if you're using C++ iterators that overload operator++ and operator++(int), 
then the difference matters -- in which case, knock yourself out minimizing the 
number of rules to remember by always using ++i; -- but in nsBasePrincipal.cpp I 
see no iterators.

Sorry, this is getting too personal: my PDP-11 assembly habits make me prefer 
--p and p++ for scalar and pointer types.  I'll shut up now.

/be

Comment 6

[Scott Collins]

from email:

These changes all look good to me with this caveat: in several places,
you change declarations from this

  class A : B

to this

  class A : public B

you should _definitely_ check with module owners in this case, since
the meaning of the former declaration is actually

  class A : private B

so your patch introduces a semantic change.  If it were me, I would be
more likely to substitute this last form to quiet warnings.  Please
check with owners to make sure this is what they intended.

Comment 7

[Jim Dunn]

rdf/content/src/nsXULKeyListener.cpp r= hyatt
intl/uconv/src/nsCharsetConverterManager.cpp  r= cata
intl/uconv/src/nsCharsetMenu.cpp r= cata
xpfe/browser/src/nsBrowserInstance.cpp r= alecf

r= scc & brendan for the following files (return code warnings)
NOTE: brendan suggested putting 'breaks' in the for (;;) and
doing the normal return.  I personally don't want to do this since
my change does not effect the current codepath, I just prevent the
warnings.  I don't plan to rearrange the code, but will stick to
the changes indicated.  
  caps/src/nsBasePrincipal.cpp
  extensions/wallet/src/singsign.cpp
  layout/base/src/nsSelection.cpp
  mailnews/addrbook/src/nsDirPrefs.cpp
  mailnews/imap/src/nsIMAPGenericParser.cpp

Comment 8

[Brendan Eich [:brendan]]

No worries, I already bozo'd on the start = space + 1 (not space = space + 1; or
space++ or ++space!  Need new glasses). Minimal change, a=[e-mail]

/be

Comment 9

[Jim Dunn]

layout/xul/base/src/nsBoxToBlockAdaptor.h r= evaughan
editor/base/nsHTMLEditRules.h r= jfrancis

Comment 10

[Jim Dunn]

dom/src/base/nsJSEnvironment.h r= jst

Comment 11

[Jim Dunn]

I checked in everything except rules.mk.  I am going through
some things that CLS brought up.  So will mark this fixed and 
deal with that separately.

THANKS EVERYONE!