Skip to content

G-API: Disable Windows warnings with 4996 code#20937

Merged
alalek merged 4 commits intoopencv:masterfrom
mpashchenkov:mp/ocv-gapi-warnings
Oct 28, 2021
Merged

G-API: Disable Windows warnings with 4996 code#20937
alalek merged 4 commits intoopencv:masterfrom
mpashchenkov:mp/ocv-gapi-warnings

Conversation

@mpashchenkov
Copy link
Copy Markdown
Contributor

@mpashchenkov mpashchenkov commented Oct 25, 2021 •
edited
Loading

The PR handles problem with Windows warnings.
There are "deprecated" warnings on build.
IE has IInterfaces which are wrapped with Interfaces. For example: IInferRequest and InferRequest:
https://docs.openvino.ai/latest/deprecated.html
G-API module doesn't use IInterfaces (IInferRequest, IExecutableNetwork...) so warnings can't be solved with changing of API usage.

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

Build configuration

force_builders=XCustom,Custom Win,Custom Mac
build_gapi_standalone:Linux x64=ade-0.1.1f
build_gapi_standalone:Win64=ade-0.1.1f
build_gapi_standalone:Mac=ade-0.1.1f
build_gapi_standalone:Linux x64 Debug=ade-0.1.1f

Xbuild_image:Custom=centos:7
Xbuildworker:Custom=linux-1
build_gapi_standalone:Custom=ade-0.1.1f

build_image:Custom=ubuntu-openvino-2021.3.0:20.04
build_image:Custom Win=openvino-2021.4.1-vc14
build_image:Custom Mac=openvino-2021.2.0

test_modules:Custom=gapi,python2,python3,java
test_modules:Custom Win=gapi,python2,python3,java
test_modules:Custom Mac=gapi,python2,python3,java

buildworker:Custom=linux-1
test_opencl:Custom=OFF
test_bigdata:Custom=1
test_filter:Custom=*

alalek
alalek reviewed Oct 25, 2021
View reviewed changes
modules/gapi/CMakeLists.txt Outdated

if(MSVC)
# Disable obsollete warning C4503 popping up on MSVC <<2017
# Disable obsolete warning C4503 popping up on MSVC <<2017
Copy link
Copy Markdown
Member

@alalek alalek Oct 25, 2021

Choose a reason for hiding this comment

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

  1. PR's description should state the problem which you want to resolve here.
  2. "MSVC <<2017": use MSVC version check.
  3. It is better to fix deprecated API usage. Code will be broken in the future.

Copy link
Copy Markdown
Contributor Author

@mpashchenkov mpashchenkov Oct 25, 2021

Choose a reason for hiding this comment

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

  1. Updated;
  2. Added if(MSVC_VERSION LESS 1910) i.e. < visual studio 15 2017;
  3. It is unreal. G-API doesn't use deprecated API. DNN module has the same warnings without disabling (C4996):

    opencv/modules/dnn/CMakeLists.txt

    Line 49 in 6a2077c

    ocv_warnings_disable(CMAKE_CXX_FLAGS /wd4244 /wd4267 /wd4018 /wd4355 /wd4800 /wd4251 /wd4996 /wd4146
    .

Or am i wrong?
Do you propose to associate disabling with IE version (2022.1 release)?

Copy link
Copy Markdown
Member

@alalek alalek Oct 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

The PR handles problem with Windows warnings.

Not sure if it is time to trying fix issues with OV 2021.4.0 + MSVS 2015.
Please check the latest builds, with OV 2021.4.1 + MSVS 2019. There is no such warnings anymore

BTW, https://docs.openvino.ai/latest/openvino_docs_install_guides_installing_openvino_windows.html

Install these dependencies:
Microsoft Visual Studio* 2019 with MSBuild

We need to use the right conditions.
wd4996 is used for IE builds only.
Current patch is a regression for non-IE scope.

Copy link
Copy Markdown
Contributor Author

@mpashchenkov mpashchenkov Oct 25, 2021

Choose a reason for hiding this comment

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

There is no such warnings anymore
BUT

  1. Warnings are reproducible on MS_BUILD of MSVS 2015 (make no sense anymore?). Builds fine with 2019 version.
  2. -- Inference Engine: NO reference
    Is it normal? We won't get warnings without IE.

Copy link
Copy Markdown
Contributor Author

@mpashchenkov mpashchenkov Oct 25, 2021

Choose a reason for hiding this comment

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

I will check the OV 2021.4.1 + MSVS 2019.

Copy link
Copy Markdown
Member

@alalek alalek Oct 25, 2021

Choose a reason for hiding this comment

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

Warnings should be disabled through precise conditions.

For testing with MSVS 2015 / 2019 use this (one of them at once):

build_image:Custom Win=openvino-2021.4.1
build_image:Custom Win=openvino-2021.4.1-vc14

Copy link
Copy Markdown
Member

@alalek alalek Oct 25, 2021

Choose a reason for hiding this comment

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

FYI, no warnings on neighbor PR against OV 2021.2.0 + MSVS2015: http://pullrequest.opencv.org/buildbot/builders/precommit_custom_windows/builds/2760

Copy link
Copy Markdown
Member

@alalek alalek Oct 26, 2021

Choose a reason for hiding this comment

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

Inference Engine: NO

Fixed nightly builds, no warnings observed: http://pullrequest.opencv.org/buildbot/builders/master_openvino-win64/builds/887

Copy link
Copy Markdown
Contributor Author

@mpashchenkov mpashchenkov Oct 27, 2021

Choose a reason for hiding this comment

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

I removed disabling for 4996 and built with build_image:Custom Win=openvino-2021.4.1-vc14,
received "deprecated" warnings.
Conclusion:

  1. No warnings for OV 2021.2.0 + MSVS2015;
  2. No warnings for OV 2021.4.1 + MSVS 2019;
  3. No warnings for OV 2021.4 + MSVS 2019;
  4. Warnings for OV 2021.4.1 + MSVS 2015;
  5. Warnings for OV 2021.4 + MSVS 2015.

Does MSVS 2015 make sense now?

  • If yes, i suggest disable 4996 warnings (like in dnn module), because gapi module doesn't use deprecated API of IE.
  • If no, let's leave as it was and add check if(MSVC_VERSION LESS 1910) for C4503 instead comment.

Copy link
Copy Markdown
Member

@alalek alalek Oct 27, 2021

Choose a reason for hiding this comment

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

Lets add MSVC_VERSION LESS 1910 check for MSVS 2015

alalek
alalek approved these changes Oct 28, 2021
View reviewed changes
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.

Looks good to me! Thank you 👍

@alalek alalek merged commit eb152d7 into opencv:master Oct 28, 2021
@alalek alalek mentioned this pull request Dec 30, 2021
Merged
@alalek alalek mentioned this pull request Feb 22, 2022
Merged
a-sajjad72 pushed a commit to a-sajjad72/opencv that referenced this pull request Mar 30, 2023
Merge pull request opencv#20937 from mpashchenkov:mp/ocv-gapi-warnings
ab47d64 
G-API: Disable Windows warnings with 4996 code

* Windows warnings 4503 and 4996 are disabled with dnn style

* Applying comments to review

* Reproducing

* Added check MSVC_VERSION for both warnings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alalek alalek alalek approved these changes

@TolyaTalamanov TolyaTalamanov Awaiting requested review from TolyaTalamanov

Assignees

@alalek alalek

Labels

category: g-api / gapi

Projects

None yet

Milestone

4.5.5

Development

Successfully merging this pull request may close these issues.

2 participants

@mpashchenkov @alalek