Part 2 of static analysis code improvements (#1453)

* Exclude 3rd party from static analysis checks
* Static analysis improvement for Biasing and Conditions
* Static analysis improvement for DQM and DetDescr
* Static analysis improvement for Ecal
* Static analysis improvement for Framework
* Static analysis improvement for Hcal
* Static analysis improvement for Packing, Tools and Recon
* Static analysis improvement for Tracking
* Static analysis improvement for TrigScint and Trigger
* Delete unused code in Ecal, Frameworks, Biasing as agreed with Tom
* Delete unused code in HCAL as agreed with Einar
* Delete unused code in Tracking, as agreed with Matt

Biasing/include/Biasing/PhotoNuclearTopologyFilters.h

@@ -91,7 +91,7 @@ class PhotoNuclearTopologyFilter : public simcore::UserAction {
    *
    */
   constexpr bool skipCountingParticle(const int pdgcode) const {
-    return !(pdgcode < 10000 || count_light_ions_ && isLightIon(pdgcode));
+    return !(pdgcode < 10000 || (count_light_ions_ && isLightIon(pdgcode)));
   }
 
   constexpr bool isNeutron(const int pdgID) const { return pdgID == 2112; }

Biasing/include/Biasing/TargetProcessFilter.h

@@ -32,7 +32,7 @@ class TargetProcessFilter : public simcore::UserAction {
                       framework::config::Parameters &parameters);
 
   /// Destructor
-  ~TargetProcessFilter();
+  virtual ~TargetProcessFilter() = default;
 
   /**
    * Implementmthe stepping action which performs the target volume biasing.
@@ -65,9 +65,6 @@ class TargetProcessFilter : public simcore::UserAction {
   /** Pointer to the current track being processed. */
   G4Track *currentTrack_{nullptr};
 
-  /** Flag indicating if the reaction of intereset occurred. */
-  bool reactionOccurred_{false};
-
   /// The process to bias
   std::string process_{""};
 };

Biasing/src/Biasing/PhotoNuclearTopologyFilters.cxx

@@ -44,9 +44,9 @@ bool SingleNeutronFilter::rejectEvent(
 PhotoNuclearTopologyFilter::PhotoNuclearTopologyFilter(
     const std::string& name, framework::config::Parameters& parameters)
     : UserAction{name, parameters},
+      count_light_ions_{parameters.getParameter<bool>("count_light_ions")},
       hard_particle_threshold_{
-          parameters.getParameter<double>("hard_particle_threshold")},
-      count_light_ions_{parameters.getParameter<bool>("count_light_ions")} {}
+          parameters.getParameter<double>("hard_particle_threshold")} {}
 
 void PhotoNuclearTopologyFilter::stepping(const G4Step* step) {
   // Get the track associated with this step.

Biasing/src/Biasing/TargetProcessFilter.cxx

@@ -28,8 +28,6 @@ TargetProcessFilter::TargetProcessFilter(
   process_ = parameters.getParameter<std::string>("process");
 }
 
-TargetProcessFilter::~TargetProcessFilter() {}
-
 G4ClassificationOfNewTrack TargetProcessFilter::ClassifyNewTrack(
     const G4Track* track, const G4ClassificationOfNewTrack& currentTrackClass) {
   if (track == currentTrack_) {

CMakeLists.txt

@@ -30,6 +30,12 @@ list(APPEND CMAKE_MODULE_PATH ${LDMX_SW_SOURCE_DIR}/cmake/)
 # is set to NOTFOUND.
 include(BuildMacros RESULT_VARIABLE build_macros_found)
 
+# Adding ROOT, ACTS, HLS arb prec as "SYSTEM" 
+# which will silence compiler warnings from these 3rd party softwares
+include_directories(SYSTEM /usr/local/include/root)
+include_directories(SYSTEM Tracking/acts/Core/include)
+include_directories(SYSTEM Trigger/HLS_arbitrary_Precision_Types/include)
+
 # If an install location hasn't been set via CMAKE_INSTALL_PREFIX, set it to 
 # a reasonable default ($PWD/install).  
 if (CMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT)

Conditions/src/Conditions/GeneralCSVLoader.cxx

@@ -52,9 +52,9 @@ bool GeneralCSVLoader::nextRow() {
       colNames_.swap(line_split);
       continue;
     }
-    if (line_split.size() == colNames_.size())
+    if (line_split.size() == colNames_.size()) {
       rowData_.swap(line_split);
-    else {
+    } else {
       EXCEPTION_RAISE("CSVLineMismatch",
                       "Reading CSV found line with " +
                           std::to_string(line_split.size()) + " in CSV with " +
@@ -73,12 +73,13 @@ StringCSVLoader::StringCSVLoader(const std::string& source,
 
 std::string StringCSVLoader::getNextLine() {
   std::string retval;
-  if (rowBegin_ != rowEnd_)
+  if (rowBegin_ != rowEnd_) {
     if (rowEnd_ == std::string::npos) {
       retval = source_.substr(rowBegin_, rowEnd_);
     } else {
       retval = source_.substr(rowBegin_, rowEnd_ - rowBegin_);
     }
+  }
   // now we look for the follow on.
   // find the first non-end-of-line character
   rowBegin_ = source_.find_first_not_of(linesep_, rowEnd_);

Conditions/src/Conditions/SimpleTableStreamers.cxx

@@ -19,8 +19,7 @@ static void storeIdFields(unsigned int id, std::ostream& s) {
 
 template <class T, class V>
 void storeT(const T& t, std::ostream& s, bool expandIds) {
-  char buffer[100];
-  // write the header line
+  //  write the header line
   s << "\"DetID\"";
   if (expandIds && t.getRowCount() > 0) {
     storeIdFields(t.getRowId(0), s);
@@ -160,15 +159,15 @@ void loadT(T& table, std::istream& is) {
                                                  ") on line " +
                                                  std::to_string(iline));
     }
-    unsigned int id(0);
+    unsigned int tempId(0);
     std::vector<V> values(table_to_csv.size(), 0);
-    if (iDetID >= 0) id = strtoul(split[iDetID].c_str(), 0, 0);
+    if (iDetID >= 0) tempId = strtoul(split[iDetID].c_str(), 0, 0);
     V dummy(0);
     for (auto icopy : table_to_csv) {
       values[icopy.first] = convert(split[icopy.second], dummy);
     }
-    if (id != 0) {
-      table.add(id, values);
+    if (tempId != 0) {
+      table.add(tempId, values);
     }
   }
 }

Conditions/src/Conditions/URLStreamer.cxx

@@ -22,7 +22,7 @@ void urlstatistics(unsigned int& http_requests, unsigned int& http_failures) {
 }
 
 std::unique_ptr<std::istream> urlstream(const std::string& url) {
-  if (url.find("file://") == 0 || url.length() > 0 && url[0] == '/') {
+  if (url.find("file://") == 0 || (url.length() > 0 && url[0] == '/')) {
     std::string fname = url;
     if (fname.find("file://") == 0)
       fname = url.substr(url.find("file://") + strlen("file://"));
@@ -47,7 +47,7 @@ std::unique_ptr<std::istream> urlstream(const std::string& url) {
             fname, "-o", "/tmp/wget.log", url.c_str(), (char*)0);
     } else {
       int wstatus;
-      int wrv = waitpid(apid, &wstatus, 0);
+      waitpid(apid, &wstatus, 0);
       //      std::cout << "EXITED: " << WIFEXITED(wstatus) << " STATUS: " <<
       //      WEXITSTATUS(wstatus) << std::endl;
       if (WIFEXITED(wstatus) != 1 || WEXITSTATUS(wstatus) != 0) {

Conditions/test/TestConditions.cxx

@@ -309,9 +309,8 @@ TEST_CASE("Conditions", "[Conditions]") {
 
     cxt.setRun(128);
 
-    const DoubleTableCondition& httpTable128 =
-        hp->getConditions().getCondition<DoubleTableCondition>(
-            "testbeam22_pedestals");
+    hp->getConditions().getCondition<DoubleTableCondition>(
+        "testbeam22_pedestals");
 
     hp->getConditions().getCondition<DoubleTableCondition>(
         "testbeam22_pedestals");
@@ -331,9 +330,8 @@ TEST_CASE("Conditions", "[Conditions]") {
 
     cxt.setRun(140);
 
-    const DoubleTableCondition& httpTable140 =
-        hp->getConditions().getCondition<DoubleTableCondition>(
-            "testbeam22_pedestals");
+    hp->getConditions().getCondition<DoubleTableCondition>(
+        "testbeam22_pedestals");
 
     conditions::urlstatistics(http_requests[1], http_failures[1]);
     REQUIRE(((http_requests[1] - http_requests[0]) == 2 &&

DQM/src/DQM/DarkBremInteraction.cxx

@@ -111,7 +111,6 @@ void DarkBremInteraction::produce(framework::Event& event) {
 
   double incident_energy = energy(incident_p, recoil->getMass());
   double recoil_energy = energy(recoil_p, recoil->getMass());
-  double visible_energy = (beam->getEnergy() - incident_energy) + recoil_energy;
 
   std::vector<double> ap_vertex{aprime->getVertex()};
   std::string ap_vertex_volume{aprime->getVertexVolume()};

DQM/src/DQM/HcalInefficiencyDQM.cxx

@@ -15,10 +15,9 @@ void HcalInefficiencyAnalyzer::analyze(const framework::Event &event) {
 
   const std::vector<std::string> sectionNames{"back", "top", "bottom", "right",
                                               "left"};
-  for (const auto hit : hcalRecHits) {
+  for (const auto &hit : hcalRecHits) {
     const ldmx::HcalID id{static_cast<ldmx::DetectorID::RawValue>(hit.getID())};
     const auto section{id.section()};
-    const auto z{hit.getZPos()};
     const auto layer{id.layer()};
     if (hitPassesVeto(hit, section)) {
       if (layer < firstLayersHit[section]) {

DQM/src/DQM/NtuplizeHgcrocDigiCollection.cxx

@@ -99,7 +99,6 @@ void NtuplizeHgcrocDigiCollection::analyze(const framework::Event& event) {
     raw_id_ = static_cast<int>(d.id());
     if (using_eid_) {
       ldmx::HcalElectronicsID eid(d.id());
-      ldmx::HcalDigiID detid = detmap.get(eid);
       fpga_ = eid.fiber();
       link_ = eid.elink();
       good_link_ = (good_bxheader.at(link_) and good_trailer.at(link_));

DetDescr/src/DetDescr/EcalGeometry.cxx

@@ -19,8 +19,9 @@ static double distance(const std::pair<double, double>& p1,
               (p1.second - p2.second) * (p1.second - p2.second));
 }
 
-static double distance(const std::tuple<double, double, double>& p1,
-                       const std::tuple<double, double, double>& p2) {
+[[maybe_unused]] static double distance(
+    const std::tuple<double, double, double>& p1,
+    const std::tuple<double, double, double>& p2) {
   return sqrt((std::get<0>(p1) - std::get<0>(p2)) *
                   (std::get<0>(p1) - std::get<0>(p2)) +
               (std::get<1>(p1) - std::get<1>(p2)) *

DetDescr/src/DetDescr/HcalGeometry.cxx

@@ -57,35 +57,44 @@ std::vector<double> HcalGeometry::rotateGlobalToLocalBarPosition(
         case ScintillatorOrientation::vertical:
           return {globalPosition[2], globalPosition[0], globalPosition[1]};
         default:  // Should not be possible with current geometries
-          break;
+          EXCEPTION_RAISE("InvalidRotation",
+                          "Attempted to rotate into an invalid "
+                          "orientation for a scintillator bar!");
       }
     case ldmx::HcalID::HcalSection::TOP:
+      [[fallthrough]];
     case ldmx::HcalID::HcalSection::BOTTOM:
       switch (orientation) {
         case ScintillatorOrientation::horizontal:
           return {globalPosition[1], globalPosition[2], globalPosition[0]};
         case ScintillatorOrientation::depth:
           return {globalPosition[1], globalPosition[0], globalPosition[2]};
         default:  // Should not be possible with current geometries
-          break;
+          EXCEPTION_RAISE("InvalidRotation",
+                          "Attempted to rotate into an invalid "
+                          "orientation for a scintillator bar!");
       }
     case ldmx::HcalID::HcalSection::LEFT:
+      [[fallthrough]];
     case ldmx::HcalID::HcalSection::RIGHT:
       switch (orientation) {
         case ScintillatorOrientation::vertical:
           return {globalPosition[0], globalPosition[2], globalPosition[1]};
         case ScintillatorOrientation::depth:
           return globalPosition;
         default:  // Should not be possible with current geometries
-          break;
+          EXCEPTION_RAISE("InvalidRotation",
+                          "Attempted to rotate into an invalid "
+                          "orientation for a scintillator bar!");
       }
+    default:
+      // Can only reach this part if we somehow didn't match any of the options
+      // above. This could happen if someone introduces a new geometry but
+      // doesn't patch this part.
+      EXCEPTION_RAISE("InvalidRotation",
+                      "Attempted to rotate into an invalid "
+                      "orientation for a scintillator bar!");
   }
-  // Can only reach this part if we somehow didn't match any of the options
-  // above. This could happen if someone introduces a new geometry but doesn't
-  // patch this part.
-  EXCEPTION_RAISE("InvalidRotation",
-                  "Attempted to rotate into an invalid "
-                  "orientation for a scintillator bar!");
 }
 
 HcalGeometry::ScintillatorOrientation HcalGeometry::getScintillatorOrientation(

Ecal/include/Ecal/EcalDigiProducer.h

@@ -109,7 +109,8 @@ class EcalDigiProducer : public framework::Producer {
   std::unique_ptr<ldmx::HgcrocEmulator> hgcroc_;
 
   /// Total number of channels in the ECal
-  int nTotalChannels_;
+  // In a real data, zero-compression scenario the number of channels per event
+  // will change. Unused now, but keeping if for future dev int nTotalChannels_;
 
   /// Conversion from time in ns to ticks of the internal clock
   double ns_;

Ecal/include/Ecal/EcalVetoProcessor.h

@@ -70,8 +70,8 @@ class EcalVetoProcessor : public framework::Producer {
   /* Function to take loaded hit maps and find isolated hits in them */
   void fillIsolatedHitMap(const std::vector<ldmx::EcalHit>& ecalRecHits,
                           ldmx::EcalID globalCentroid,
-                          std::map<ldmx::EcalID, float>& cellMap_,
-                          std::map<ldmx::EcalID, float>& cellMapIso_,
+                          std::map<ldmx::EcalID, float>& cellMap,
+                          std::map<ldmx::EcalID, float>& cellMapIso,
                           bool doTight = false);
 
   std::vector<XYCoords> getTrajectory(std::vector<double> momentum,
@@ -113,10 +113,8 @@ class EcalVetoProcessor : public framework::Producer {
   std::vector<std::vector<double>> roc_range_values_;
 
   int nEcalLayers_{0};
-  int backEcalStartingLayer_{0};
   int nReadoutHits_{0};
   int deepestLayerHit_{0};
-  int doBdt_{0};
 
   double summedDet_{0};
   double summedTightIso_{0};
@@ -152,7 +150,6 @@ class EcalVetoProcessor : public framework::Producer {
   double beamEnergyMeV_{0};
 
   bool verbose_{false};
-  bool doesPassVeto_{false};
 
   std::string bdtFileName_;
   std::string rocFileName_;

Ecal/include/Ecal/Event/EcalVetoResult.h

@@ -3,6 +3,7 @@
  * @brief Class used to encapsulate the results obtained from
  *        EcalVetoProcessor.
  * @author Omar Moreno, SLAC National Accelerator Laboratory
+ * @author Danyi Zhang, Tamas Almos Vami (UCSB)
  */
 
 #ifndef EVENT_ECALVETORESULT_H_
@@ -27,7 +28,7 @@ class EcalVetoResult {
   EcalVetoResult();
 
   /** Destructor */
-  ~EcalVetoResult();
+  virtual ~EcalVetoResult();
 
   /**
    * Set the sim particle and 'is findable' flag.
@@ -219,23 +220,23 @@ class EcalVetoResult {
   };
 
   /** Return the x position of the recoil at the Ecal face. */
-  const double getRecoilX() const { return recoilX_; };
+  double getRecoilX() const { return recoilX_; };
 
   /** Return the y position of the recoil at the Ecal face. */
-  const double getRecoilY() const { return recoilY_; };
+  double getRecoilY() const { return recoilY_; };
 
   /// Number of straight tracks found
-  const int getNStraightTracks() const { return nStraightTracks_; }
+  int getNStraightTracks() const { return nStraightTracks_; }
 
   /// Number of linear-regression tracks found
-  const int getNLinRegTracks() const { return nLinregTracks_; }
-
-  const int getFirstNearPhLayer() const { return firstNearPhLayer_; }
-  const int getNNearPhHits() const { return nNearPhHits_; }
-  const int getPhotonTerritoryHits() const { return photonTerritoryHits_; }
-  const float getEPAng() const { return epAng_; }
-  const float getEPSep() const { return epSep_; }
-  const float getEPDot() const { return epDot_; }
+  int getNLinRegTracks() const { return nLinregTracks_; }
+
+  int getFirstNearPhLayer() const { return firstNearPhLayer_; }
+  int getNNearPhHits() const { return nNearPhHits_; }
+  int getPhotonTerritoryHits() const { return photonTerritoryHits_; }
+  float getEPAng() const { return epAng_; }
+  float getEPSep() const { return epSep_; }
+  float getEPDot() const { return epDot_; }
 
  private:
   /** Flag indicating whether the event is vetoed by the Ecal. */

Ecal/include/Ecal/MyClusterWeight.h

@@ -30,12 +30,9 @@ class MyClusterWeight {
     double bZ = b.centroid().Pz();
 
     double dijz;
-    double eFrac;
     if (aE >= bE) {
-      eFrac = bE / aE;
       dijz = bZ - aZ;
     } else {
-      eFrac = aE / bE;
       dijz = aZ - bZ;
     }