CARVIEW |
Navigation Menu
-
-
Notifications
You must be signed in to change notification settings - Fork 56.2k
G-API: Disable Windows warnings with 4996 code #20937
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
modules/gapi/CMakeLists.txt
Outdated
@@ -29,13 +29,10 @@ ocv_add_module(gapi | |||
) | |||
|
|||
if(MSVC) | |||
# Disable obsollete warning C4503 popping up on MSVC <<2017 | |||
# Disable obsolete warning C4503 popping up on MSVC <<2017 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- PR's description should state the problem which you want to resolve here.
- "MSVC <<2017": use MSVC version check.
- It is better to fix deprecated API usage. Code will be broken in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Updated;
- Added
if(MSVC_VERSION LESS 1910)
i.e. < visual studio 15 2017; - 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)?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
- Warnings are reproducible on MS_BUILD of MSVS 2015 (make no sense anymore?). Builds fine with 2019 version.
-- Inference Engine: NO
reference
Is it normal? We won't get warnings without IE.
There was a problem hiding this comment.
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
.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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: https://pullrequest.opencv.org/buildbot/builders/precommit_custom_windows/builds/2760
There was a problem hiding this comment.
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: https://pullrequest.opencv.org/buildbot/builders/master_openvino-win64/builds/887
There was a problem hiding this comment.
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:
- No warnings for
OV 2021.2.0 + MSVS2015
; - No warnings for
OV 2021.4.1 + MSVS 2019
; - No warnings for
OV 2021.4 + MSVS 2019
; - Warnings for
OV 2021.4.1 + MSVS 2015
; - 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.
There was a problem hiding this comment.
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
There was a problem hiding this 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 👍
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
The PR handles problem with Windows warnings.
There are "deprecated" warnings on build.
IE has
IInterfaces
which are wrapped withInterfaces
. For example:IInferRequest
andInferRequest
: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
Patch to opencv_extra has the same branch name.
Build configuration