Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CalibCalorimetry/CaloTPG: definite static const float members #13044

Merged

Conversation

davidlt
Copy link
Contributor

@davidlt davidlt commented Jan 24, 2016

The patch resolves issue with Clang compiler.
N3690 (should be C++11 standard) and latest draft N4567, 9.4.2/3

...
The member shall still be defined in a namespace scope if it
is odr-used (3.2) in the program and the namespace scope definition
shall not contain an initializer.
9.4.2/4 talks about how const static data members are being handled.

Also standard says that no diagnostic is required by compiler.

src/CalibCalorimetryCaloTPG/CaloTPGTranscoderULUT.o: In function aloTPGTranscoderULUT::setup(HcalLutMetadata const&, HcalTrigTowerGeometry const&)':
src/CalibCalorimetry/CaloTPG/src/CaloTPGTranscoderULUT.cc:(.text+0x168b): undefined reference to aloTPGTranscoderULUT::LSB_HBHE'
src/CalibCalorimetry/CaloTPG/src/CaloTPGTranscoderULUT.cc:(.text+0x16b6): undefined reference to aloTPGTranscoderULUT::LSB_HF'

Signed-off-by: David Abdurachmanov David.Abdurachmanov@cern.ch

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @davidlt for CMSSW_8_0_X.

It involves the following packages:

CalibCalorimetry/CaloTPG

@diguida, @cerminar, @cmsbuild, @franzoni, @mmusich, @mulhearn, @davidlange6 can you please review it and eventually sign? Thanks.
@tocheng this is something you requested to watch as well.
@slava77, @Degano, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@cmsbuild
Copy link
Contributor

-1
Tested at: e52da7b
I found an error when building:

>> Compiling  /tmp/cmsbuild/workspace/ib-any-integration/CMSSW_8_0_X_2016-01-22-2300/src/Utilities/StaticAnalyzers/src/getParamDumper.cpp 
>> Compiling  /tmp/cmsbuild/workspace/ib-any-integration/CMSSW_8_0_X_2016-01-22-2300/src/Utilities/StaticAnalyzers/src/dablooms.c 
>> Compiling  /tmp/cmsbuild/workspace/ib-any-integration/CMSSW_8_0_X_2016-01-22-2300/src/Utilities/StaticAnalyzers/src/murmur.c 
>> Compiling  /tmp/cmsbuild/workspace/ib-any-integration/CMSSW_8_0_X_2016-01-22-2300/src/CalibCalorimetry/CaloTPG/src/CaloTPGTranscoderULUT.cc 
>> Compiling edm plugin /tmp/cmsbuild/workspace/ib-any-integration/CMSSW_8_0_X_2016-01-22-2300/src/CalibCalorimetry/CaloTPG/plugins/CaloTPGTranscoderULUTs.cc 
/tmp/cmsbuild/workspace/ib-any-integration/CMSSW_8_0_X_2016-01-22-2300/src/CalibCalorimetry/CaloTPG/src/CaloTPGTranscoderULUT.cc:18:36: error: 'constexpr' needed for in-class initialization of static data member 'const float CaloTPGTranscoderULUT::LSB_HF' of non-integral type [-fpermissive]
 const float CaloTPGTranscoderULUT::LSB_HF;
                                    ^
/tmp/cmsbuild/workspace/ib-any-integration/CMSSW_8_0_X_2016-01-22-2300/src/CalibCalorimetry/CaloTPG/src/CaloTPGTranscoderULUT.cc:19:36: error: 'constexpr' needed for in-class initialization of static data member 'const float CaloTPGTranscoderULUT::LSB_HBHE' of non-integral type [-fpermissive]
 const float CaloTPGTranscoderULUT::LSB_HBHE;
                                    ^


you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-13044/10698/summary.html

The patch resolves issue with Clang compiler.
N3690 (should be C++11 standard) and latest draft N4567, 9.4.2/3

...
The member shall still be defined in a namespace scope if it
is odr-used (3.2) in the program and the namespace scope definition
shall not contain an initializer.
9.4.2/4 talks about how const static data members are being handled.

Also standard says that no diagnostic is required by compiler.

    src/CalibCalorimetryCaloTPG/CaloTPGTranscoderULUT.o: In function aloTPGTranscoderULUT::setup(HcalLutMetadata const&, HcalTrigTowerGeometry const&)':
    src/CalibCalorimetry/CaloTPG/src/CaloTPGTranscoderULUT.cc:(.text+0x168b): undefined reference to aloTPGTranscoderULUT::LSB_HBHE'
    src/CalibCalorimetry/CaloTPG/src/CaloTPGTranscoderULUT.cc:(.text+0x16b6): undefined reference to aloTPGTranscoderULUT::LSB_HF'

Before:

    $ nm -a ../lib/slc6_amd64_gcc493/libCalibCalorimetryCaloTPG.so | wc -l
    153

After:

    $ nm -a ../lib/slc6_amd64_gcc493/libCalibCalorimetryCaloTPG.so | wc -l
    155

    $ nm -a ../lib/slc6_amd64_gcc493/libCalibCalorimetryCaloTPG.so | c++filt | grep -E '(LSB_HF|LSB_HBHE)'
    000000000000871c R CaloTPGTranscoderULUT::LSB_HF
    0000000000008718 R CaloTPGTranscoderULUT::LSB_HBHE

Signed-off-by: David Abdurachmanov <David.Abdurachmanov@cern.ch>
@cmsbuild
Copy link
Contributor

Pull request #13044 was updated. @diguida, @cerminar, @cmsbuild, @franzoni, @mmusich, @mulhearn, @davidlange6 can you please check and sign again.

@davidlt
Copy link
Contributor Author

davidlt commented Jan 24, 2016

Fixed and tested on GCC and Clang.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@mulhearn
Copy link
Contributor

+1

@mmusich
Copy link
Contributor

mmusich commented Jan 25, 2016

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_0_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @Degano, @smuzaffar

@davidlange6
Copy link
Contributor

+1

cmsbuild added a commit that referenced this pull request Jan 25, 2016
CalibCalorimetry/CaloTPG: definite static const float members
@cmsbuild cmsbuild merged commit 4c86b01 into cms-sw:CMSSW_8_0_X Jan 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants