Make PixelChanneldentifier::thePacking constexpr #28617
Conversation
|
The code-checks are being triggered in jenkins. |
|
I happen to be working on a different problem which I thought might have to do with the static. It turned out not to be the case, but I thought this change was good none the less. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28617/13141
|
|
A new Pull Request was created by @Dr15Jones (Chris Jones) for master. It involves the following packages: DataFormats/SiPixelDetId @cmsbuild, @civanch, @mdhildreth can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test |
|
The tests are being triggered in jenkins. |
|
|
||
| static int pixelToChannel(int row, int col) { return (row << thePacking.column_width) | col; } | ||
|
|
||
| namespace pixelchanelidentifierimpl { |
There was a problem hiding this comment.
The Packing class had to be removed from within PixelChannelIdentifier for a technicality about C++ and constexpr. You can not declare a constexpr member data where the type is defined within the class. The reason is the class is not defined until its closing }; and that includes classes defined within it. The last bit is you can't use constexpr on an undefined type. Declaring Packing outside of PixelChannelIdentifier makes use Packing is fully defined when the compiler reaches the constexpr.
|
+1 |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
|
@civanch this looks a technical update that does not show any regression as expected , I will integrate it, please have anyway a look at it |
|
+1 |
|
merge |
PR description:
Converted the const static PixelChanneldentifier::thePacking to be constexpr
PR validation:
The code compiles.