Closed kutschke closed 4 months ago
@sophiemiddleton when you get a chance, please to a test merge in your own area and verify that my use of #ifndef __ROOTCLING__
in inc/CollectionFiller.hh did not break anything. I am not sure how to test it. I expect that it will be OK.
Yes I will do that before merging
@sophiemiddleton The CollectionFiller class is included in classes_def.xml and classes.h. I am curious why it needs to be. Do you call it from the GUI or command line?
Its used to pass in the arguments from fcl
Its used to pass in the arguments from fcl
Perhaps I am missing something really simply. What functionality exists that requires that CollectionFiller be visible to genreflex? Is there a way to change values inside of the instance that lives in REveEventDisplay_module from the root command line or the GUI?
There is the explanation from Kyle about why changes to fhiclcpp trigger the error on this release:
with art 3.15, we are using C++20 concepts to constrain the types of arguments allowed for function templates. ROOT's genreflex and rootcling functionalities do not yet understand C++20 concepts; hence the failures. I'm glad you were able to get around the issue–we've tried to avoid using C++20 concepts in places where dictionaries will be generated...but that does require some reworking of code.
SInce the code builds with my changes it appears that we have no other fhicl code exposed to dictionary builders.
My updates to the PR are in. Modulo responding to comments and requests, this is done. The most recent changes are
You need to put the test of NUMERIC_ROOT_VERION after the #include to defines the class. I suggest putting it after all includes. A corollary is that we cannot make one file with all of the usings - we need to put them in files as needed. If it's only needed in a .cc file, put it there, not in the .hh. Of course there will be times when it is needed in the .hh
Sophie: please do a test merge and check if everything behaves as expected.
@rlcee Please have a look at the machinery of NUMERIC_ROOT_VERION . Is this a viable solution moving forward? Can we do it better?
@kutschke this is currently a draft, let me know when it moves to a real PR
@kutschke this is currently a draft, let me know when it moves to a real PR
It's ready to do. I just un-drafted it.
I see there are conflicts - I will look into it after the meeting
I rebased on top of the new HEAD of main. This overwrote the commit history in the commit tab above.
To push to my fork I had to add the --force-with-lease option to my push command.
This PR builds with both art v3_14_03 and v3_15_00 on SL7.
This PR is a companion to Offline PR 1257. The two can be merged independently.
Make changes to that are required for art suite v3_15_00 that upgrades root to v6_30_06. The submitted code builds with both the new art suite and the with the current art suite v3_14_03. I have not yet run-time tested the REve changes.
In most cases the problem was that fhicl code was exposed to dictionary makers because of people having includes that were either unneeded or were in the .hh file rather than the .cc file, where they belonged. I don't understand the details of what in fhicl is causing the problem but there is no reason for fhicl code to be exposed to dictionary makers.
The one special case was inc/CollectionFiller.hh. In this case I used
#ifndef __ROOTCLING__
to hide the offending code from root.There is one more commit coming about dealing with the movement of some classes from ROOT::Experimental to ROOt:: . I am still working on it and this will be in draft status until it is resolved.