Remove defaulted constructors and operators of StdArray #43153

iarspider · 2023-10-31T08:36:45Z

PR description:

Compilation of StdArray_t test fails in CPP20 IBs:

>> Compiling  /data/cmsbld/jenkins/workspace/build-any-ib/w/tmp/BUILDROOT/d3d9f42af53f45146856f52283745854/opt/cmssw/el8_amd64_gcc12/cms/cmssw/CMSSW_13_3_CPP20_X_2023-10-30-1100/src/DataFormats/Common/test/StdArray_t.cpp
CMSSW_CPU_TYPE= /data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/gcc/12.3.1-40d504be6370b5a30e3947a6e575ca28/bin/c++ -c -DGNU_GCC -D_GNU_SOURCE -DTBB_USE_GLIBCXX_VERSION=120301 -DTBB_SUPPRESS_DEPRECATED_MESSAGES -DTBB_PREVIEW_RESUMABLE_TASKS=1 -DTBB_PREVIEW_TASK_GROUP_EXTENSIONS=1 -DBOOST_SPIRIT_THREADSAFE -DPHOENIX_THREADSAFE -DBOOST_MATH_DISABLE_STD_FPCLASSIFY -DBOOST_UUID_RANDOM_PROVIDER_FORCE_POSIX -DCMSSW_GIT_HASH='CMSSW_13_3_CPP20_X_2023-10-30-1100' -DPROJECT_NAME='CMSSW' -DPROJECT_VERSION='CMSSW_13_3_CPP20_X_2023-10-30-1100' -I/data/cmsbld/jenkins/workspace/build-any-ib/w/tmp/BUILDROOT/d3d9f42af53f45146856f52283745854/opt/cmssw/el8_amd64_gcc12/cms/cmssw/CMSSW_13_3_CPP20_X_2023-10-30-1100/src -I/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/pcre/8.43-37eb2e8b73bab83d6645ecfd5d73dcaa/include -isystem/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/boost/1.80.0-826a207b8543c52970cb1f72d50f068c/include -I/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/bz2lib/1.0.6-d065ccd79984efc6d4660f410e4c81de/include -I/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/cppunit/1.15.x-fb84a4bbf5a436317d208e3ef0864e91/include -I/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/libuuid/2.34-27ce4c3579b5b1de2808ea9c4cd8ed29/include -isystem/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/lcg/root/6.29.99-f64c115b93334fa10a42b4d2a530947f/include -isystem/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/tbb/v2021.9.0-7352cece9624dbf25b0dcb67cdd11a91/include -I/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/xz/5.2.5-6f3f49b07db84e10c9be594a1176c114/include -I/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/zlib/1.2.11-51072030b7f93c3ac6c4235f21e413cb/include -I/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/catch2/2.13.6-17102db92de47c6a473c6e67627c548a/include -I/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/fmt/8.0.1-54e94b39f5cf29341bb9c4765764e1ca/include -I/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/md5/1.0.0-5b594b264e04ae51e893b1d69a797ec6/include -I/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/tinyxml2/6.2.0-d17873b4d6a42a43226cf689f82ec1ef/include -O2 -pthread -pipe -Werror=main -Werror=pointer-arith -Werror=overlength-strings -Wno-vla -Werror=overflow -std=c++20 -ftree-vectorize -Werror=array-bounds -Werror=format-contains-nul -Werror=type-limits -fvisibility-inlines-hidden -fno-math-errno --param vect-max-version-for-alias-checks=50 -Xassembler --compress-debug-sections -Wno-error=array-bounds -Warray-bounds -fuse-ld=bfd -msse3 -felide-constructors -fmessage-length=0 -Wall -Wno-non-template-friend -Wno-long-long -Wreturn-type -Wextra -Wpessimizing-move -Wclass-memaccess -Wno-cast-function-type -Wno-unused-but-set-parameter -Wno-ignored-qualifiers -Wno-deprecated-copy -Wno-unused-parameter -Wunused -Wparentheses -Wno-deprecated -Werror=return-type -Werror=missing-braces -Werror=unused-value -Werror=unused-label -Werror=address -Werror=format -Werror=sign-compare -Werror=write-strings -Werror=delete-non-virtual-dtor -Werror=strict-aliasing -Werror=narrowing -Werror=unused-but-set-variable -Werror=reorder -Werror=unused-variable -Werror=conversion-null -Werror=return-local-addr -Wnon-virtual-dtor -Werror=switch -fdiagnostics-show-option -Wno-unused-local-typedefs -Wno-attributes -Wno-psabi -Wno-error=unused-variable -DBOOST_DISABLE_ASSERTS -flto -fipa-icf -flto-odr-type-merging -fno-fat-lto-objects -Wodr  -fPIC  -MMD -MF tmp/el8_amd64_gcc12/src/DataFormats/Common/test/StdArray_t/StdArray_t.cpp.d /data/cmsbld/jenkins/workspace/build-any-ib/w/tmp/BUILDROOT/d3d9f42af53f45146856f52283745854/opt/cmssw/el8_amd64_gcc12/cms/cmssw/CMSSW_13_3_CPP20_X_2023-10-30-1100/src/DataFormats/Common/test/StdArray_t.cpp -o tmp/el8_amd64_gcc12/src/DataFormats/Common/test/StdArray_t/StdArray_t.cpp.o
/data/cmsbld/jenkins/workspace/build-any-ib/w/tmp/BUILDROOT/d3d9f42af53f45146856f52283745854/opt/cmssw/el8_amd64_gcc12/cms/cmssw/CMSSW_13_3_CPP20_X_2023-10-30-1100/src/DataFormats/Common/test/StdArray_t.cpp: In function 'void ____C_A_T_C_H____T_E_S_T____0()':
  /data/cmsbld/jenkins/workspace/build-any-ib/w/tmp/BUILDROOT/d3d9f42af53f45146856f52283745854/opt/cmssw/el8_amd64_gcc12/cms/cmssw/CMSSW_13_3_CPP20_X_2023-10-30-1100/src/DataFormats/Common/test/StdArray_t.cpp:28:45: error: no matching function for call to 'edm::StdArray<int, 4>::StdArray(<brace-enclosed initializer list>)'
    28 |     edm::StdArray<int, 4> array{{0, 1, 2, 3}};
      |                                             ^
In file included from /data/cmsbld/jenkins/workspace/build-any-ib/w/tmp/BUILDROOT/d3d9f42af53f45146856f52283745854/opt/cmssw/el8_amd64_gcc12/cms/cmssw/CMSSW_13_3_CPP20_X_2023-10-30-1100/src/DataFormats/Common/test/StdArray_t.cpp:4:
/data/cmsbld/jenkins/workspace/build-any-ib/w/tmp/BUILDROOT/d3d9f42af53f45146856f52283745854/opt/cmssw/el8_amd64_gcc12/cms/cmssw/CMSSW_13_3_CPP20_X_2023-10-30-1100/src/DataFormats/Common/interface/StdArray.h:64:15: note: candidate: 'constexpr edm::StdArray<T, N>::StdArray(edm::StdArray<T, N>&&) [with T = int; long unsigned int N = 4]'
   64 |     constexpr StdArray(StdArray<T, N>&& init) = default;
      |               ^~~~~~~~
/data/cmsbld/jenkins/workspace/build-any-ib/w/tmp/BUILDROOT/d3d9f42af53f45146856f52283745854/opt/cmssw/el8_amd64_gcc12/cms/cmssw/CMSSW_13_3_CPP20_X_2023-10-30-1100/src/DataFormats/Common/interface/StdArray.h:64:41: note:   no known conversion for argument 1 from '<brace-enclosed initializer list>' to 'edm::StdArray<int, 4>&&'
   64 |     constexpr StdArray(StdArray<T, N>&& init) = default;
      |                        ~~~~~~~~~~~~~~~~~^~~~
/data/cmsbld/jenkins/workspace/build-any-ib/w/tmp/BUILDROOT/d3d9f42af53f45146856f52283745854/opt/cmssw/el8_amd64_gcc12/cms/cmssw/CMSSW_13_3_CPP20_X_2023-10-30-1100/src/DataFormats/Common/interface/StdArray.h:61:15: note: candidate: 'constexpr edm::StdArray<T, N>::StdArray(const edm::StdArray<T, N>&) [with T = int; long unsigned int N = 4]'
   61 |     constexpr StdArray(StdArray<T, N> const& init) = default;
      |               ^~~~~~~~
/data/cmsbld/jenkins/workspace/build-any-ib/w/tmp/BUILDROOT/d3d9f42af53f45146856f52283745854/opt/cmssw/el8_amd64_gcc12/cms/cmssw/CMSSW_13_3_CPP20_X_2023-10-30-1100/src/DataFormats/Common/interface/StdArray.h:61:46: note:   no known conversion for argument 1 from '<brace-enclosed initializer list>' to 'const edm::StdArray<int, 4>&'
   61 |     constexpr StdArray(StdArray<T, N> const& init) = default;
      |                        ~~~~~~~~~~~~~~~~~~~~~~^~~~
/data/cmsbld/jenkins/workspace/build-any-ib/w/tmp/BUILDROOT/d3d9f42af53f45146856f52283745854/opt/cmssw/el8_amd64_gcc12/cms/cmssw/CMSSW_13_3_CPP20_X_2023-10-30-1100/src/DataFormats/Common/interface/StdArray.h:58:15: note: candidate: 'constexpr edm::StdArray<T, N>::StdArray() [with T = int; long unsigned int N = 4]'
   58 |     constexpr StdArray() = default;
      |               ^~~~~~~~

This happens because the conditions for enabling aggregate initialization changed in C++20 compared to C++17:

no user-provided, inherited, or explicit constructors - until C++20
no user-declared or inherited constructors - since C++20

This PR conditionally removes user-declared constructors when code is compiled with C++20.

PR validation:

Bot tests

cmsbuild · 2023-10-31T08:44:09Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43153/37450

This PR adds an extra 24KB to repository

cmsbuild · 2023-10-31T08:44:37Z

A new Pull Request was created by @iarspider for master.

It involves the following packages:

DataFormats/Common (core)

@Dr15Jones, @makortel, @cmsbuild, @smuzaffar can you please review it and eventually sign? Thanks.
@makortel, @missirol, @rovere, @wddgit this is something you requested to watch as well.
@rappoccio, @antoniovilela, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

iarspider · 2023-10-31T08:46:23Z

@fwyzard FYI

iarspider · 2023-10-31T09:04:50Z

please test for CMSSW_13_3_CPP20_X

fwyzard · 2023-10-31T09:36:10Z

I'm starting to hate c++20, and we are not even using it yet.

This is the second breaking change they have introduced between c++17 and c++20 "just because"... ok, so maybe it would be more fair to say that I really dislike the approach followed by the c++ standard committee over the past years - which is true irrespective of these specific breaking changes.

fwyzard · 2023-10-31T09:37:17Z

@iarspider can you just remove the whole // Constructors section and all the defaulted constructors, unconditionally of c++20 ?

fwyzard · 2023-10-31T09:40:24Z

i.e.

change line 55 to // Assignment operators
delete lines from 56 to 64 inclusive

iarspider · 2023-10-31T09:40:29Z

@iarspider can you just remove the whole // Constructors section and all the defaulted constructors, unconditionally of c++20 ?

done

iarspider · 2023-10-31T09:40:37Z

please test for CMSSW_13_3_CPP20_X

iarspider · 2023-10-31T09:41:54Z

please test for CMSSW_13_3_CPP20_X

cmsbuild · 2023-10-31T09:47:38Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43153/37453

This PR adds an extra 20KB to repository

cmsbuild · 2023-10-31T09:48:00Z

Pull request #43153 was updated. @makortel, @smuzaffar, @Dr15Jones can you please check and sign again.

iarspider · 2023-10-31T13:37:56Z

please test

cmsbuild · 2023-10-31T13:42:33Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43153/37458

This PR adds an extra 20KB to repository

fwyzard · 2023-10-31T13:42:34Z

I understand, but I assumed that if you remove the defaulted assignment operators and provide user-specific ones, the compiler will not implement the default ones.

I might be wrong :-)

cmsbuild · 2023-10-31T13:43:00Z

Pull request #43153 was updated. @Dr15Jones, @smuzaffar, @makortel can you please check and sign again.

makortel · 2023-10-31T13:43:23Z

@iarspider You need to remove the copy and move assignment operators as well.

But then we cannot have the assignment from std::array, can we ?

I hope the assignment operators from std::array do not qualify for "copy assignment" or "move assignment". cppreference seems to confirm (emphasis mine)

https://en.cppreference.com/w/cpp/language/copy_assignment

A copy assignment operator is a non-template non-static member function with the name operator= that can be called with an argument of the same class type and copies the content of the argument without mutating the argument.

https://en.cppreference.com/w/cpp/language/move_assignment

A move assignment operator is a non-template non-static member function with the name operator= that can be called with an argument of the same class type and copies the content of the argument, possibly mutating the argument.

Dr15Jones · 2023-10-31T13:50:56Z

compiler explorer shows that the following compiles

https://godbolt.org/z/GWdj76afE

iarspider · 2023-10-31T14:28:07Z

please test for CMSSW_13_3_CPP20_X

cmsbuild · 2023-10-31T17:04:16Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b44ba4/35528/summary.html
COMMIT: d2e0ce4
CMSSW: CMSSW_13_3_X_2023-10-31-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/43153/35528/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially removed 22 lines from the logs
Reco comparison results: 26 differences found in the comparisons
DQMHistoTests: Total files compared: 50
DQMHistoTests: Total histograms compared: 3362691
DQMHistoTests: Total failures: 1067
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3361602
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
Checked 214 log files, 167 edm output root files, 50 DQM output files
TriggerResults: no differences found

makortel · 2023-10-31T17:55:04Z

Comparison differences are related to #39803

makortel · 2023-10-31T17:56:17Z

+core

cmsbuild · 2023-10-31T17:56:41Z

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @rappoccio, @antoniovilela, @sextonkennedy (and backports should be raised in the release meeting by the corresponding L2)

cmsbuild · 2023-10-31T18:07:19Z

-1

Failed Tests: Build
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b44ba4/35531/summary.html
COMMIT: d2e0ce4
CMSSW: CMSSW_13_3_CPP20_X_2023-10-30-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/43153/35531/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b44ba4/35531/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b44ba4/35531/git-merge-result

Build

I found compilation error when building:

>> Building shared library tmp/el8_amd64_gcc12/src/HeterogeneousTest/CUDADevice/src/HeterogeneousTestCUDADevice/libHeterogeneousTestCUDADevice.so
Copying tmp/el8_amd64_gcc12/src/HeterogeneousTest/CUDADevice/src/HeterogeneousTestCUDADevice/libHeterogeneousTestCUDADevice.so to productstore area:
Leaving library rule at HeterogeneousTest/CUDADevice
>> Compiling  /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_13_3_CPP20_X_2023-10-30-1100/src/HeterogeneousTest/CUDADevice/test/testDeviceAddition.cu
nvcc error   : 'cudafe++' died due to signal 9 (Kill signal)
gmake: *** [tmp/el8_amd64_gcc12/src/HeterogeneousTest/CUDADevice/test/testCudaDeviceAddition/testDeviceAddition.cu.o] Error 1
>> Cuda Device Link tmp/el8_amd64_gcc12/src/HeterogeneousTest/CUDADevice/test/testCudaDeviceAddition/testCudaDeviceAddition_cudadlink.o 
nvlink fatal   : Could not open input file 'tmp/el8_amd64_gcc12/src/HeterogeneousTest/CUDADevice/test/testCudaDeviceAddition/testDeviceAddition.cu.o' (target: sm_60)
gmake: *** [tmp/el8_amd64_gcc12/src/HeterogeneousTest/CUDADevice/test/testCudaDeviceAddition/testCudaDeviceAddition_cudadlink.o] Error 1
>> Building binary testCudaDeviceAddition
/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02809/el8_amd64_gcc12/external/gcc/12.3.1-40d504be6370b5a30e3947a6e575ca28/bin/../lib/gcc/x86_64-redhat-linux-gnu/12.3.1/../../../../x86_64-redhat-linux-gnu/bin/ld.bfd: cannot find tmp/el8_amd64_gcc12/src/HeterogeneousTest/CUDADevice/test/testCudaDeviceAddition/testDeviceAddition.cu.o: No such file or directory

iarspider · 2023-11-01T08:15:15Z

-1

Failed Tests: Build Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b44ba4/35531/summary.html COMMIT: d2e0ce4 CMSSW: CMSSW_13_3_CPP20_X_2023-10-30-1100/el8_amd64_gcc12

cudafe++ fails in IBs as well

antoniovilela · 2023-11-01T14:34:17Z

+1

Remove defaulted constructors of StdArray for C++20

33eb0fc

iarspider changed the title ~~Remove defaulted constructors of StdArray for C++20~~ Conditionally remove defaulted constructors of StdArray for C++20 Oct 31, 2023

cmsbuild added this to the CMSSW_13_3_X milestone Oct 31, 2023

cmsbuild added core-pending pending-signatures tests-pending orp-pending code-checks-pending labels Oct 31, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 31, 2023

cmsbuild added tests-started and removed tests-pending labels Oct 31, 2023

Remove constructors unconditionally

9df2d4f

cmsbuild added code-checks-pending and removed code-checks-approved labels Oct 31, 2023

Update StdArray.h

07d9aba

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 31, 2023

cmsbuild added tests-pending code-checks-pending labels Oct 31, 2023

cmsbuild added tests-started and removed tests-pending labels Oct 31, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 31, 2023

cmsbuild added tests-approved and removed tests-started labels Oct 31, 2023

cmsbuild added core-approved fully-signed and removed core-pending pending-signatures labels Oct 31, 2023

cmsbuild added orp-approved and removed orp-pending labels Nov 1, 2023

cmsbuild merged commit fc8a26b into cms-sw:master Nov 1, 2023
12 of 13 checks passed

iarspider deleted the patch-12 branch November 1, 2023 15:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove defaulted constructors and operators of StdArray #43153

Remove defaulted constructors and operators of StdArray #43153

iarspider commented Oct 31, 2023 •

edited

Loading

cmsbuild commented Oct 31, 2023

cmsbuild commented Oct 31, 2023

iarspider commented Oct 31, 2023

iarspider commented Oct 31, 2023

fwyzard commented Oct 31, 2023

fwyzard commented Oct 31, 2023 •

edited

Loading

fwyzard commented Oct 31, 2023

iarspider commented Oct 31, 2023

iarspider commented Oct 31, 2023

iarspider commented Oct 31, 2023

cmsbuild commented Oct 31, 2023

cmsbuild commented Oct 31, 2023

iarspider commented Oct 31, 2023

cmsbuild commented Oct 31, 2023

fwyzard commented Oct 31, 2023

cmsbuild commented Oct 31, 2023

makortel commented Oct 31, 2023

Dr15Jones commented Oct 31, 2023

iarspider commented Oct 31, 2023

cmsbuild commented Oct 31, 2023

makortel commented Oct 31, 2023

makortel commented Oct 31, 2023

cmsbuild commented Oct 31, 2023

cmsbuild commented Oct 31, 2023

iarspider commented Nov 1, 2023

antoniovilela commented Nov 1, 2023

Remove defaulted constructors and operators of StdArray #43153

Remove defaulted constructors and operators of StdArray #43153

Conversation

iarspider commented Oct 31, 2023 • edited Loading

PR description:

PR validation:

cmsbuild commented Oct 31, 2023

cmsbuild commented Oct 31, 2023

iarspider commented Oct 31, 2023

iarspider commented Oct 31, 2023

fwyzard commented Oct 31, 2023

fwyzard commented Oct 31, 2023 • edited Loading

fwyzard commented Oct 31, 2023

iarspider commented Oct 31, 2023

iarspider commented Oct 31, 2023

iarspider commented Oct 31, 2023

cmsbuild commented Oct 31, 2023

cmsbuild commented Oct 31, 2023

iarspider commented Oct 31, 2023

cmsbuild commented Oct 31, 2023

fwyzard commented Oct 31, 2023

cmsbuild commented Oct 31, 2023

makortel commented Oct 31, 2023

Dr15Jones commented Oct 31, 2023

iarspider commented Oct 31, 2023

cmsbuild commented Oct 31, 2023

Comparison Summary

makortel commented Oct 31, 2023

makortel commented Oct 31, 2023

cmsbuild commented Oct 31, 2023

cmsbuild commented Oct 31, 2023

Build

iarspider commented Nov 1, 2023

antoniovilela commented Nov 1, 2023

iarspider commented Oct 31, 2023 •

edited

Loading

fwyzard commented Oct 31, 2023 •

edited

Loading