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 circular dependency between utils_ranges_sycl.h and ranges_defs.h #2065

Merged
merged 2 commits into from
Feb 13, 2025

Conversation

mmichel11
Copy link
Contributor

PR #1976 introduced a circular dependency between oneapi/dpl/pstl/hetero/dpcpp/utils_ranges_sycl.h and oneapi/dpl/pstl/ranges_defs.h. This shows up as an issue in kernel templates where the following produces a compiler error:

#include <oneapi/dpl/experimental/kernel_templates>

int main()
{
}

In utils_ranges_sycl.h, the include was added to be able to use __nanorange::nano::ranges::contiguous_range, so we can fix this by directly including the nanorange header. This resolves the circular dependency and the compilation issue.

Copy link
Contributor

@dmitriy-sobolev dmitriy-sobolev left a comment

Choose a reason for hiding this comment

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

LGTM.

utils_ranges_sycl.h defines what ranges_defs.h uses (such as all_view), so utils_ranges_sycl.h should not include ranges_defs.h. Including nanorange.hpp directly looks good.

@akukanov
Copy link
Contributor

The patch is OK as a quick fix,

But in the long run, the fix should be different. Namely, I believe ranges_defs.h should NOT include any utility headers, because its purpose is to provide forward declarations, not implementation parts.

@timmiesmith
Copy link
Contributor

@mmichel11 would you please open an issue capturing @akukanov's direction for the correct fix?

@timmiesmith timmiesmith merged commit f3d47a8 into main Feb 13, 2025
23 checks passed
@timmiesmith timmiesmith deleted the dev/mmichel11/fix_circular_dependency branch February 13, 2025 13:48
timmiesmith pushed a commit that referenced this pull request Feb 13, 2025
@mmichel11
Copy link
Contributor Author

I have opened #2066 which will remove these utility header includes and use forward declarations instead.

timmiesmith added a commit that referenced this pull request Feb 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants