Skip to content

Conversation

@nigels-com
Copy link
Contributor

Quite interested in using block_indirect_sort for a current project that concerns rather large arrays.

However our organisation has some conventions (policies?) about compiler flags which includes gcc --pedantic.
And since boost::sort is templated we'd need to touch various modules to relax the compiler flags in various ways.

$ g++ --version
g++ (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0
Copyright (C) 2021 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
$ g++ test_block_indirect_sort.cpp -I $(pwd)/../include --pedantic
In file included from /home/nigels/dev/sort/test/../include/boost/sort/insert_sort/insert_sort.hpp:21,
                 from /home/nigels/dev/sort/test/../include/boost/sort/spinsort/spinsort.hpp:25,
                 from /home/nigels/dev/sort/test/../include/boost/sort/sort.hpp:17,
                 from test_block_indirect_sort.cpp:22:
/home/nigels/dev/sort/test/../include/boost/sort/common/util/traits.hpp:115:6: warning: extra ‘;’ [-Wpedantic]
  115 |     };
      |      ^
      |      -
In file included from /home/nigels/dev/sort/test/../include/boost/sort/block_indirect_sort/blk_detail/backbone.hpp:25,
                 from /home/nigels/dev/sort/test/../include/boost/sort/block_indirect_sort/blk_detail/merge_blocks.hpp:22,
                 from /home/nigels/dev/sort/test/../include/boost/sort/block_indirect_sort/block_indirect_sort.hpp:22,
                 from /home/nigels/dev/sort/test/../include/boost/sort/sort.hpp:20,
                 from test_block_indirect_sort.cpp:22:
/home/nigels/dev/sort/test/../include/boost/sort/common/stack_cnc.hpp:102:6: warning: extra ‘;’ [-Wpedantic]
  102 |     };
      |      ^
      |      -
/home/nigels/dev/sort/test/../include/boost/sort/common/stack_cnc.hpp:135:6: warning: extra ‘;’ [-Wpedantic]
  135 |     };
      |      ^
      |      -

So I took a bit of a quick look at the block_indirect_sort and what it might require to make gcc --pedantic happy.
In my understanding semicolons are needed after a lambda function, struct or class, but not for if/while/for blocks, functions or namespaces.

It does seem that other code linters may well also fuss over this and while the extra semicolons seem harmless to me, they're not what I'm used to looking at, to be honest.

So as a conversation starter with boost::sort maintainers, is this sort of change something likely to be accepted as a contribution? I'm open to covering all the headers, but this initial effort seemed like enough to reduce some tooling friction for block_indirect_sort.

@nigels-com
Copy link
Contributor Author

Just to mention, the other two more obvious issues are -Wshadow and -fno-operator-names which raise a similar question. Is there an inclination to appease these compiler flags?

@fjtapia
Copy link
Collaborator

fjtapia commented Sep 4, 2023 via email

@nigels-com
Copy link
Contributor Author

Hello @fjtapia. Thanks for block_indirect_sort, it's doing a great job for what we need.

I'm not familiar with the github CI, but it looks like it's Mac OS X that is failing to build, not Ubuntu.
My guess is that the CI needs to be migrated to a newer version of Mac OS X, one that is supported by github.
I do happen to have an Intel Mac I can try for this, if that's helpful.

Screenshot_20230905_064502

@nigels-com
Copy link
Contributor Author

And to note that macOS Catalina (version 10.15) went out of support in November 2022.
https://en.wikipedia.org/wiki/MacOS_Catalina

@fjtapia
Copy link
Collaborator

fjtapia commented Sep 4, 2023 via email

@nigels-com
Copy link
Contributor Author

Francisco,

I have some familiarity with orchestrating larger scale compute using AWS EC2 (elastic Linux instances) and S3 (storage).
So the good news is that you wouldn't need to build your own cluster, and there is a fair variety of instances available such as tiny ARM and multi-core with plentiful RAM. Usage charges can add up, however.

Our situation is somewhat memory constrained, the block-based aspect of block_indirect_sort makes it a winner for our application, but if we knew we had the headroom, would be happy to go faster still.

-- Nigel

@fjtapia
Copy link
Collaborator

fjtapia commented Sep 6, 2023 via email

@nigels-com
Copy link
Contributor Author

Francisco,

I reviewed all these trivial changes again just now.
Is there anything further needed to have these merged in?

-- Nigel Stewart

@nigels-com
Copy link
Contributor Author

Re-based again just now.

@fjtapia fjtapia merged commit 213efe6 into boostorg:develop Aug 31, 2024
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.

2 participants