Add C++23 support #17253
Add C++23 support #17253
Conversation
CHN-beta
requested review from
osschar,
linev,
lmoneta,
guitargeek and
dpiparo
as code owners
|
Thanks for these changes. I started the builds and tests on the master branch. |
| @@ -70,8 +70,7 @@ template <class T> | |||
| std::unique_ptr<T> compileForNormSet(T const &arg, RooArgSet const &normSet) | |||
| { | |||
| RooFit::Detail::CompileContext ctx{normSet}; | |||
| std::unique_ptr<RooAbsArg> head = arg.compileForNormSet(normSet, ctx); | |||
| return std::unique_ptr<T>{static_cast<T *>(head.release())}; | |||
| return std::unique_ptr<T>{static_cast<T *>(arg.compileForNormSet(normSet, ctx).release())}; | |||
There was a problem hiding this comment.
Could you please comment about the need of this change?
There was a problem hiding this comment.
Ok, let me explain the need of this change.
In the code without my patch, std::unque_ptr<RooAbsArg> head is constructed in the function, and it is needed to be destructed at the end of the function. However, RooAbsArg was still not defined, so it could not be destructed. In my test (gcc 13, with -std=c++23), this will cause a compile error.
So, generally speaking, we should move the function body to c++ source file. However, this is a function template, simply move the definition into source file will lead to failure in template implicit instantiation. Thus, I removed the definition of local variable head to avoid calling to its destrutor.
Of course, the return value of arg.compileForNormSet(normSet, ctx) is typed std::unqie_ptr and should also be destructed in this function, but this will not cause a compile error in my test. To my understand, this is because compiler did not know the type of the return value of arg.compileForNormSet before T is specified (i.e., before the function template is called and instantiated), so this statement will only be compiled when T is specified (i.e. when the function is actually called, where the template is implicitely instantiated). I am not sure this is a compiler specified behavior or in C++ standard.
Test Results 18 files 18 suites 4d 12h 56m 1s ⏱️ For more details on these failures, see this check. Results for commit 1d2acc9. ♻️ This comment has been updated with latest results. |
No problem. |
Thank you. |
This Pull request:
Changes or fixes:
C++23 support of ROOT (see https://root-forum.cern.ch/t/enable-c-23-in-root-6-34-00-rc1/62433).
Some changes are needed for code using
std::unique_ptr. According to C++ standards, in the context thatTis declared but not defined,std::unique_ptr<T>is constructible but not destructible, thus we need to move the definitions of some functions callingstd::unique_ptr<T>::~unique_ptrfrom the header file into the source file. This rule is not new in C++ 23, but it causes compile failure when-std=c++23is specified, so we must fix it to support C++ 23.Checklist: