Fixed a problem where AlwaysAssert macro does not like std::unique_pt…#52
Fixed a problem where AlwaysAssert macro does not like std::unique_pt…#52VoronkovMA merged 1 commit intocasacore:masterfrom
Conversation
…r as an expression
|
Self-assigned myself as the reviewer again. I'm fine with the change (although doing it as a one-liner may look better). Ideally, we should push the explicit cast into casacore's AlwaysAssert (and other assert macros). Because casacore's code is old-ish and doesn't use smart pointers much, this issue hasn't surfaced before. But for now it is fine. I'll keep it open for a bit to let others comment. But otherwise, it is ready to merge. |
VoronkovMA
left a comment
VoronkovMA
left a comment
There was a problem hiding this comment.
I'll repeat my in-code comment here because @vuo006 cannot see them for some reason. Although I'd personally prefer a one-liner AlwaysAssert(static_cast(restoredCoords)); if it is done with the intermediate assignment step, technically static_cast is not needed.
| // written with raw pointers did the equivalent thing | ||
| std::unique_ptr<casacore::CoordinateSystem> restoredCoords(casacore::CoordinateSystem::restore(rec, "coords")); | ||
| AlwaysAssert(restoredCoords, casacore::AipsError); | ||
| const bool notNull = static_cast<bool>(restoredCoords); |
There was a problem hiding this comment.
although personally I'd prefer a one-liner AlwaysAssert(static_cast(restoredCoords)); if you have it with the assignment step, you technically don't need the static_cast (it will cast automatically - this didn't work in the original code because the macro uses the type for the template type resolution)
2 participants
The compiler error message generated for this problem is shown below:
casacore/casa/Utilities/Assert.h:156:36: error: no matching function for call to ‘casacore::assert_casacore::AipsError::assert_(std::unique_ptrcasacore::CoordinateSystem&, const char [35], const char [107], casacore::Int)’
156 | {casacore::assert_ dummy_(expr, "Failed AlwaysAssert " #expr,FILE,(casacore::Int)LINE); dummy_.null(); }
casarest/synthesis/Images//ADIOSImage.tcc:289:3: note: in expansion of macro ‘AlwaysAssert’
289 | AlwaysAssert(restoredCoords, casacore::AipsError);