Part 3 of static analysis: clang-tidy

Biasing/src/Biasing/PhotoNuclearProductsFilter.cxx

@@ -72,7 +72,9 @@ void PhotoNuclearProductsFilter::stepping(const G4Step* step) {
 
   // Once the PN gamma has been procesed, untag it so its not reprocessed
   // again.
-  trackInfo->tagPNGamma(false);
+  if (trackInfo) {
+    trackInfo->tagPNGamma(false);
+  }
 }
 }  // namespace biasing
 

Biasing/src/Biasing/PhotoNuclearTopologyFilters.cxx

@@ -70,7 +70,9 @@ void PhotoNuclearTopologyFilter::stepping(const G4Step* step) {
 
   // Once the PN gamma has been procesed, untag it so its not reprocessed
   // again.
-  trackInfo->tagPNGamma(false);
+  if (trackInfo) {
+    trackInfo->tagPNGamma(false);
+  }
 }
 
 }  // namespace biasing

Biasing/src/Biasing/TaggerHitFilter.cxx

@@ -45,12 +45,25 @@ void TaggerHitFilter::stepping(const G4Step* step) {
     return;
 
   // The copy number is used to identify which layer energy was deposited into.
-  auto copy_number{step->GetPreStepPoint()
-                       ->GetTouchableHandle()
-                       ->GetHistory()
-                       ->GetVolume(2)
-                       ->GetCopyNo()};
-
+  int copy_number{0};
+  // Get the pre-step point
+  auto* preStepPoint = step->GetPreStepPoint();
+  if (preStepPoint) {
+    // Get the touchable handle
+    auto touchableHandle = preStepPoint->GetTouchableHandle();
+    if (touchableHandle) {
+      // Get the history
+      auto* history = touchableHandle->GetHistory();
+      if (history) {
+        // Get the volume
+        auto* volumeAtTwo = history->GetVolume(2);
+        if (volumeAtTwo) {
+          // Get the copy number
+          copy_number = volumeAtTwo->GetCopyNo();
+        }
+      }
+    }
+  }
   layer_count_.insert(copy_number);
 }
 

CMakeLists.txt

@@ -20,7 +20,7 @@ project(LDMX_SW VERSION 3.1.1
                 LANGUAGES CXX    
 )
 
-set(CMAKE_CXX_STANDARD 17)
+set(CMAKE_CXX_STANDARD 20)
 
 # Load additional macros used by this project. 
 list(APPEND CMAKE_MODULE_PATH ${LDMX_SW_SOURCE_DIR}/cmake/)
@@ -36,6 +36,9 @@ include_directories(SYSTEM /usr/local/include/root)
 include_directories(SYSTEM Tracking/acts/Core/include)
 include_directories(SYSTEM Trigger/HLS_arbitrary_Precision_Types/include)
 
+# FunctionalCoreTest.cxx doesnt comply with clang-tidy but that's ok
+set_source_files_properties(Framework/test/FunctionalCoreTest.cxx PROPERTIES COMPILE_OPTIONS "-Wno-null-dereference")
+
 # 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)

DQM/src/DQM/SampleValidation.cxx

@@ -20,7 +20,7 @@ void SampleValidation::analyze(const framework::Event& event) {
 
   std::vector<int> primary_daughters;
 
-  double hard_thresh;
+  double hard_thresh{9999.0};
 
   // Loop over all SimParticles
   for (auto const& it : particle_map) {

DetDescr/include/DetDescr/IDField.h

@@ -81,27 +81,27 @@ class IDField {
   /**
    * The name of the field.
    */
-  std::string fieldName;
+  std::string fieldName_;
 
   /**
    * The index of the field.
    */
-  unsigned index;
+  unsigned index_;
 
   /**
    * The start bit of the field.
    */
-  unsigned startBit;
+  unsigned startBit_;
 
   /**
    * The end bit of the field.
    */
-  unsigned endBit;
+  unsigned endBit_;
 
   /**
    * The bit mask of the field.
    */
-  unsigned bitMask;
+  unsigned bitMask_;
 };
 }  // namespace ldmx
 

DetDescr/src/DetDescr/IDField.cxx

@@ -4,20 +4,23 @@ namespace ldmx {
 
 IDField::IDField(std::string fieldName, unsigned index, unsigned startBit,
                  unsigned endBit)
-    : fieldName(fieldName), index(index), startBit(startBit), endBit(endBit) {
+    : fieldName_(fieldName),
+      index_(index),
+      startBit_(startBit),
+      endBit_(endBit) {
   // Create bit mask for the field.
-  bitMask = IDField::createBitMask(startBit, endBit);
+  bitMask_ = IDField::createBitMask(startBit, endBit);
 }
 
-const std::string& IDField::getFieldName() { return fieldName; }
+const std::string& IDField::getFieldName() { return fieldName_; }
 
-unsigned IDField::getIndex() { return index; }
+unsigned IDField::getIndex() { return index_; }
 
-unsigned IDField::getStartBit() { return startBit; }
+unsigned IDField::getStartBit() { return startBit_; }
 
-unsigned IDField::getEndBit() { return endBit; }
+unsigned IDField::getEndBit() { return endBit_; }
 
-unsigned IDField::getBitMask() { return bitMask; }
+unsigned IDField::getBitMask() { return bitMask_; }
 
 unsigned IDField::createBitMask(unsigned startBit, unsigned endBit) {
   unsigned mask = 0;

Ecal/src/Ecal/print_ecal_hex_readout.cxx

@@ -76,7 +76,7 @@ int main() {
       std::make_pair(+1. * hexCornerRadius * cos(M_PI / 3),
                      -1. * hexCornerRadius * sin(M_PI / 3)),
       std::make_pair(+1. * hexCornerRadius, 0.)};
-  TLine moduleHexBorder;
+  TLine moduleHexBorder{0., 0., 0., 0.};
   moduleHexBorder.SetLineColorAlpha(kRed, 0.5);
   moduleHexBorder.SetLineWidth(2);
   for (int i = 1; i < hexCorners.size(); i++) {

Framework/include/Framework/Histograms.h

@@ -164,7 +164,10 @@ class HistogramHelper {
    * @param val value to fill
    */
   void fill(const std::string& name, const double& val) {
-    dynamic_cast<TH1F*>(this->get(name))->Fill(val, theWeight_);
+    auto hist = dynamic_cast<TH1F*>(this->get(name));
+    if (hist) {
+      hist->Fill(val, theWeight_);
+    }
   }
 
   /**
@@ -177,7 +180,10 @@ class HistogramHelper {
    * @param valy y value to fill
    */
   void fill(const std::string& name, const double& valx, const double& valy) {
-    dynamic_cast<TH2F*>(this->get(name))->Fill(valx, valy, theWeight_);
+    auto hist = dynamic_cast<TH2F*>(this->get(name));
+    if (hist) {
+      hist->Fill(valx, valy, theWeight_);
+    }
   }
 
   /**

Framework/src/Framework/Conditions.cxx

@@ -21,8 +21,7 @@ void Conditions::createConditionsObjectProvider(
     if (providerMap_.find(provides) != providerMap_.end()) {
       EXCEPTION_RAISE(
           "ConditionAmbiguityException",
-          std::string(
-              "Multiple ConditonsObjectProviders configured to provide ") +
+          "Multiple ConditonsObjectProviders configured to provide " +
               provides);
     }
     providerMap_[provides] = cop;
@@ -62,9 +61,8 @@ const ConditionsObject* Conditions::getConditionPtr(
     auto copptr = providerMap_.find(condition_name);
 
     if (copptr == providerMap_.end()) {
-      EXCEPTION_RAISE(
-          "ConditionUnavailable",
-          std::string("No provider is available for : " + condition_name));
+      EXCEPTION_RAISE("ConditionUnavailable",
+                      "No provider is available for : " + condition_name);
     }
 
     std::pair<const ConditionsObject*, ConditionsIOV> cond =
@@ -73,8 +71,7 @@ const ConditionsObject* Conditions::getConditionPtr(
     if (!cond.first) {
       EXCEPTION_RAISE(
           "ConditionUnavailable",
-          std::string("Null condition returned for requested item : " +
-                      condition_name));
+          "Null condition returned for requested item : " + condition_name);
     }
 
     // first request, create a cache entry

Framework/src/Framework/Event.cxx

@@ -121,8 +121,15 @@ void Event::setInputTree(TTree* tree) {
 
   // find the names of all the existing branches
   TObjArray* branches = inputTree_->GetListOfBranches();
+  if (!branches) {
+    EXCEPTION_RAISE("RunHeaderError", "Branch doesnt exists");
+    return;
+  }
   for (int i = 0; i < branches->GetEntriesFast(); i++) {
-    std::string brname = branches->At(i)->GetName();
+    std::string brname;
+    if (branches->At(i)) {
+      brname = branches->At(i)->GetName();
+    }
     if (brname != ldmx::EventHeader::BRANCH) {
       size_t j = brname.find("_");
       auto br = dynamic_cast<TBranchElement*>(branches->At(i));

Framework/src/Framework/EventFile.cxx

@@ -413,9 +413,13 @@ void EventFile::importRunHeaders() {
     TTreeReaderValue<ldmx::RunHeader> oldRunHeader(oldRunTree, "RunHeader");
     // TODO check that setup went correctly
     while (oldRunTree.Next()) {
-      // copy input run tree into run map
-      runMap_[oldRunHeader->getRunNumber()] =
-          std::make_pair(true, new ldmx::RunHeader(*oldRunHeader));
+      auto *oldRunHeaderPtr = oldRunHeader.Get();
+      if (oldRunHeaderPtr != nullptr) {
+        // copy input run tree into run map
+        // We should consider moving to a shared_ptr instead of 'new'
+        runMap_[oldRunHeaderPtr->getRunNumber()] =
+            std::make_pair(true, new ldmx::RunHeader(*oldRunHeader));
+      }
     }
   }
 

Framework/test/NtupleManagerTest.cxx

@@ -1,5 +1,5 @@
 /**
- * @file FunctionalCoreTest.cxx
+ * @file NtupleManagerTest.cxx
  * @brief Test the operation of Framework processing
  *
  * @author Tom Eichlersmith, University of Minnesota

Hcal/include/Hcal/Event/HcalHit.h

@@ -199,34 +199,34 @@ class HcalHit : public ldmx::CalorimeterHit {
 
  private:
   /** The number of PE estimated for this hit. */
-  float pe_{0};
+  float pe_{0.0};
 
   /** The minimum number of PE estimated for this hit, different from pe_ when
    * you have two ended readout */
-  float minpe_{-99};
+  float minpe_{-99.0};
 
   /// section, layer, strip and end
-  int section_;
-  int layer_;
-  int strip_;
-  int end_;
+  int section_{-99};
+  int layer_{-99};
+  int strip_{-99};
+  int end_{-99};
 
   /// isADC
-  int isADC_;
+  int isADC_{-99};
 
-  double timeDiff_;
-  double position_;
-  double isX_;
+  double timeDiff_{-9999.};
+  double position_{-9999.};
+  double isX_{-9999.};
 
-  double toaPos_;
-  double toaNeg_;
-  double amplitudePos_;
-  double amplitudeNeg_;
+  double toaPos_{-9999.};
+  double toaNeg_{-9999.};
+  double amplitudePos_{-9999.};
+  double amplitudeNeg_{-9999.};
 
   /**
    * The ROOT class definition.
    */
-  ClassDef(HcalHit, 3);
+  ClassDef(HcalHit, 4);
 };
 }  // namespace ldmx
 

Hcal/include/Hcal/HcalWABVetoProcessor.h

@@ -30,7 +30,7 @@ class HcalWABVetoProcessor : public framework::Producer {
   HcalWABVetoProcessor(const std::string &name, framework::Process &process);
 
   /** Destructor */
-  virtual ~HcalWABVetoProcessor();
+  virtual ~HcalWABVetoProcessor() = default;
 
   /**
    * Configure the processor using the given user specified parameters.

Hcal/src/Hcal/HcalWABVetoProcessor.cxx

@@ -19,8 +19,6 @@ HcalWABVetoProcessor::HcalWABVetoProcessor(const std::string &name,
                                            framework::Process &process)
     : Producer(name, process) {}
 
-HcalWABVetoProcessor::~HcalWABVetoProcessor() {}
-
 void HcalWABVetoProcessor::configure(
     framework::config::Parameters &parameters) {
   maxtotalEnergyCompare_ =
@@ -58,7 +56,7 @@ void HcalWABVetoProcessor::produce(framework::Event &event) {
   float totalHCALEnergy{0};
   float totalECALEnergy{0};
   float maxPE{-1000};
-  const ldmx::HcalHit *maxPEHit;
+  const ldmx::HcalHit *maxPEHit = nullptr;
   for (const ldmx::HcalHit &hcalHit : hcalRecHits) {
     if (hcalHit.isNoise() == 0) {
       totalHCALEnergy += hcalHit.getPE();
@@ -67,7 +65,7 @@ void HcalWABVetoProcessor::produce(framework::Event &event) {
     // Find the maximum PE in the list
     if (maxPE < hcalHit.getPE()) {
       maxPE = hcalHit.getPE();
-      maxPEHit = &hcalHit;
+      maxPEHit = const_cast<ldmx::HcalHit *>(&hcalHit);
     }
   }
 

Recon/src/Recon/OverlayProducer.cxx

@@ -184,14 +184,14 @@ void OverlayProducer::produce(framework::Event &event) {
     // sample a poisson distribution, or use mu as fixed number of overlay
     // events
     int nEvsOverlay =
-        doPoissonOOT_ ? (int)rndm_->Poisson(poissonMu_) : (int)poissonMu_;
+        doPoissonOOT_ ? rndm_->Poisson(poissonMu_) : (int)poissonMu_;
 
     // special case: in-time pileup at bunch 0
     if (bunchOffset == 0) {
       if (!doPoissonIT_)
         nEvsOverlay = (int)poissonMu_;          // fix it to the average
       else if (doPoissonIT_ && !doPoissonOOT_)  // then we haven't set this yet
-        nEvsOverlay = (int)rndm_->Poisson(poissonMu_);
+        nEvsOverlay = rndm_->Poisson(poissonMu_);
 
       // paticularly useful in the poisson fluctuated case
       event.getEventHeader().setIntParameter("inTimePU", nEvsOverlay);

SimCore/src/SimCore/G4User/TrackingAction.cxx

@@ -35,7 +35,9 @@ void TrackingAction::PreUserTrackingAction(const G4Track* track) {
           track->GetDynamicParticle()
               ->GetPrimaryParticle()
               ->GetUserInformation());
-      curGenStatus = primaryInfo->getHepEvtStatus();
+      if (primaryInfo) {
+        curGenStatus = primaryInfo->getHepEvtStatus();
+      }
     }
 
     /**

SimCore/src/SimCore/SDs/EcalSD.cxx

@@ -47,11 +47,21 @@ G4bool EcalSD::ProcessHits(G4Step* aStep, G4TouchableHistory*) {
       0.5 * (prePoint->GetPosition() + postPoint->GetPosition());
 
   // Create the ID for the hit.
-  int cpynum = aStep->GetPreStepPoint()
-                   ->GetTouchableHandle()
-                   ->GetHistory()
-                   ->GetVolume(layer_depth)
-                   ->GetCopyNo();
+  int cpynum{0};  // Initialize cpynum to 0
+
+  auto preStepPoint = aStep->GetPreStepPoint();
+  if (preStepPoint) {
+    auto touchableHandle = preStepPoint->GetTouchableHandle();
+    if (touchableHandle) {
+      auto history = touchableHandle->GetHistory();
+      if (history) {
+        auto volume = history->GetVolume(layer_depth);
+        if (volume) {
+          cpynum = volume->GetCopyNo();
+        }
+      }
+    }
+  }
   int layerNumber;
   layerNumber = cpynum / 7;
   int module_position = cpynum % 7;