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: re-allow devicen_t with two color components #654

Merged

Conversation

striezel
Copy link
Contributor

@striezel striezel commented Apr 29, 2022

Description

devicen_t<2> was possible in Boost 1.71, but broke in Boost 1.72.
Now it's possible to use it again.

Since there was no real need to disallow it in the first place, I adjusted the static_assert which prevented it to allow two components, too.

References

Fixes #519.

See that issue for more information.

Tasklist

  • Ensure all CI builds pass
  • Review and approve

devicen_t<2> was possible in Boost 1.71, but broke in Boost 1.72.
Now it's possible to use it again.

Fixes boostorg#519.
@mloskot mloskot added this to the Boost 1.80 milestone May 18, 2022
@mloskot mloskot added the core boost/gil label May 18, 2022
@mloskot
Copy link
Member

mloskot commented May 18, 2022

Despite some jobs fail (w/ older clang and GCC 8 cxxstd=17) I'm happy to merge it. Thanks!

@mloskot mloskot merged commit 3090f86 into boostorg:develop May 18, 2022
@striezel
Copy link
Contributor Author

Thanks for merging it. 👍

The jobs for GCC 8 and older Clang variants also failed before that, see e. g. here: https://github.com/boostorg/gil/actions/runs/2321824140
I don't know why the GCC 8 job fails, but in case of the Clang jobs it's just that there are no GitHub Actions runners with Ubuntu 16.04 anymore, and those older Clang jobs are still trying to use Ubuntu 16.04.

That's why I mentioned that those Clang jobs should possibly be removed in #640 (comment):

Note: Some older Clang versions (Clang 3.8 and older) seem to be unavailable in the APT package repository, so those are not
changed to 18.04. This is intended. Maybe tests for Clang 3.4 to Clang 3.8 should be removed, but that is probably worth another discussion.

If removal of those jobs is not an option, then maybe someone should look into how to build with Clang 3.x on other, still available runners.

@striezel striezel deleted the fix-allow-devicen_t-with-two-channels branch May 18, 2022 09:34
striezel added a commit to striezel-stash/gil that referenced this pull request May 18, 2022
striezel added a commit to striezel-stash/gil that referenced this pull request May 18, 2022
@mloskot
Copy link
Member

mloskot commented May 18, 2022

I don't know why the GCC 8 job fails

We have seen similar failures where a particular compiler version with particular C+ version and build configuration variant generates segfaulting binary or it fails with internal compiler error. Either GIL stretches some compilers to the limits or there is a subtle bug that is hard to reproduce :-)

So, I don't mind disabling such peculiar CI jobs. We are not testing compilers after all.

UPDATE: Done in 4ad824e

mloskot pushed a commit that referenced this pull request May 18, 2022
@mloskot mloskot mentioned this pull request Jul 5, 2022
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core boost/gil
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DeviceN support for N=2 broke in Boost 1.72
2 participants