Conversation
8ce0e20 to
136c08b
Compare
alalek
left a comment
There was a problem hiding this comment.
More warnings from clang 14 (Ubuntu 22.04) are here: http://pullrequest.opencv.org/buildbot/builders/precommit_custom_linux/builds/100202
modules/ts/include/opencv2/ts.hpp
Outdated
| #if defined(__OPENCV_BUILD) && defined(__clang__) | ||
| #pragma clang diagnostic push | ||
| #pragma clang diagnostic ignored "-Winconsistent-missing-override" | ||
| #pragma clang diagnostic ignored "-Wdeprecated-copy" |
There was a problem hiding this comment.
Need to merge it with line 128.
There are 1400+ warnings for clang 6.0: http://pullrequest.opencv.org/buildbot/builders/precommit_custom_linux/builds/100200
There was a problem hiding this comment.
Done, there are only 11 warnings now: 10 in ADE (should be fixed by opencv/ade#33) and 1 in sample.
There was a problem hiding this comment.
I have not checked clang-6 though, will try it later. Checked with clang-6, it should be clean now.
modules/ts/include/opencv2/ts.hpp
Outdated
| #endif | ||
|
|
||
| #if defined(__OPENCV_BUILD) && defined(__clang__) | ||
| #pragma clang diagnostic push |
There was a problem hiding this comment.
Missing push/pop is here for a reason: to avoid massive patching of legacy tests with override.
$ grep -Rni prepare_to_validation ./modules/ | wc -l
135
There was a problem hiding this comment.
This is strange, but it works for me, I don't get warnings related to prepare_to_validation missing override. I tried clang-6, clang-14, clang-17 and only warnings I can see are from ADE.
| return cmp(pts_[idx1], pts_[idx2]); | ||
| } | ||
| private: | ||
| KeypointComparator& operator=(const KeypointComparator&) = delete; |
There was a problem hiding this comment.
Removal is not an option for = delete.
Usually it is a guard, because default compiler generated code is not correct and we should not use copying at all.
Try to declare deleted copy ctor.
There was a problem hiding this comment.
In this specific case I thought that this class is only used in a single place to sort keypoints and nobody actually use operator= because it is also private. So I decided that probably this was just a habit of implementing safe class.
Maybe it would be better to rewrite sort like this? What do you think?
comparators::KeypointGreater cmp;
std::sort(idxs.data(), idxs.data() + desc.rows,
[&](int lhs, int rhs){ return cmp(pts[lhs], pts[rhs]); });
There was a problem hiding this comment.
There is no labmda's on 3.4 branch (no C++11)
| } | ||
| } | ||
|
|
||
| DepthFrameProcessor::~DepthFrameProcessor() |
There was a problem hiding this comment.
It is better to drop noexcept from the header file instead.
136c08b to
e4acd74
Compare
Numerous warnings (mostly in tests) have been produced when compiling with clang 14. Most are about missing
override.Update: added opencv/opencv_contrib#3436 with more warnings fix