>
> I can't find the code for the SampleFormat thing, but the rest of them
> look like ODR violations to me.
I think it is some define renaming the field, I was also puzled by this one.
>
> > /aux/hubicka/firefox/netwerk/sctp/datachannel/DataChannel.h:64:0: warning:
> > field ‘mSpa’ (of type ‘struct BufferedMsg’) violates one definition rule
> > [-Wodr]
> > struct sctp_sendv_spa *mSpa;
> > ^
> > ../../../../dist/include/mozilla/net/DataChannel.h:64:0: note: a field of
> > same name but different type is defined in another translation unit
> > /aux/hubicka/firefox/netwerk/streamconv/converters/nsMultiMixedConv.cpp:466:0:
> > warning: field ‘mBuffer’ (of type ‘struct AutoFree’) violates one
> > definition rule [-Wodr]
> > char *mBuffer;
> > ^
> > /aux/hubicka/firefox/dom/base/nsJSEnvironment.cpp:307:0: note: a field with
> > different name is defined in another translation unit
>
> it seems to me this doesn't get at the real issue that the type names
> are the same but the fields are different. maybe "a type of the same
> name with different fields defined here"?
This is what I print when I see name mismatch. It usually means completely
different structure/field.
It speaks of fields names rather than type names. Better wording is welcome.
>
> > void *mPtr;
> > ^
> > lto1: note: Conflicting compilation units:
> > /aux/hubicka/firefox/dom/base/nsJSEnvironment.cpp and
> > /aux/hubicka/build-firefox491-inst4/netwerk/streamconv/converters/Unified_cpp_netwerk_streamconv_converters0.cpp
> > ../../dist/include/zipstruct.h:47:25: warning: field ‘MOZ_Z_crc32’ (of type
> > ‘struct ZipCentral_’) violates one definition rule [-Wodr]
> > ../../dist/include/zipstruct.h:47:0: note: a field with different name is
> > defined in another translation unit
>
> is it worth poking into why this case doesn't have the caret stuff?
>
> > + /* In Firefox it is a common bug to have same types but in
> > + different namespaces. Be a bit more informative on
> > + this. */
>
> hm? I make no claim to being a spec lawyer, but I thought something like
> namespace foo {
> struct bar { int x; };
> }
>
> namespace baz {
> struct bar { float b; };
> }
> was legal? or are you refering to some other construct?
This is legal as long as you don't define
struct foo {
struct bar a;
}
Where foo is same name but bar is once from foo and once from baz.
There are cases where "struct bar" is in an namespace in one unit, but
it is toplevel in another.
>
> > + if (TYPE_CONTEXT (t1) && TYPE_CONTEXT (t2)
> > + && (((TREE_CODE (TYPE_CONTEXT (t1)) == NAMESPACE_DECL)
> > + != (TREE_CODE (TYPE_CONTEXT (t2)) == NAMESPACE_DECL))
> > + || (TREE_CODE (TYPE_CONTEXT (t1)) == NAMESPACE_DECL
> > + && (DECL_NAME (TYPE_CONTEXT (t1)) !=
> > + DECL_NAME (TYPE_CONTEXT (t2))))))
>
> imho that would be a lot more readable with some temporary variables,
> but shrug
What? stepping away from old-school GCC writting style???
>
> > + inform (DECL_SOURCE_LOCATION (TYPE_NAME (t1)),
> > + "type %qT should match type %qT but is defined "
> > + "in different namespace ",
> > + t1, t2);
> > + else
> > + inform (DECL_SOURCE_LOCATION (TYPE_NAME (t1)),
> > + "type %qT should match type %qT",
> > + t1, t2);
> > + inform (DECL_SOURCE_LOCATION (TYPE_NAME (t2)),
> > + "the incompatible type is defined here");
>
> I didn't see any of these come up in the list of warnings at the
> beginning of your mail?
I get it here:
../../dist/include/mp4_demuxer/audio_decoder_config.h:112:0: warning: field
�~@~Xsample_format_�~@~Y (of type �~@~Xstruct AudioDecoderConfig�~@~Y) violates
one definition rule [-Wodr]
../../../dist/include/mp4_demuxer/audio_decoder_config.h:112:0: note: a field
of same name but different type is defined in another translation unit
SampleFormat sample_format_;
^
../../dist/include/mp4_demuxer/audio_decoder_config.h:41:0: note: type
�~@~XSampleFormat�~@~Y should match type �~@~XAVSampleFormat�~@~Y
../../../dist/include/mp4_demuxer/audio_decoder_config.h:41:0: note: the
incompatible type is defined here
enum SampleFormat {
Which suggest that autdio_decoder_config.h is sometimes included with define
turing SampleFormat to XSampleFormat
and sometimes to XAVSampleFormat. Not the most readable warning around, but
probably can't do much better.
The carret diagnostics should not get confused by relative names - I was
thinking about turing all locations into absolute names, but
that is probably not coolest thing either.
>
> > @@ -445,31 +955,23 @@ add_type_duplicate (odr_type val, tree t
> > {
> > bool merge = true;
> > bool base_mismatch = false;
> > - bool warned = 0;
> > unsigned int i,j;
> > + bool warned = 0;
>
> false?
Yep.
Thanks!
I will prepare updated patch with all the comments.
Honza
>
> Trev
