switches with no break?

[namapane]

Dear MELA developers,
While checking compilation warnings, I noticed this switch() where some of the case do not include a break:
https://github.com/JHUGen/JHUGenMELA/blob/master/MELA/src/MELANCSplineCore.cc#L94-L101
Is this really intended?
The effect here is that eg if bcBegin==bcApproximatedSecondDerivative then both the case bcApproximatedSecondDerivative and the following one case bcApproximatedSlope are executed, which loosk suspect.

Same for case bcClamped and case bcQuadratic (both will be executed if bcBegin==bcClamped).
HTH,
N.

[usarica]

@namapane:

The behavior is intended: bcval is initialized to 0, so cases bcNaturalSpline and bcClamped use this initial condition. Cases bcApproximatedSecondDerivative and bcApproximatedSlope differ from bcNaturalSpline and bcClamped, respectively, with only a change in the value of bcval in the formula. You might get verbose warnings depending on compiler flags, but it should be harmless. Nevertheless, if you tell me the warnings, those could be investigated.

I also see a bug on L139. The left-hand side should be ecval, not bcval. I will fix that with a PR at the same time I investigate the warnings if you mention them.

[namapane]

Thanks @usarica for the quick reply. The warning is the one documented here:
https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wimplicit-fallthrough
essentially, it warns aboout all case statements with no break , as these are often unintended.
As documented in the link above, when the behaviour is intended It is possible to suppress the warning in gcc adding a statement:
__attribute__ ((fallthrough));
which has also the side effect of making it apparent to the reader of the code.
BTW in case you make a bugfix PR, may I suggest to also add new architectures/compilers in setup.sh?
In the 12X series the default arch is el8_amd64_gcc10, while for 13X it is el8_amd64_gcc11. In these cases the script sets slc7_amd64_gcc920 as arch, which is probably still fine, but you may consider updating it nevertheless.
BTW SLC7 will arrive at end-of-life mid next year.

[usarica]

I am not sure whether the issue is still being considered - in case it is still ongoing, @namapane would you please let us know? Otherwise, I will close this issue.

[namapane]

I am not sure whether the issue is still being considered - in case it is still ongoing, @namapane would you please let us know? Otherwise, I will close this issue.

As far as I can tell, the code would still produce a warning. My suggestion is still to follow the gcc prescription adding:
__attribute__ ((fallthrough));
which suppresses the warning, and also makes it clear to the casual reader that the lack of "break" is intended.

I can make a PR for that, if you wish.