Skip to content

Conversation

@tangxifan
Copy link
Contributor

@tangxifan tangxifan commented Apr 6, 2023

Description

Due to the compiler version upgrades in Ubuntu 22.04, build compatibility should be restored.
In this PR, the build compatibility is expanded to

  • gcc-12
  • clang-11
  • clang-12
  • clang-13
  • clang-14

Related Issue

To address issue #2270

Motivation and Context

How Has This Been Tested?

Types of changes

  • Bug fix (change which fixes an issue)
  • New feature (change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

@github-actions github-actions bot added the infra Project Infrastructure label Apr 6, 2023
@tangxifan
Copy link
Contributor Author

@vaughnbetz @hzeller I managed to reproduce the error using gcc-12 on my local machine. Also you can see the error on CI machine. As you predicted, it is indeed related to the bool type of swap. I am working on it now.

@tangxifan
Copy link
Contributor Author

Following the suggestions in https://stackoverflow.com/questions/58660207/why-doesnt-stdswap-work-on-vectorbool-elements-under-clang-win

I am using:

std::vector<bool>::swap()

Now the problem is resolved. Let me know if you have any preference.

@tangxifan tangxifan mentioned this pull request Apr 7, 2023
7 tasks
@vaughnbetz
Copy link
Contributor

Great, thanks @tangxifan . For some reason the template function matching must not have been finding the bool specialization until you put in the more explicit request. I think this is now ready to be merged ... let me know if you want me to merge it now.

@tangxifan
Copy link
Contributor Author

Great, thanks @tangxifan . For some reason the template function matching must not have been finding the bool specialization until you put in the more explicit request. I think this is now ready to be merged ... let me know if you want me to merge it now.

Hi @vaughnbetz Before merging, I just need to fix a minor issue on clang-11 which is found on the CI machine. It should be quick.

@tangxifan
Copy link
Contributor Author

tangxifan commented Apr 8, 2023

@vaughnbetz All the build compatibility tests passed. If you see any needs on updating documentation, just let me know. This PR is ready to merge.

@vaughnbetz vaughnbetz merged commit 84a6883 into master Apr 8, 2023
@vaughnbetz vaughnbetz deleted the xt_ci_build_compatibility branch April 8, 2023 17:00
@vaughnbetz
Copy link
Contributor

Thanks @tangxifan . I've updated the documentation that I think needs updating. If I missed anything that you see please update it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

infra Project Infrastructure libvtrutil

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants