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

Fix for iterators with const-qualified value type #1527

Merged
merged 3 commits into from
Apr 25, 2024

Conversation

danhoeflinger
Copy link
Contributor

@danhoeflinger danhoeflinger commented Apr 24, 2024

Iterator inputs to parallel APIs with a device policy which have a const-qualified value type are experiencing build errors on with MSVC or GCC standard library implementation because of static assertions in the class definition for std::vector.

Our check for USM allocated vector iterators checks all input iterators, and within the check, instantiates a std::vector with a value type which matches the input iterator's value type. Prior to this PR, we do not protect against "ill formed" vector instantiations, like const-qualified, reference, or function types. When the value type of the input iterator are any of these, the build hits std::vector static assertions while attempting to determine if the iterator is a USM allocator std::vector iterator.
This bug was introduced in #1438.

This PR adds a short circuit to avoid such ill-formed std::vector instantiations for specific value types (reference, const, and function types). These will not be USM allocated std::vector iterators, so it should not effect our ability to detect those types.

This fixes the bug reproducer as reported. I have also confirmed that input data processing still works as intended using #1429.

@danhoeflinger danhoeflinger added this to the 2022.6.0 milestone Apr 24, 2024
@danhoeflinger danhoeflinger marked this pull request as ready for review April 24, 2024 16:33
@danhoeflinger danhoeflinger changed the title Fix for iterators with constant value type Fix for iterators with const-qualified value type Apr 24, 2024
Signed-off-by: Dan Hoeflinger <[email protected]>
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.

LGTM

@danhoeflinger
Copy link
Contributor Author

This also affects GCC standard library implementation in addition to MSVC. The PR fixes both. Libc++ does not have a static assertion to check constness of value type.

Copy link
Contributor

@timmiesmith timmiesmith left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you.

@danhoeflinger danhoeflinger merged commit 141aa38 into main Apr 25, 2024
20 checks passed
@danhoeflinger danhoeflinger deleted the dev/dhoeflin/fix_const_valtype_iters branch April 25, 2024 16:31
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