shift-count-overflow warnings in epa portable_oarchive with Clang compiler

[drbenmorgan]

In fixing the warnings on Clang in #61, the major unsolved one is, to give a canonical example:

[49/1178] Building CXX object source/CMakeFiles/mctools-test_signal_shape_builder.dir/bxmctools/testing/test_signal_shape_builder.cxx.o
In file included from ../source/bxmctools/testing/test_signal_shape_builder.cxx:15:
In file included from ../source/bxdatatools/include/datatools/io_factory.h:109:
In file included from ../source/bxdatatools/include/datatools/archives_list.h:55:
In file included from ../source/bxdatatools/include/datatools/portable_archives_support.h:46:
../source/bxepa/include/boost/archive/portable_oarchive.hpp:287:43: warning: shift count >= width of type [-Wshift-count-overflow]
                                do { temp >>= CHAR_BIT; ++size; } while (temp != 0 && temp != (T)-1);
                                          ^   ~~~~~~~~
../source/bxepa/include/boost/archive/portable_oarchive.hpp:384:25: note: in instantiation of function template specialization 'boost::archive::portable_oarchive::save<unsigned char>' requested here
                        save((typename boost::uint_t<sizeof(T)*CHAR_BIT>::least)(t));

There are a huge number of these so I don't have a full picture, but they seem to occur for the specializations of boost::archive::portable_oarchive on unsigned char and signed char. I haven't found a different case, but there are thousands of lines here...

That it appears limited to those two types should hopefully help to track it down, but it needs fixing otherwise Bayeux's basically un-developable due to other errors and warnings being impossible to track.

There's a related bug which is that Bayeux isn't respecting any CMAKE_CXX_FLAGS supplied by the user, but I haven't tracked that down as yet, but will add an Issue/PR when I do.

[fmauger]

I see no other solution than pragma-muting this warning explicitly for (un)signed char types. We could also specialize the save method for these types, forcing "size=1" but this would break the legacy code and make it (a little) more complex.

[fmauger]

For the second point, I guess this prevents to activate manually some specific flags to help diagnostics... this should be fixed sure!

[fmauger]

It seems GCC 9.3 does have the same sensitivity than Clang when "-Wshift-count-overflow" is activated. So far I was not able to reproduce the eap's warnings...

[fmauger]

So I have pushed an attempt to mute the warning with some pragmas. Let me know if it works on your side.

Considering the code more carefully, the line to blame makes a correct job and it seems to me that -Wshift-count-overflow is quite touchy. I would have understood for the <<= operator where left shifting may accidentally induce some lossy result. But this completely depends on the context in terms of bit manipulation. Right shifting is by essence lossy (discard LSB) and IMHO under the responsibility of the programmer. However compiling the following lines with gcc results in the same warnings for b and d:

  auto a = 1 << 31;
  auto b = 1 << 32;
  auto c = 1UL << 32;
  auto d = 1 >> 31;
  auto e = 1 >> 32;
  auto f = 1UL >> 32;

That obviously makes sense (symmetrical behaviour for << and >> ops) but I think we are here in the grey zone. Maybe it would be safer to follow the hint raised from this kind of warning. But my guess is that a lot of low level bit manipulation code (C fashioned) should be rewritten then.

[drbenmorgan]

Thanks @fmauger, if it's truly an overly specific warning then the pragma's the right way to go! The GCC warnings may not have appeared before (if this was in Bayeux rather than independent tests), as the CMAKE_CXX_FLAGS are getting overwritten by the inclusion of the Geant4 "use file". That's a separate issue which I'll have a look at and propose a PR.

[fmauger]

Ok. In fact, as written before, I have tried to enforce the warning with GCC but nothing was emitted despite the fact, from the code snippet above, I'm rather sure GCC should tag it also. May be I've hacked CMake at the wrong place.

We can let this issue open until you find a fix for this CMAKE_CXX_FLAGS issue.

[drbenmorgan]

As #64 is merged, closing this!