Iss53 clang-patches

[EinarElen]

This resolves #53 by switching from variable length arrays to either arrays with static length or a corresponding vector.

Should wait with merging until https://github.com/LDMX-Software/docker/issues/64 so we can confirm that it fixes what it tries to fix.

[EinarElen]

The docker PR isn't merged, but I've tested with the dev branch and TrigScint now compiles just fine without any issues :)

This PR does the following

• Change the use of dynamic variable length arrays to static arrays or vectors (see d3e7b92817e5dce7e9d246c192eb0af6c15a3342 and 4ba64d7db68f74624389c8e535aeb4066fe6a821)
• Remove all the uses of onFileOpen/onFileClose that were not ever used due to having the wrong signature. I decided to remove them rather than patching them because I suspect adding debug logging saying something equivalent to "file opened" and "file closed" for each processor probably isn't what we want (see 52284ee3089c38cf54a36fbe074301270618a3a6)
• Add missing virtual destructors: Any type that has the ROOT ClassDef macro has virtual functions even if we don't declare any. If you are deleting an object through a pointer to base, which ROOT does underneath the hood, and the destructor isn't virtual, we can run into some annoying (although most likely just leaks and occasional crashes) undefined behavior (see 225b9b38d2062ac2495d0c543046ec3d30edbd8d)
• clang-format the affected files

[EinarElen]

Right, i was thinking the test/PR validation would be sufficient but if there's no checks for the TB code I'll give that part an extra review.

The changes should not change anything, but it's worth making sure

[EinarElen]

I forgot to tag @GNiramay as one of the reviewers here. I'm not sure who the 3rd reviewer here in the TS module usually is. Who would you suggest @bryngemark?

[bryngemark]

i would suggest bypassing a 3rd reviewer, i don't know who that would be (andrew has often had a hard time keeping up, you could pull in Lauren if you want to, otherwise i typically enlist tom) -- i'm not sure 3 reviewers is a good number when we are typically only 3 current TS developers in total.

[EinarElen]

That sounds reasonable to me, although I don't think that I can do the actual PR merge then (don't think I have permissions)

[bryngemark]

I do. Let's give @GNiramay another day to chime in first.

[GNiramay]

madeTrack and skipDn variables are new to me; but the new initialization of vector of Expo looks good to me.

[EinarElen]

I think they are leftover variables from development that are still around

[bryngemark]

kind of -- they are useful in other geometries than the commonly used one. but i haven't gotten round to introducing those to shared code. so i'd rather not clean it up. @GNiramay could you review and approve just so i know you're fine with these changes?

[EinarElen]

They are still in there just commented, I could add a comment referencing this issue if you want. Other than this, is this ready to be merged? I would like to be able to try some things with clang in our CI that depends on this being merged