Skip to content
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

Silence negators deprecation warnings #2071

Closed
wants to merge 1 commit into from

Conversation

dmitriy-sobolev
Copy link
Contributor

@dmitriy-sobolev dmitriy-sobolev commented Feb 17, 2025

It suppresses the warning:

oneDPL\include\oneapi\dpl\pstl../functional(51): warning C4996: 'std::binary_negate': warning STL4008: std::not1(), std::not2(), std::unary_negate, and std::binary_negate are deprecated in C++17. They are superseded by std::not_fn(). You can define _SILENCE_CXX17_NEGATORS_DEPRECATION_WARNING or _SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to suppress this warning.

@dmitriy-sobolev dmitriy-sobolev force-pushed the dev/dmitriy-sobolev/silence-warnings branch 2 times, most recently from 64f3c8c to e73191c Compare February 17, 2025 17:13
@dmitriy-sobolev dmitriy-sobolev force-pushed the dev/dmitriy-sobolev/silence-warnings branch from e73191c to ce02821 Compare February 17, 2025 17:21
Comment on lines +21 to +22
// Silence the deprecation warnings of the negators injected from <functional>
// The macro definition is contained in this header to avoid redefinition errors on the user side
Copy link
Contributor

Choose a reason for hiding this comment

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

In my understanding - please correct if I am wrong - the "silence" macro effectively does not add the deprecation attribute to the negation functions. That means that a) if the standard header has been included before oneDPL headers, the warnings will still be there, and b) if we include the header with the "silence" macro defined, it will impact the whole translation unit anyway.

The direct conclusion from (b) is that undefining the macro makes no good whatsoever, and maybe even harms, by disabling the warnings and leaving no clue it has been made.

Copy link
Contributor

@akukanov akukanov Feb 19, 2025

Choose a reason for hiding this comment

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

Overall, I think we should not do this suppression. Since we position our headers as substitutes for the standard ones, in most if not all cases we should not change the behavior of the standard headers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Overall, I think we should not do this suppression. Since we position our headers as substitutes for the standard ones, in most if not all cases we should not change the behavior of the standard headers.

The suppression was questionable, and I wanted to hear an argument like that. I agree with it. Let me close the PR then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants