-
Notifications
You must be signed in to change notification settings - Fork 29
Add K4_GAUDI_CHECK macro. #341
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see the utility of this, but I think we might need to
- Adapt the name.
K4_CHECK
almost implies that it's usable anywhere in Key4hep, but it only works within Algorithms (which is going to be a large part of things in Key4hep). MaybeK4_GAUDI_CHECK
, but I am also not entirely happy with that. - Make it accept a message argument or provide a version that takes a user message and adds that to the error message as well.
For a little context, this is a much-simplified version of the https://gitlab.cern.ch/atlas/athena/-/blob/main/Control/AthenaKernel/AthenaKernel/errorcheck.h?ref_type=heads (Having both names is historical; we'd use just ATH_CHECK if we were doing You're right in that this can only be used in a Gaudi component. In ATLAS, I could add an option to give a custom error message. However, (In passing, i have some pending calorimeter changes which will use this.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could add an option to give a custom error message. However,
at ATLAS i've never really felt the need for that (giving the failed expression
along with the line number is generally sufficient), so i'd prefer to keep
this simple to start with and extend later if desired.
Yeah, that makes sense. Usually one of the first thing you do in any case is to grep for the error message and hope for only a handful of results. Also if it allows us to keep boost preprocessor out for now, I am happy.
Add a macro to reduce repetitive code involved in checking for error return statuses in Gaudi components. This allows for example to replace code like ``` if (!m_cellPositionsTool.retrieve()) { error() << "Unable to retrieve cell positions tool." << endmsg; return StatusCode::FAILURE; } ``` with ``` K4_GAUDI_CHECK( m_cellPositionsTool.retrieve() ); ```
0f8bfba
to
048e0aa
Compare
Squashed. |
If this is used in ATLAS and now here, doesn't it make sense that we should have something like this in Gaudi? In some cases we do not always return. I see that in ATLAS every possible one is defined. How does it look in ATLAS, is it only these macros? |
We could consider that, i guess, but using a project-specific name allows for customization later, eg., for error reporting (a la ATLAS).
Not quite sure what you're getting at here. Yes, one sometimes need to be aware that the
https://acode-browser1.usatlas.bnl.gov/lxr/search?%21v=head&_filestring=&_string=ATH_CHECK as I said mostly in |
Add a macro to reduce repetitive code involved in checking for error return statuses in Gaudi components.
This allows for example to replace code like
with
BEGINRELEASENOTES
ENDRELEASENOTES