search

InsightSoftwareConsortium / ITK

Insight Toolkit (ITK) -- Official Repository. ITK builds on a proven, spatially-oriented architecture for processing, segmentation, and registration of scientific images in two, three, or more dimensions.
https://itk.org
Apache License 2.0
1.37k stars 660 forks source link

COMP: Use the c++ 17 common [[fallthrough]] attribute #4580

Closed hjmjohnson closed 2 months ago

hjmjohnson commented 2 months ago

[fallthrough] indicates that the fall through from the previous case label is intentional and should not be diagnosed by a compiler that warns on fall-through (attribute specifier)

Remove complicated #ifdef macro logic to enforce compiler specific behaviors.

PR Checklist

blowekamp commented 2 months ago

This change looks good. How close are we to 5.4? Do we have another RC? Are we planning some kind of new feature freeze soon?

This seems like using a new C++17 feature. How certain are we that all compilers support this feature correctly?

hjmjohnson commented 2 months ago

@blowekamp I do think all compilers that support c++17 do support this. It is not an onerous addition to the language. Many compilers have used similar non-standard methods. c++17 standardized the mechanism.

I have not looked at "all compilers", but the common ones have definitive support for this feature.

thewtex commented 2 months ago

How close are we to 5.4? Do we have another RC?

ITK 5.4RC3 was tagged recently, but an issue was discovered immediately -- 5.4RC4 will be tagged and announced with the fix next week.

We should not add major new features until 5.4.0, but this should be fine -- we have had a C++17 requirement for a while, and it is passing all CI.

blowekamp commented 2 months ago

Yes, this change "should" be fine. Let us get it in for 5.4rc4?