Skip to content

resolves #21038#21107

Merged
alalek merged 4 commits intoopencv:3.4from
take1014:remove_assert_21038
Nov 27, 2021
Merged

resolves #21038#21107
alalek merged 4 commits intoopencv:3.4from
take1014:remove_assert_21038

Conversation

@take1014
Copy link
Copy Markdown
Contributor

@take1014 take1014 commented Nov 23, 2021

resolves #21038

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • There is reference to original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

@take1014
Copy link
Copy Markdown
Contributor Author

take1014 commented Nov 23, 2021

@alalek
I'm getting this warning with compiling on linux opencl.
warning: assuming signed overflow does not occur when assuming that (X + c) < X is always false [-Wstrict-overflow]

on this line:
CV_Assert( (from_idx > to_idx && !elem) || (from_idx < to_idx && elem) );

I can't understand this warning correctly.
I think this warning was generated because it was determined to be unnecessary for optimization.
(When the condition "from_idx <to_idx" is satisfied, the condition of "elem! = 0" is always satisfied in all test pattern..)

May I remove this assert on this line?

Copy link
Copy Markdown
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for contribution!

dc1394error_t err = dc1394_get_control_register(camera, offset, &value);

assert(err == DC1394_SUCCESS);
CV_Assert(err == DC1394_SUCCESS);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this line should be removed.

{
static const int scharr[8] = { 3, 10, 3, -1, 0, 1, 0, 0 }; // extra elements to eliminate "-Warray-bounds" bogus warning
assert( size == 3 );
CV_DbgAssert( size == 3 );
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is OK to use CV_Assert() even for per-pixel processing in tests code.

Comment on lines 2932 to 2933
CV_Assert(fabs(matR[2][1]) < FLT_EPSILON);
matR[2][1] = 0;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use CV_DbgAssert() here (and 2 cases below) to keep code compatible with previous version.
Code itself should be updated to properly handle computation / accuracy issues in a right way (not a part of this patch).

Next line resets checked value to zero anyway.

{
CvSetElem* _elem = (CvSetElem*)elem;
assert( _elem->flags >= 0 /*&& (elem->flags & CV_SET_ELEM_IDX_MASK) < set_header->total*/ );
CV_Assert( _elem->flags >= 0 /*&& (elem->flags & CV_SET_ELEM_IDX_MASK) < set_header->total*/ );
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a C header.
Probably we should left it "as is" as there is no exceptions in C (at last for 3.4 branch).

CvMat m;

assert( (unsigned)CV_MAT_DEPTH(type) <= CV_64F );
CV_Assert( (unsigned)CV_MAT_DEPTH(type) <= CV_64F );
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C header too

if( from_idx == to_idx )
return;
assert( (from_idx > to_idx && !elem) || (from_idx < to_idx && elem) );
CV_Assert( (from_idx > to_idx && !elem) || (from_idx < to_idx && elem) );
Copy link
Copy Markdown
Member

@alalek alalek Nov 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test_ds.cpp:53:58: warning: assuming signed overflow does not occur when assuming that (X + c) < X is always false [-Wstrict-overflow]

Please use this workaround:

    if (elem)
        CV_Assert(from_idx < to_idx);
    else
        CV_Assert(from_idx > to_idx);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alalek
Thanks for your review. I fixed them.

Copy link
Copy Markdown
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you 👍

@alalek alalek merged commit a627737 into opencv:3.4 Nov 27, 2021
@alalek alalek mentioned this pull request Dec 3, 2021
@alalek alalek mentioned this pull request Dec 30, 2021
@alalek alalek mentioned this pull request Feb 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

port/backport done Label for maintainers. Authors of PR can ignore this

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants