70386 - Kill "was hidden by" warnings.

Status: RESOLVED WONTFIX

(SeaMonkey :: General, defect, P3)

Description

[Dan Rosen]

There are some warnings on Linux which really should go away (they could very
well be errors on other platforms). Some are weird, and they irk me. Some are
simple, and they still irk me. I fix.

Comment 1

[Dan Rosen]

warning in nsStyleContext.cpp, line 3798: "this" isn't a void* (fix: cast to void*).
warning in mimemoz2.cpp, line 1010: '????' is interpreted as char, not char*
(fix: should be "????", in double-quotes).

Comment 2

[Dan Rosen]

Attachment: Linux warning-fix patch #1

Comment 3

[timeless]

r=timeless for 1/2
veto=timeless for 2/2
Index: mailnews/mime/src/mimemoz2.cpp
===================================================================
RCS file: /cvsroot/mozilla/mailnews/mime/src/mimemoz2.cpp,v
'????' is mac style evilness, it's used a lot.
1011 #ifdef XP_MAC
1012 static PRUint32
1013 mime_convert_chars_to_ostype(const char *osTypeStr)
please note that the return type is static PRUint32. '????' becomes a number. 
it's also in XP_MAC, please explain How your linux box managed to compile that 
block.

Comment 4

[Dan Rosen]

That's a good question... Could be a preprocessor oddity. I'll ignore it for now
and just apply the first half of the patch instead.

Comment 5

[Dan Rosen]

Ok, checked in the first half (nsStyleContext.cpp rev 3.154). The '????' problem
seems to be, the preprocessor interprets two question-marks followed by another
character as a "preprocessor trigram," and I don't know what that does. As
timeless points out, '????' is supposed to be a PRUint32, so I'll try to figure
out what's going on with that at some point. Not a big issue, afaict.

Comment 6

[Mike Pinkerton (not reading bugmail)]

this isn't another 'dr has a crack-baby compiler' issue, is it? ;)

Comment 7

[Dan Rosen]

Don't make me kick your ass, beavis. No, I'm pretty sure this one has nothing to
do with my crack-baby compiler... Although maybe you could tell me what sort of
crack-baby fu '????' is...

Comment 8

[timeless]

<bbaetz> timeless: You've seen what the gcc info pages say about trigraphs, 
right?
`-trigraphs'
Support ANSI C trigraphs.  You don't want to know about this
brain-damage.  The `-ansi' option implies `-trigraphs'.

So much for crack compilers.

As for '????' afaik it's the name of a special owner, specifically an 
application type that may not be used, documents can be assigned it to be 
treated as orphans.

Comment 9

[Simon Fraser [no longer active]]

cc: ducarroz, who probably owns the '????' in the first place. Why does the linux 
compiler ever see these, though?

Comment 10

[timeless]

basic example:
//??\
codethatwillnothappen();
reason: the by power of ??, \ overides the newline character.


in order for this to translate into out scenario, consider this:
#ifdef XP_MAC
...????\
#else
codethatwillnothappen();
#endif

hence the stupid compiler feels a need to check _all_ code even though it's in 
a preprocessor def.

I think it's probably safe for mozilla.org to ban trigraphs and just add the 
disable-trigraphs option (whatever that is) to the build system.

-disclaimer: i don't speak as an expert, these are just my poor interpretations 
of what i've read and seen.

Comment 11

[Dan Rosen]

sfraser: timeless was correct, but the simple answer is basically that one of
these "trigraph" things is a preprocessing token. So they're processed at the
same time that you're dealing with ifdefs, comments and such; not after.

If the mac compiler doesn't recognize these as trigraphs, but instead as
something else, it must be somewhat non-ansi-compliant, but I'm not sure we give
a rat's ass. I'd be just as happy to add -Wno-trigraphs to the gcc compile
options (we use -Wall which turns on trigraph warnings).

What do you think, Simon? I'd really like to know what '????' means to the mac...

Comment 12

[Simon Fraser [no longer active]]

Mac uses four character codes for file types/creators, using the Mac type 
'OSType', which is defined:

typedef unsigned long                   FourCharCode;
typedef FourCharCode                    OSType;

and the Mac compilers allow you to fill one of these using a 4-char literal 
enclosed in single quotes:
OSType foo = 'TEXT'
or
OSType bar = '????'

For cross-platform compatibility, these values should really be assigned using a 
macros that works on other platforms (taking endianness into account). The Mac 
headers have a macro for this:

Mac:
#define FOUR_CHAR_CODE(x)           (x) 

Windows:

#define FOUR_CHAR_CODE(x)       (((unsigned long) ((x) & 0x000000FF)) << 24) \
                                    | (((unsigned long) ((x) & 0x0000FF00)) << 8) 
\
                                    | (((unsigned long) ((x) & 0x00FF0000)) >> 8) 
\
                                    | (((unsigned long) ((x) & 0xFF000000)) >> 
24)

The code should then read
PRUint32 foo = FOUR_CHAR_CODE('????');

Comment 13

[Dan Rosen]

kick ass. but will those macros still prevent the trigraph-ish misinterpretation
of the ??'s?

Comment 14

[Simon Fraser [no longer active]]

I still don't understand how the linux compiler sees the line containing '????'.

Comment 15

[Mike Shaver (:shaver emeritus)]

How does that FOUR_CHAR_CODE macro help? It's going to expand to

(((unsigned long) (('????') & 0x000000FF)) << 24) ...

and you're back to square one.

#define FOUR_CHAR_CODE(a, b, c, d) ((a) << 24 | (b) << 16 | (c) << 8 | (d))

FOUR_CHAR_CODE('?', '?', '?', '?')

Comment 16

[Dan Rosen]

Simon: the compiler doesn't see it, the preprocessor does. The #ifdef XP_MAC
block gets dealt with by the preprocessor at the same time that the trigraph
does -- there's no precedence or ordering as far as I can tell.

Shaver: exactly. We should really have them not in quotes, and have the macros
be something like:

Mac:
#define FOUR_CHAR_CODE(a, b, c, d) 'a##b##c##d'
Other:
#define FOUR_CHAR_CODE(a, b, c, d) ((a) << 24 | (b) << 16 | (c) << 8 | (d))

and call FOUR_CHAR_CODE(?, ?, ?, ?), but that's still unwieldy, since you don't
really want to have to call FOUR_CHAR_CODE(T, E, X, T)... Perhaps since only mac
uses it, you could say:

Mac:
#define FOUR_CHAR_CODE(x) 'x'
Other:
#define FOUR_CHAR_CODE(x) #error blah blah blah

or something...

I'll open a separate bug for this issue.

Comment 17

[Dan Rosen]

Ok, bug 70631 is open for the mac '????' vs. ansi ??' trigraph issue.

Comment 18

[Jean-Francois Ducarroz]

You can replace '????' by '\077\077\077\077'. Does that will fix your problem?

Comment 19

[timeless]

fix was checked in 3.154

Comment 20

[Dan Rosen]

Timeless: don't fuck with my bugs. There are plenty more warnings on linux I
want to deal with. I should mention that if you wanted to get your patch
attaching "foo shadows previous local" in, here would be a good place...

Comment 21

[Dan Rosen]

Removing all CC's to avoid spamming the innocent, and turning this into a
meta-bug. I'll leave this as linux-only for the time being, since the warning
logs are generated on linux...

Comment 22

[Dan Rosen]

Let's try to get a lot of warnings killed for 1.0.

Comment 23

[Dan Rosen]

scc: There's no way to silence the pantloads of "was hidden by" warnings we get
by virtue of interface and implementation being different classes, is there?
That would cut down a lot of noise and make people more aware of their own
warnings...

Comment 24

[Scott Collins]

On compilers that support |using|, you bring the other names into scope in the
derived class.  I can provide an example if you desire.  This would eliminate
warnings on those platforms, and is the right thing to do in general (since the
original functions are statically hidden, and |using| un-hides them).

Comment 25

[Dan Rosen]

scc: Yes, I'd love to see an example -- it would be great if I can add onto some
macro referenced by the NS_DECL_(interface) macros, or some such...

Comment 26

[Bradley Baetz (:bbaetz)]

dr - we could just remove the -Woverloaded-virtual from the CFLAGS. The warning
isn't on by default, and its just hiding real warnings.

Since this is a meta bug - scc, is there a portable solution to the NS_FORWARD_
macros to fix the ## stuff? Currently gcc gives warnings because the pasting
don't give a preprocessing token, and again, it just hides real warnings.

Comment 27

[Dan Rosen]

->bbaetz. Feel free to WONTFIX this if you like. I've grown weary.

Comment 28

[Bradley Baetz (:bbaetz)]

I'm sick of this too, but I'm not going to fix it, mainly because some people
(and I can't remember who) said that -Woverloaded-virtual were useful.

See bug 90366 for a patch to get rid of the "not a preprocessor token" warnings.