Non-standard variable length arrays preventing compilation with clang

[EinarElen]

Hi TS folks,

I've been working a bit on ways to add more static analysis into the CI of ldmx-sw to, hopefully, catch bugs before they make their way into the software. One of the ways to do that is to use the warnings from more than one compiler and there are two standard ones available, GCC and Clang. Clang is designed to be almost 100% compatible with GCC so most of the time you can just change which compiler gets invoked and it just works but not always. For ldmx-sw, everything compiles with clang except two things in the TrigScint module.

In TrigScintQIEDigiProducer and QualityFlagAnalyzer we are using C-variable length arrays which GCC is fine with compiling but Clang rejects it. Patching it is relatively trivial, for the QualityFlagAnalyzer where the variable length is a constant you could just change

  const int nChannels{16};

to

  static constexpr int nChannels{16};

In the DigiProducer, the array length is dynamic. I would probably just replace the cases where we do something like

Expo* ex[stripsPerArray_] = {0};

with a vector

std::vector<Expo*> ex(stripsPerArray_, nullptr);

Does anyone have any objections to making this change? I can create a PR for it otherwise :)

[bryngemark]

I have no objections, the QIE digi producer is @GNiramay's code though so I'll let him comment. Overall this doesn't sound like a problem at all to me.

[GNiramay]

Yeah. You can go ahead and make the change to ex. As long as the vector has a predefined length, it shouldn't make problems.

[EinarElen]

I noticed one thing when rebuilding things. A lot of the producers in the TrigScint module have a virtual onFileClose and onFileOpen method but they don't actually override anything in the processor class meaning they will never actually be called. You can see this by e.g. marking them as override, similar to what's done for onProcessStart. The reason for this is that the base class function takes an ldmx::EventFile& argument.

This doesn't do any damage because the methods are all empty, but if anyone is going to try and use them for something they are probably in for a bit of an unpleasant debugging session. If you want, I could patch these as well during this PR.

[bryngemark]

Thanks @EinarElen, patching them sounds like a good idea. I can't really imagine what those methods would be used for to begin with.

[EinarElen]

Given that the content of most of them is along the lines of

void QIEAnalyzer::onFileOpen(EventFile& /*eventFile*/) {
  std::cout << "\n\n File is opening! My analyzer should do something -- like "
               "print this \n\n"
            << std::endl;
  return;
}

I suspect removing them would be a better option than getting them to work