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.

Sign up for GitHub

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

Jump to bottom

Add pmr image typedefs #529

Merged
merged 5 commits into from
Jun 28, 2022
Merged

Add pmr image typedefs #529

merged 5 commits into from
Jun 28, 2022

Conversation

sdebionne
Copy link
Contributor

@sdebionne sdebionne commented Nov 9, 2020 •
edited by mloskot
Loading

Description

when building with C++17 standard, adds pmr (polymorphic memroy resource) image typedefs (e.g. gil::pmr::gray8_image_t = using image<gray8_pixel_t, false, std::pmr::polymorphic_allocator<unsigned char>).

Fixes #528.

Tasklist

  • Conditionally include <memory_resource> and typedefs
  • Add test case(s)
  • Ensure all CI builds pass
  • Review and approve

@sdebionne sdebionne force-pushed the add-pmr-image branch 2 times, most recently from 56f968d to 979b7f1 Compare May 4, 2022 10:08
@codecov
Copy link

codecov bot commented May 4, 2022 •
edited
Loading

Codecov Report

Merging #529 (59703e6) into develop (8d7034c) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop     #529   +/-   ##
========================================
  Coverage    80.84%   80.84%           
========================================
  Files          116      116           
  Lines         5121     5127    +6     
========================================
+ Hits          4140     4145    +5     
- Misses         981      982    +1

@sdebionne sdebionne marked this pull request as ready for review May 4, 2022 10:38
@sdebionne sdebionne requested a review from mloskot May 4, 2022 11:55
@sdebionne
Copy link
Contributor Author

I would be happy to remove the #if !defined(BOOST_NO_CXX17_HDR_MEMORY_RESOURCE) guards if we decide to move to C++17 😁

@sdebionne sdebionne added the cat/annoyance Not a bug, not a feature, but something that should be improved label May 12, 2022
@sdebionne sdebionne added this to the Boost 1.80 milestone May 12, 2022
mloskot
mloskot approved these changes May 20, 2022
View reviewed changes
Copy link
Member

@mloskot mloskot left a comment

Choose a reason for hiding this comment

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

LGTM

By the way, see #676

@mloskot
Copy link
Member

mloskot commented Jun 26, 2022

I have updated this PR with latest develop and I am going to merge it if no major CI issues are revealed.

@mloskot
Copy link
Member

mloskot commented Jun 27, 2022 •
edited
Loading

The GCC 11 drops the warning:

include/boost/gil/image.hpp:241:12: warning: ‘if constexpr’ only available with ‘-std=c++17’ or ‘-std=gnu++17’
  241 |         if constexpr (std::allocator_traits<Alloc>::propagate_on_container_swap::value)
      |            ^~~~~~~~~

and C++14 compilation mode is failing, obviously. Some #ifdef-s are missing :)

@mloskot mloskot merged commit 4dbf35a into boostorg:develop Jun 28, 2022
@sdebionne
Copy link
Contributor Author

sdebionne commented Jun 28, 2022 •
edited
Loading

Thanks for looking into the issue, std::allocator_traits is C++11 but indeed if constexpr is C++17... I guess I would need to re-implement that part with SFINAE. But since we are switching to C++17 anyway...

@sdebionne sdebionne deleted the add-pmr-image branch June 28, 2022 07:53
mloskot added a commit that referenced this pull request Jun 29, 2022
@mloskot
Merge branch 'develop'
bba6d62 
* develop:
  test: Add more basic cases for image class (#423)
  test: Add virtual_2d_locator fixture; is_2d_traversable test case
  test: Check more properties of indexed_image_view from extension/toolbox
  test: Add basic is_1d_traversable cases for image_view
  chore: Update CMakeSettings.json sample [ci skip]
  chore: Update CMake to use latest cmake-conan/0.18.1 [ci skip]
  Add pmr image typedefs (#529)
  test: Add test cases for image with empty dimensions (#702)
  test: Case test_constructor_from_view was not called
  fix: Memory leak in image class for empty dimensions (#649)
  docs: Bump C++11 to C++14 as current required (#700)
  ci: Remove C++11 build jobs after C++14 switch (#698)
  build: Fix CMake source file extensions must be explicit
  refactor: Switch to trailing return types (#599)
  build: Bump Boost required by CMake from 1.72 to 1.80
  build: Update CMAKE_CXX_STANDARD from 11 to 14
@mloskot mloskot mentioned this pull request Jul 5, 2022
Closed
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mloskot mloskot mloskot approved these changes

Assignees
No one assigned
Labels
cat/annoyance Not a bug, not a feature, but something that should be improved
Projects
None yet
Milestone
Boost 1.80
Development

Successfully merging this pull request may close these issues.

Add support for pmr::polymorphic_allocator
2 participants
@sdebionne @mloskot