Project

General

Profile

Necessary Maintenance #24524

Clang-tidy issue cert-err60-cpp

Added by Eric Flumerfelt about 2 months ago. Updated 27 days ago.

Status:
Closed
Priority:
Normal
Assignee:
Target version:
Start date:
06/09/2020
Due date:
% Done:

100%

Estimated time:
Spent time:
Experiment:
-
SSI Package:
Duration:

Description

A minor issue, but it would be nice to have cet::exception conform to this requirement...

/home/eflumerf/Desktop/artdaq-mrb-base/srcs/artdaq/artdaq/DAQrate/SharedMemoryEventManager.cc:96:9: warning: thrown exception type is not nothrow copy constructible [cert-err60-cpp]
                throw cet::exception(app_name + "_SharedMemoryEventManager") << "Unable to attach to Shared Memory!";

History

#1 Updated by Kyle Knoepfel about 2 months ago

  • Status changed from New to Feedback

Eric, thank you for bringing this to our attention. This issue requires some teasing out, and the following links are relevant to this discussion:

  1. cert-err60-cpp
  2. C++ core guideline E.12

Reference 1 discusses the dangers of allowing an exception to escape the exception type's copy constructor. Indeed, if the copy constructor throws an exception that is not handled within the constructor, an abrupt program termination occurs. Reference 1 also presents some solutions to make exception types compliant wrt ERR60, each solution entailing changing the implementation of the exception class.

However, there is at least one more solution possible, which is referred to in reference 2. As the C++ core guideline states, functions should be labeled noexcept when an escaped exception is either impossible or unacceptable. If an exception were to be thrown during copy-construction of a cet::exception object, it would likely be due to memory exhaustion. Under such a circumstance, program recovery is incredibly unlikely. Fortunately, in roughly a decade of art support we are aware of no circumstance ever occurring.

Rewriting the class to avoid program termination on copy construction would be quite awkward. However, we think adding the noexcept specifier to the copy constructor is reasonable. This would:

  • State the intention that no exception escaping the copy constructor is allowed, resulting in program termination if an attempt is made.
  • Silence clang-tidy's warning.

Please let us know if you wish to discuss this further.

#2 Updated by Eric Flumerfelt about 2 months ago

This looks good to me, and I definitely agree that any major API changes at this point would be much more damaging than the as-yet-unseen unexpected termination that the error is warning about. I've been using // NOLINT(cert-err60-cpp) to silence this issue, but if you're willing to add a noexcept, that's much better ;).

#3 Updated by Kyle Knoepfel about 2 months ago

  • Target version set to 3.06.00
  • Assignee set to Kyle Knoepfel
  • Status changed from Feedback to Assigned
  • Tracker changed from Feature to Necessary Maintenance

#4 Updated by Kyle Knoepfel about 2 months ago

  • % Done changed from 0 to 100
  • Status changed from Assigned to Resolved

Implemented with commit cetlib_except:a0584518.

#5 Updated by Kyle Knoepfel 27 days ago

  • Status changed from Resolved to Closed


Also available in: Atom PDF