On Wed, Jul 02, 2014 at 09:28:03PM +0200, Jan Hubicka wrote: > > > > 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.
I was sort of suggesting that part I quoted, but its not great either. maybe note: First differing member of $whatever defined here ? > > > 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. h, that seems like a kind of confusing thing to write. > 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??? absolutely ;) > > > + 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: yeah, should have read more carefully. Trev > ../../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