spradlin / WCSim

The WCSim GEANT4 application
0 stars 0 forks source link

Compiler warnings in WCSimEventAction.{hh,cc} #2

Closed spradlin closed 3 years ago

spradlin commented 3 years ago

Compiling WCSim with gcc/g++ version 4.8.5 reports the warnings related to WCSimEventAction.{hh,cc} in the attached file make.log. The warnings were generated by touching the source files WCSimEventAction.{hh,cc} and makeing.

spradlin commented 3 years ago

The source file has #define compiler variables to optionally compile additional reportage. Be sure to check the compilation both with and without these variables enabled.

spradlin commented 3 years ago

See the code at https://github.com/spradlin/WCSim/blob/e5d51e71612d6a9fdb69a425518a519ef8450aad/src/WCSimEventAction.cc#L207-L215 for context of the warning

src/WCSimEventAction.cc: In member function 'virtual void WCSimEventAction::EndOfEventAction(const G4Event*)':
src/WCSimEventAction.cc:211:21: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
   for( Int_t u=0; u<nvtxs; u++ ){
                     ^

It and other identical warnings occur becuase the loop index is signed rather than unsigned.

The return type of WCSimPrimaryGeneratorAction::GetNvtxs() is G4int, which is an alias of int in the GEANT4 header G4Types.hh. There is no reason for the variable nvtxs to be unsigned. Probably the best solution is to make both the variable ntvxs and the loop indexes G4ints.

Well, the best solution would probably be to use auto variables for guaranteed and durable consistency, but that would also obscure the type. If the signed/unsigned distinction is important to the code's function, it could get lost.

spradlin commented 3 years ago

The warning

src/WCSimEventAction.cc: In member function 'void WCSimEventAction::FillRootEvent(G4int, const ntupleStruct&, G4TrajectoryContainer*, WCSimWCDigitsCollection*, WCSimWCTriggeredDigitsCollection*, G4String)':
src/WCSimEventAction.cc:810:62: warning: declaration of 'jhfNtuple' shadows a global declaration [-Wshadow]
                                      G4String detectorElement)
                                                              ^
In file included from include/WCSimRootEvent.hh:18:0,
                 from include/WCSimRunAction.hh:11,
                 from src/WCSimEventAction.cc:3:
include/jhfNtuple.h:55:28: warning: shadowed declaration is here [-Wshadow]
 extern struct ntupleStruct jhfNtuple;
                            ^

is interesting.

The only calls to WCSimEventAction::FillRootEvent() that i have so far found are in WCSimEventAction::EndOfEventAction(): https://github.com/spradlin/WCSim/blob/e5d51e71612d6a9fdb69a425518a519ef8450aad/src/WCSimEventAction.cc#L658-L672

In each of these cases, the global instance of jhfNtuple is passed into WCSimEventAction::FillRootEvent() as the argument with a same local name.

The most transparent solution is probably to change the name of the local variable.

spradlin commented 3 years ago

There are warnings related to WCSimTrajectory.hh and WCSimRunAction.hh, which are included WCSimEventAction.cc, that should be addressed separately.