Enable compiler warnings and fix them (#53)

* Enable warnings during build

* Fix all compiler warnings

* Keep old overloads but deprecate them

* Remove effectively unused variable

CMakeLists.txt

@@ -37,6 +37,8 @@ endif()
 
 # Ninja compiler output
 include(cmake/compiler_output.cmake)
+include(cmake/build_flags.cmake)
+k4edm4hep2lcioconv_set_compiler_flags()
 
 find_package(LCIO 2.20.1 REQUIRED)
 find_package(podio REQUIRED)

cmake/build_flags.cmake

@@ -0,0 +1,58 @@
+#---------------------------------------------------------------------------------------------------
+#---k4edm4hep2lcioconv_set_compiler_flags
+#
+#  Set compiler and linker flags
+#
+#---------------------------------------------------------------------------------------------------
+
+macro(k4edm4hep2lcioconv_set_compiler_flags)
+  include(CheckCXXCompilerFlag)
+
+  SET(COMPILER_FLAGS -fPIC -Wall -Wextra -Wpedantic)
+
+  # AppleClang/Clang specific warning flags
+  if(CMAKE_CXX_COMPILER_ID MATCHES "^(Apple)?Clang$")
+    set ( COMPILER_FLAGS ${COMPILER_FLAGS} -Winconsistent-missing-override -Wheader-hygiene )
+  endif()
+
+  FOREACH( FLAG ${COMPILER_FLAGS} )
+    ## need to replace the minus or plus signs from the variables, because it is passed
+    ## as a macro to g++ which causes a warning about no whitespace after macro
+    ## definition
+    STRING(REPLACE "-" "_" FLAG_WORD ${FLAG} )
+    STRING(REPLACE "+" "P" FLAG_WORD ${FLAG_WORD} )
+
+    CHECK_CXX_COMPILER_FLAG( "${FLAG}" CXX_FLAG_WORKS_${FLAG_WORD} )
+    IF( ${CXX_FLAG_WORKS_${FLAG_WORD}} )
+      message( STATUS "Adding ${FLAG} to CXX_FLAGS" )
+      SET ( CMAKE_CXX_FLAGS "${FLAG} ${CMAKE_CXX_FLAGS} ")
+    ELSE()
+      message( STATUS "NOT Adding ${FLAG} to CXX_FLAGS" )
+    ENDIF()
+  ENDFOREACH()
+
+  # resolve which linker we use
+  execute_process(COMMAND ${CMAKE_CXX_COMPILER} -Wl,--version OUTPUT_VARIABLE stdout ERROR_QUIET)
+  if("${stdout}" MATCHES "GNU ")
+    set(LINKER_TYPE "GNU")
+    message( STATUS "Detected GNU compatible linker" )
+  else()
+    execute_process(COMMAND ${CMAKE_CXX_COMPILER} -Wl,-v ERROR_VARIABLE stderr )
+    if(("${stderr}" MATCHES "PROGRAM:ld") AND ("${stderr}" MATCHES "PROJECT:ld64"))
+      set(LINKER_TYPE "APPLE")
+      message( STATUS "Detected Apple linker" )
+    else()
+      set(LINKER_TYPE "unknown")
+      message( STATUS "Detected unknown linker" )
+    endif()
+  endif()
+
+  if("${LINKER_TYPE}" STREQUAL "APPLE")
+    SET ( CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -Wl,-undefined,dynamic_lookup")
+  elseif("${LINKER_TYPE}" STREQUAL "GNU")
+    SET ( CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -Wl,--allow-shlib-undefined")
+  else()
+    MESSAGE( WARNING "No known linker (GNU or Apple) has been detected, pass no flags to linker" )
+  endif()
+
+endmacro(k4edm4hep2lcioconv_set_compiler_flags)

k4EDM4hep2LcioConv/include/k4EDM4hep2LcioConv/k4EDM4hep2LcioConv.h

@@ -123,23 +123,36 @@ namespace EDM4hep2LCIOConv {
     const edm4hep::RawCalorimeterHitCollection* const rawcalohit_coll,
     RawCaloHitMapT& raw_calo_hits_vec);
 
-  template<typename SimCaloHitMapT, typename MCParticleMapT>
+  template<typename SimCaloHitMapT>
   lcio::LCCollectionVec* convSimCalorimeterHits(
+    const edm4hep::SimCalorimeterHitCollection* const simcalohit_coll,
+    const std::string& cellIDstr,
+    SimCaloHitMapT& sim_calo_hits_vec);
+
+  template<typename SimCaloHitMapT, typename MCParticleMapT>
+  [[deprecated("remove MCParticleMap argument since it is unused")]] lcio::LCCollectionVec* convSimCalorimeterHits(
     const edm4hep::SimCalorimeterHitCollection* const simcalohit_coll,
     const std::string& cellIDstr,
     SimCaloHitMapT& sim_calo_hits_vec,
-    const MCParticleMapT& mcparticles);
+    const MCParticleMapT&)
+  {
+    return convSimCalorimeterHits(simcalohit_coll, cellIDstr, sim_calo_hits_vec);
+  }
 
   template<typename TPCHitMapT>
   lcio::LCCollectionVec* convTPCHits(
     const edm4hep::RawTimeSeriesCollection* const tpchit_coll,
     TPCHitMapT& tpc_hits_vec);
 
+  template<typename ClusterMapT>
+  lcio::LCCollectionVec* convClusters(const edm4hep::ClusterCollection* const cluster_coll, ClusterMapT& cluster_vec);
+
   template<typename ClusterMapT, typename CaloHitMapT>
-  lcio::LCCollectionVec* convClusters(
-    const edm4hep::ClusterCollection* const cluster_coll,
-    ClusterMapT& cluster_vec,
-    const CaloHitMapT& calohits_vec);
+  [[deprecated("remove CaloHitMap argument since it is unused")]] lcio::LCCollectionVec*
+  convClusters(const edm4hep::ClusterCollection* const cluster_coll, ClusterMapT& cluster_vec, const CaloHitMapT&)
+  {
+    return convClusters(cluster_coll, cluster_vec);
+  }
 
   template<typename VertexMapT, typename RecoPartMapT>
   lcio::LCCollectionVec* convVertices(

k4EDM4hep2LcioConv/include/k4EDM4hep2LcioConv/k4EDM4hep2LcioConv.ipp

@@ -22,7 +22,7 @@ namespace EDM4hep2LCIOConv {
         // The Type of the Tracks need to be set bitwise in LCIO since the setType(int) function is private for the LCIO
         // TrackImpl and only a setTypeBit(bitnumber) function can be used to set the Type bit by bit.
         int type = edm_tr.getType();
-        for (int i = 0; i < sizeof(int) * 8; i++) {
+        for (auto i = 0u; i < sizeof(int) * 8; i++) {
           lcio_tr->setTypeBit(i, type & (1 << i));
         }
         lcio_tr->setChi2(edm_tr.getChi2());
@@ -33,15 +33,15 @@ namespace EDM4hep2LCIOConv {
 
         // Loop over the hit Numbers in the track
         lcio_tr->subdetectorHitNumbers().resize(edm_tr.subdetectorHitNumbers_size());
-        for (int i = 0; i < edm_tr.subdetectorHitNumbers_size(); ++i) {
+        for (auto i = 0u; i < edm_tr.subdetectorHitNumbers_size(); ++i) {
           lcio_tr->subdetectorHitNumbers()[i] = edm_tr.getSubdetectorHitNumbers(i);
         }
 
         // Pad until 50 hitnumbers are resized
         const int hit_number_limit = 50;
         if (edm_tr.subdetectorHitNumbers_size() < hit_number_limit) {
           lcio_tr->subdetectorHitNumbers().resize(hit_number_limit);
-          for (int i = edm_tr.subdetectorHitNumbers_size(); i < hit_number_limit; ++i) {
+          for (auto i = edm_tr.subdetectorHitNumbers_size(); i < hit_number_limit; ++i) {
             lcio_tr->subdetectorHitNumbers()[i] = 0;
           }
         }
@@ -326,12 +326,11 @@ namespace EDM4hep2LCIOConv {
   // Convert EDM4hep Sim Calorimeter Hits to LCIO
   // Add converted LCIO ptr and original EDM4hep collection to vector of pairs
   // Add converted LCIO Collection Vector to LCIO event
-  template<typename SimCaloHitMapT, typename MCParticleMapT>
+  template<typename SimCaloHitMapT>
   lcio::LCCollectionVec* convSimCalorimeterHits(
     const edm4hep::SimCalorimeterHitCollection* const simcalohit_coll,
     const std::string& cellIDstr,
-    SimCaloHitMapT& sim_calo_hits_vec,
-    const MCParticleMapT& mcparticles)
+    SimCaloHitMapT& sim_calo_hits_vec)
   {
     auto* simcalohits = new lcio::LCCollectionVec(lcio::LCIO::SIMCALORIMETERHIT);
 
@@ -387,7 +386,7 @@ namespace EDM4hep2LCIOConv {
         lcio_tpchit->setQuality(edm_tpchit.getQuality());
 
         std::vector<int> rawdata;
-        for (int i = 0; i < edm_tpchit.adcCounts_size(); ++i) {
+        for (auto i = 0u; i < edm_tpchit.adcCounts_size(); ++i) {
           rawdata.push_back(edm_tpchit.getAdcCounts(i));
         }
 
@@ -407,11 +406,8 @@ namespace EDM4hep2LCIOConv {
   // Convert EDM4hep Clusters to LCIO
   // Add converted LCIO ptr and original EDM4hep collection to vector of pairs
   // Add converted LCIO Collection Vector to LCIO event
-  template<typename ClusterMapT, typename CaloHitMapT>
-  lcio::LCCollectionVec* convClusters(
-    const edm4hep::ClusterCollection* const cluster_coll,
-    ClusterMapT& cluster_vec,
-    const CaloHitMapT& calohits_vec)
+  template<typename ClusterMapT>
+  lcio::LCCollectionVec* convClusters(const edm4hep::ClusterCollection* const cluster_coll, ClusterMapT& cluster_vec)
   {
     auto* clusters = new lcio::LCCollectionVec(lcio::LCIO::CLUSTER);
 
@@ -421,7 +417,7 @@ namespace EDM4hep2LCIOConv {
         auto* lcio_cluster = new lcio::ClusterImpl();
 
         std::bitset<sizeof(uint32_t)> type_bits = edm_cluster.getType();
-        for (int j = 0; j < sizeof(uint32_t); j++) {
+        for (auto j = 0u; j < sizeof(uint32_t); j++) {
           lcio_cluster->setTypeBit(j, (type_bits[j] == 0) ? false : true);
         }
         lcio_cluster->setEnergy(edm_cluster.getEnergy());
@@ -578,7 +574,7 @@ namespace EDM4hep2LCIOConv {
             is_same = is_same && (lcio_pid->getPDG() == edm_pid_used.getPDG());
             is_same = is_same && (lcio_pid->getLikelihood() == edm_pid_used.getLikelihood());
             is_same = is_same && (lcio_pid->getAlgorithmType() == edm_pid_used.getAlgorithmType());
-            for (int i = 0; i < edm_pid_used.parameters_size(); ++i) {
+            for (auto i = 0u; i < edm_pid_used.parameters_size(); ++i) {
               is_same = is_same && (edm_pid_used.getParameters(i) == lcio_pid->getParameters()[i]);
             }
             if (is_same) {
@@ -800,7 +796,7 @@ namespace EDM4hep2LCIOConv {
     // collection(s) should be converted!
     for (auto& [lcio_sch, edm_sch] : update_pairs.simCaloHits) {
       // add associated Contributions (MCParticles)
-      for (int i = 0; i < edm_sch.contributions_size(); ++i) {
+      for (auto i = 0u; i < edm_sch.contributions_size(); ++i) {
         const auto& contrib = edm_sch.getContributions(i);
         if (not contrib.isAvailable()) {
           // We need a logging library independent of Gaudi for this!
@@ -810,7 +806,6 @@ namespace EDM4hep2LCIOConv {
         auto edm_contrib_mcp = contrib.getParticle();
         std::array<float, 3> step_position {
           contrib.getStepPosition()[0], contrib.getStepPosition()[1], contrib.getStepPosition()[2]};
-        bool mcp_found = false;
         EVENT::MCParticle* lcio_mcp = nullptr;
         if (edm_contrib_mcp.isAvailable()) {
           // if we have the MCParticle we look for its partner

k4EDM4hep2LcioConv/include/k4EDM4hep2LcioConv/k4Lcio2EDM4hepConv.ipp

@@ -8,31 +8,31 @@ namespace LCIO2EDM4hepConv {
     // handle srting params
     EVENT::StringVec keys;
     const auto stringKeys = params.getStringKeys(keys);
-    for (int i = 0; i < stringKeys.size(); i++) {
+    for (auto i = 0u; i < stringKeys.size(); i++) {
       EVENT::StringVec sValues;
       const auto stringVals = params.getStringVals(stringKeys[i], sValues);
       event.putParameter(stringKeys[i], stringVals);
     }
     // handle float params
     EVENT::StringVec fkeys;
     const auto floatKeys = params.getFloatKeys(fkeys);
-    for (int i = 0; i < floatKeys.size(); i++) {
+    for (auto i = 0u; i < floatKeys.size(); i++) {
       EVENT::FloatVec fValues;
       const auto floatVals = params.getFloatVals(floatKeys[i], fValues);
       event.putParameter(floatKeys[i], floatVals);
     }
     // handle int params
     EVENT::StringVec ikeys;
     const auto intKeys = params.getIntKeys(ikeys);
-    for (int i = 0; i < intKeys.size(); i++) {
+    for (auto i = 0u; i < intKeys.size(); i++) {
       EVENT::IntVec iValues;
       const auto intVals = params.getIntVals(intKeys[i], iValues);
       event.putParameter(intKeys[i], intVals);
     }
     // handle double params
     EVENT::StringVec dkeys;
     const auto dKeys = params.getDoubleKeys(dkeys);
-    for (int i = 0; i < dKeys.size(); i++) {
+    for (auto i = 0u; i < dKeys.size(); i++) {
       EVENT::DoubleVec dValues;
       const auto dVals = params.getDoubleVals(dKeys[i], dValues);
       event.putParameter(dKeys[i], dVals);
@@ -587,7 +587,7 @@ namespace LCIO2EDM4hepConv {
     auto contrCollection = std::make_unique<edm4hep::CaloHitContributionCollection>();
     for (auto& [lcioHit, edmHit] : SimCaloHitMap) {
       auto NMCParticle = lcioHit->getNMCParticles();
-      for (unsigned j = 0; j < NMCParticle; j++) {
+      for (int j = 0; j < NMCParticle; j++) {
         auto edm_contr = contrCollection->create();
         edmHit.addToContributions(edm_contr);
 
@@ -613,9 +613,7 @@ namespace LCIO2EDM4hepConv {
   template<typename MCParticleMapT, typename MCParticleLookupMapT>
   void resolveRelationsMCParticles(MCParticleMapT& mcparticlesMap, const MCParticleLookupMapT& lookupMap)
   {
-    int edmnum = 1;
     for (auto& [lcio, edm] : mcparticlesMap) {
-      edmnum++;
       auto daughters = lcio->getDaughters();
       auto parents = lcio->getParents();
 
@@ -680,9 +678,7 @@ namespace LCIO2EDM4hepConv {
     const ClusterMapT& clusterMap,
     const TrackMapT& tracksMap)
   {
-    int edmnum = 1;
     for (auto& [lcio, edm] : recoparticlesMap) {
-      edmnum++;
 
       const auto& vertex = lcio->getStartVertex();
       if (vertex != nullptr) {

k4EDM4hep2LcioConv/src/k4EDM4hep2LcioConv.cpp

@@ -62,15 +62,15 @@ namespace EDM4hep2LCIOConv {
         lcioEvent->addCollection(lcColl, name);
       }
       else if (auto coll = dynamic_cast<const edm4hep::SimCalorimeterHitCollection*>(edmCollection)) {
-        auto lcColl = convSimCalorimeterHits(coll, cellIDStr, objectMappings.simCaloHits, objectMappings.mcParticles);
+        auto lcColl = convSimCalorimeterHits(coll, cellIDStr, objectMappings.simCaloHits);
         lcioEvent->addCollection(lcColl, name);
       }
       else if (auto coll = dynamic_cast<const edm4hep::RawTimeSeriesCollection*>(edmCollection)) {
         auto lcColl = convTPCHits(coll, objectMappings.tpcHits);
         lcioEvent->addCollection(lcColl, name);
       }
       else if (auto coll = dynamic_cast<const edm4hep::ClusterCollection*>(edmCollection)) {
-        auto lcColl = convClusters(coll, objectMappings.clusters, objectMappings.caloHits);
+        auto lcColl = convClusters(coll, objectMappings.clusters);
         lcioEvent->addCollection(lcColl, name);
       }
       else if (auto coll = dynamic_cast<const edm4hep::VertexCollection*>(edmCollection)) {
@@ -89,7 +89,7 @@ namespace EDM4hep2LCIOConv {
       else if (auto coll = dynamic_cast<const edm4hep::EventHeaderCollection*>(edmCollection)) {
         convEventHeader(coll, lcioEvent.get());
       }
-      else if (auto coll = dynamic_cast<const edm4hep::CaloHitContributionCollection*>(edmCollection)) {
+      else if (dynamic_cast<const edm4hep::CaloHitContributionCollection*>(edmCollection)) {
         // "converted" as part of FillMissingCollectoins at the end
         continue;
       }

standalone/lcio2edm4hep.cpp

@@ -95,7 +95,7 @@ ParsedArgs parseArgs(std::vector<std::string> argv)
     std::exit(0);
   }
 
-  auto argc = argv.size();
+  int argc = argv.size();
   if (argc < 3 || argc > 7) {
     printUsageAndExit();
   }
@@ -166,7 +166,7 @@ int main(int argc, char* argv[])
 
   podio::ROOTWriter writer(args.outputFile);
 
-  for (auto j = 0u; j < lcreader->getNumberOfRuns(); ++j) {
+  for (int j = 0; j < lcreader->getNumberOfRuns(); ++j) {
     if (j % 1 == 0) {
       std::cout << "processing RunHeader: " << j << std::endl;
     }
@@ -177,7 +177,7 @@ int main(int argc, char* argv[])
   }
 
   const int nEvt = args.nEvents > 0 ? args.nEvents : lcreader->getNumberOfEvents();
-  for (auto i = 0u; i < nEvt; ++i) {
+  for (int i = 0; i < nEvt; ++i) {
     if (i % 10 == 0) {
       std::cout << "processing Event: " << i << std::endl;
     }

tests/compare_contents.cpp

@@ -68,7 +68,7 @@ int main(int argc, char* argv[])
         return 1;
       }
 
-      if (edmEvent.get(name)->size() != lcioColl->getNumberOfElements()) {
+      if (edmEvent.get(name)->size() != (unsigned) lcioColl->getNumberOfElements()) {
         std::cerr << "Collection " << name << " has different sizes. LCIO: " << lcioColl->getNumberOfElements()
                   << ", EDM4hep: " << coll->size() << std::endl;
         return 1;

tests/edm4hep_to_lcio.cpp

@@ -28,7 +28,7 @@ int main()
     }
     try {
       const auto* lcColl = lcioEvent->getCollection(name);
-      if (lcColl->getNumberOfElements() != edmColl->size()) {
+      if ((unsigned) lcColl->getNumberOfElements() != edmColl->size()) {
         std::cerr << "Collection " << name << " has different sizes. EDM4hep: " << edmColl->size()
                   << ", LCIO: " << lcColl->getNumberOfElements() << std::endl;
         return 1;