LDMX-Software
Compiler warnings
Describe the bug In the idea of having clean compiler logs, here are a few more warnings I see:
Type "control reaches end of non-void function"
/Users/tav/Documents/1Research/LDMX/ldmx-sw/DetDescr/src/DetDescr/HcalGeometry.cxx: In member function 'std::vector<double> ldmx::HcalGeometry::rotateGlobalToLocalBarPosition(const std::vector<double>&, const ldmx::HcalID&) const':
/Users/tav/Documents/1Research/LDMX/ldmx-sw/DetDescr/src/DetDescr/HcalGeometry.cxx:77:1: warning: control reaches end of non-void function [-Wreturn-type]
77 | }
| ^
/Users/tav/Documents/1Research/LDMX/ldmx-sw/DetDescr/src/DetDescr/HcalGeometry.cxx: In member function 'ldmx::HcalGeometry::ScintillatorOrientation ldmx::HcalGeometry::getScintillatorOrientation(ldmx::HcalID) const':
/Users/tav/Documents/1Research/LDMX/ldmx-sw/DetDescr/src/DetDescr/HcalGeometry.cxx:128:1: warning: control reaches end of non-void function [-Wreturn-type]
128 | }
| ^
Type " declaration of 'm' shadows a global declaration "
/Users/tav/Documents/1Research/LDMX/ldmx-sw/SimCore/G4DarkBreM/app/simulate.cxx: In constructor 'g4db::example::Hunk::Hunk(double, const string&)':
/Users/tav/Documents/1Research/LDMX/ldmx-sw/SimCore/G4DarkBreM/app/simulate.cxx:121:37: warning: declaration of 'm' shadows a global declaration [-Wshadow]
121 | Hunk(double d, const std::string& m)
| ~~~~~~~~~~~~~~~~~~~^
In file included from /usr/local/include/Geant4/CLHEP/Units/PhysicalConstants.h:42,
from /usr/local/include/Geant4/G4ParticleDefinition.hh:58,
from /usr/local/include/Geant4/G4ParticleTable.hh:58,
from /usr/local/include/Geant4/G4VUserPhysicsList.hh:92,
from /usr/local/include/Geant4/G4VModularPhysicsList.hh:59,
from /usr/local/include/Geant4/QBBC.hh:42,
from /Users/tav/Documents/1Research/LDMX/ldmx-sw/SimCore/G4DarkBreM/app/simulate.cxx:11:
/usr/local/include/Geant4/CLHEP/Units/SystemOfUnits.h:108:23: note: shadowed declaration is here
108 | static const double m = meter;
| ^
Type "deprecated"
/Users/tav/Documents/1Research/LDMX/ldmx-sw/Tools/src/Tools/ONNXRuntime.cxx: In constructor 'ldmx::Ort::ONNXRuntime::ONNXRuntime(const string&, const Ort::SessionOptions*)':
/Users/tav/Documents/1Research/LDMX/ldmx-sw/Tools/src/Tools/ONNXRuntime.cxx:70:30: warning: 'void Ort::detail::TensorTypeAndShapeInfoImpl<T>::GetDimensions(int64_t*, size_t) const [with T = Ort::detail::Unowned<const OrtTensorTypeAndShapeInfo>; int64_t = long int; size_t = long unsigned int]' is deprecated: use GetShape() [-Wdeprecated-declarations]
70 | tensor_info.GetDimensions(input_node_dims_[input_name].data(), num_dims);
| ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /Users/tav/Documents/1Research/LDMX/ldmx-sw/Tools/include/Tools/ONNXRuntime.h:10,
from /Users/tav/Documents/1Research/LDMX/ldmx-sw/Tools/src/Tools/ONNXRuntime.cxx:2:
/usr/local/include/onnxruntime_cxx_api.h:858:41: note: declared here
858 | [[deprecated("use GetShape()")]] void GetDimensions(int64_t* values, size_t values_count) const; ///< Wraps OrtApi::GetDimensions
| ^~~~~~~~~~~~~
/Users/tav/Documents/1Research/LDMX/ldmx-sw/Tools/src/Tools/ONNXRuntime.cxx:92:30: warning: 'void Ort::detail::TensorTypeAndShapeInfoImpl<T>::GetDimensions(int64_t*, size_t) const [with T = Ort::detail::Unowned<const OrtTensorTypeAndShapeInfo>; int64_t = long int; size_t = long unsigned int]' is deprecated: use GetShape() [-Wdeprecated-declarations]
92 | tensor_info.GetDimensions(output_node_dims_[output_name].data(), num_dims);
| ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /usr/local/include/onnxruntime_cxx_api.h:2088,
from /Users/tav/Documents/1Research/LDMX/ldmx-sw/Tools/include/Tools/ONNXRuntime.h:10,
from /Users/tav/Documents/1Research/LDMX/ldmx-sw/Tools/src/Tools/ONNXRuntime.cxx:2:
/usr/local/include/onnxruntime_cxx_inline.h:1061:13: note: declared here
1061 | inline void TensorTypeAndShapeInfoImpl<T>::GetDimensions(int64_t* values, size_t values_count) const {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Additional context
Similar issue at https://github.com/LDMX-Software/ldmx-sw/issues/1199
- For the first one, should we just return empty vector (
return {};outside the switch statements? - For the 2nd, hmm maybe better to avoid one letter variable names?
- For the 3rrd, I guess we should use
GetShape(), but I have no idea if it's a one to one translation (meaning, will the same arguments be needed)?
Thanks for making this issue! Cleaning up our builds and allowing us to start making use of more static analysis as part of the CI is an important goal. There's a overarching issue for these things in https://github.com/LDMX-Software/ldmx-sw/issues/1176. I'll make a list of related issues in there and link this one
Since https://github.com/LDMX-Software/docker/pull/67 and https://github.com/LDMX-Software/cmake/pull/19 we have some additional tools in the development container that can give additional static analysis. For compiler warnings, there are some CMake flags you can enable to add additional warnings that you can see in https://github.com/LDMX-Software/cmake/blob/trunk/CompilerWarnings.cmake
You can also build ldmx-sw with the clang++ compiler instead of GCC. There will be some warnings that clang catches that GCC doesn't and vice versa and sometimes the different compilers give better/worse feedback (see e.g. the warning in https://github.com/LDMX-Software/Hcal/pull/70 and https://github.com/LDMX-Software/Framework/issues/70).
Finally, for memory-issues we also have https://github.com/LDMX-Software/ldmx-sw/issues/1172 so if you find anything in that realm it might be discussed in there already.
w.r.t. the first one here, I think reaching the end of these functions should clearly signal a bug. From looking at the code, it isn't immediately obvious to me if there is a way to reach the end of those functions with valid input so I would probably suggest raising an exception.
I should say that it isn't obvious to me there's a way to trigger the end of those functions doesn't mean that the warning is silly, there have been similar problems caught by this warning before https://github.com/LDMX-Software/ldmx-sw/pull/1182/commits/ebbe456cb6b2e6e102051aecf56f301ac0dc7be7
For building with clang, you may have to manually merge https://github.com/LDMX-Software/TrigScint/pull/54 first. This reminded me that we should probably get around to merging it anyways.
For the 3rd...
I also am unsure if GetShape is just a drop-in replacement, but there is an additional complexity. The v3 development container images have ONNX v1.2 which does not have GetShape while the v4 images have ONNX v1.15 which issues this deprecation warning. The good news is that we pin the ONNX version so we can pretty safely assume that GetDimension won't disppear on us despite this deprecation warning. The bad news is that if we want to remove this deprecation warning, we'd need to either completely drop v3 images (not something I'm willing to do) or do some ugly preprocessor stuff to align the APIs like I do with getting names:
https://github.com/LDMX-Software/ldmx-sw/blob/ddcc4daff5c70b45a392c0dfce4dfdd26ebc3e51/Tools/src/Tools/ONNXRuntime.cxx#L15-L37
The other option is to disable the warning, but I would prefer doing some #pragma's if it isnt happening in a lot of places
https://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Pragmas.html
With some comment referencing this issue of course
hey @tomeichlersmith your point in https://github.com/LDMX-Software/ldmx-sw/issues/1208#issuecomment-1723386035 is now relevant again since you did drop v3 images recently, right?
So besides ONNX warning, I see this one
In file included from /usr/include/boost/smart_ptr/detail/sp_thread_sleep.hpp:22,
from /usr/include/boost/smart_ptr/detail/yield_k.hpp:23,
from /usr/include/boost/smart_ptr/detail/spinlock_gcc_atomic.hpp:14,
from /usr/include/boost/smart_ptr/detail/spinlock.hpp:42,
from /usr/include/boost/smart_ptr/detail/spinlock_pool.hpp:25,
from /usr/include/boost/smart_ptr/shared_ptr.hpp:29,
from /usr/include/boost/shared_ptr.hpp:17,
from /usr/include/boost/python/converter/shared_ptr_to_python.hpp:12,
from /usr/include/boost/python/converter/arg_to_python.hpp:15,
from /usr/include/boost/python/call.hpp:15,
from /usr/include/boost/python/object_core.hpp:14,
from /usr/include/boost/python/args.hpp:22,
from /usr/include/boost/python.hpp:11,
from /Users/tav/Documents/1Research/LDMX/BugFixes/ldmx-sw/DetDescr/src/DetDescr/DetectorIDBindings.cxx:2:
/usr/include/boost/bind.hpp:36:1: note: '#pragma message: The practice of declaring the Bind placeholders (_1, _2, ...) in the global namespace is deprecated. Please use <boost/bind/bind.hpp> + using namespace boost::placeholders, or define BOOST_BIND_GLOBAL_PLACEHOLDERS to retain the current behavior.'
36 | BOOST_PRAGMA_MESSAGE(
| ^~~~~~~~~~~~~~~~~~~~
/usr/include/boost/detail/iterator.hpp:13:1: note: '#pragma message: This header is deprecated. Use <iterator> instead.'
13 | BOOST_HEADER_DEPRECATED("<iterator>")
| ^~~~~~~~~~~~~~~~~~~~~~~
I dont know if I can do anything with this, @tomeichlersmith @EinarElen ?
I would probably just add the #define that is referenced and add a comment in the docker repo by the dependencies to check this whenever we update boost