Fix compilation warnings on LUMI with cce/17 #141
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #141 +/- ##
========================================
Coverage 63.69% 63.70%
========================================
Files 1065 1065
Lines 55140 55142 +2
Branches 4085 4085
========================================
+ Hits 35122 35126 +4
+ Misses 20018 20016 -2 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
this change adds move semantics, ctor and assignment.
previously, it was only copy
which is unusual for a class with a shared_ptr member
There was a problem hiding this comment.
Which point(s) are you referring to that are unusual with shared_ptr member?
- copy constructor
- assignment operator
- move constructor
- move-assignment operator
And why?
There was a problem hiding this comment.
using move semantics (3 and 4) on a class with a member of shared_ptr type looks unusual to me
usual use of shared_ptr type objects is to copy it around, which increments count; not move it around
There was a problem hiding this comment.
so, it is a big change of how this class works.
I like (and also prefer) the explicit definitions. it tells others it's intentional and clear.
I suggest, move ctor and assignment = delete;
There was a problem hiding this comment.
Sure, I will remove it. I just added it for completeness as the rule-of-five.
There was a problem hiding this comment.
Marking it with =delete does not work and causes further errors. So I suggest leaving it with =default marks that this is the intended and supported behaviour.
|
This PR has now gone through all the relevant tests successfully. |
This fixes a few warnings when compiling eckit on LUMI with CrayClang compiler cce/17.
One warning was for unrecognised flag for CrayClang as opposed to "just" Cray.
Another related to deprecated implicit assignment operator.