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

Removing noexcept to resolve issue reported by static analysis. #1462

Merged
merged 2 commits into from
Mar 22, 2024

Conversation

timmiesmith
Copy link
Contributor

@timmiesmith timmiesmith commented Mar 21, 2024

The pattern is used by uninitialized_fill, which uses placement new to created objects, and new may throw a length_error exception. Issue was reported by static analysis tools.

Copy link
Contributor

@mmichel11 mmichel11 left a comment

Choose a reason for hiding this comment

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

The equivalent noexcept for the declaration in algorithm_fwd.h should also be removed. This should resolve the CI failures.

Copy link
Contributor

@SergeyKopienko SergeyKopienko left a comment

Choose a reason for hiding this comment

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

What about noexcept from the files for some patterns:

  • include/oneapi/dpl/pstl/numeric_impl.h
  • include/oneapi/dpl/pstl/experimental/internal/for_loop_impl.h

Of course, forward declarations should be fixed too.

…oexcept from forward declaration of __pattern_walk1_n
@timmiesmith
Copy link
Contributor Author

What about noexcept from the files for some patterns:

  • include/oneapi/dpl/pstl/numeric_impl.h
  • include/oneapi/dpl/pstl/experimental/internal/for_loop_impl.h

Of course, forward declarations should be fixed too.

If the patterns aren't used with functors that perform memory allocation then the noexcept is valid. I'm not looking for a wide removal of the keyword, just removing it where we identify that an exception may indeed be thrown in the use cases we're testing.

@SergeyKopienko SergeyKopienko self-requested a review March 22, 2024 13:28
Copy link
Contributor

@SergeyKopienko SergeyKopienko left a comment

Choose a reason for hiding this comment

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

LGTM

@timmiesmith timmiesmith dismissed mmichel11’s stale review March 22, 2024 13:32

Changed were made by @SergeyKopienko to address feedback.

@timmiesmith timmiesmith merged commit 91c3a9a into main Mar 22, 2024
20 checks passed
@timmiesmith timmiesmith deleted the static_analysis_fix branch March 22, 2024 13:33
dmitriy-sobolev pushed a commit that referenced this pull request Mar 22, 2024
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.

3 participants