Please add support for built-in arrays to boost::swap

[niels_dekker]

Please add support for built-in array types to the boost::swap utility, currently located in the ​sandbox. For example:

  T arr1[N];
  T arr2[N];
  // Should do an element-wise swap of arr1 and arr2:
  boost::swap(arr1, arr2);

When boost::swap would support arrays, swap functions for other Boost libraries could be implemented more easily, and more generically. For example, array::swap could call boost::swap, instead of std::swap_ranges, to ensure that a custom swap of array::value_type would be called, whenever appropriate. It would also allow adding a swap function to value_initialized<T>, calling boost::swap<T>, without breaking code that had value_initialized wrapping an array.

A similar request for std::swap has been well received by the Library Working Group of the C++ Standards Committee: ​LWG issue 809, std::swap should be overloaded for array types But of course, it will still take a while before STL implementations will actually have such an std::swap overload. In the meantime, it would be helpful if Boost could already start supporting swapping arrays.

This issue was discussed by David Abrahams, Howard Hinnant, Matt Calabrese, Frank Mori Hess, and me at the Boost Developers mailing list, ​(utility/swap) Okay to add array support to Boost.Swap (sandbox/swap)?

[niels_dekker]

swap_arrays.cpp test file, to be added to libs/utility/swap/test

[Dave Abrahams]

I tried to move the sandbox and apply these patches, but the sandbox is at least missing something to link the documentation into the rest of the Boost docs, a Jamfile for testing, and an editorial pass over the docs (at least one glaringly incomplete sentence). I really want to get this into Boost but don't have the time to fix these minor problems up.

Also, it would help if you would supply a *single* unified patch to the trunk that adds this stuff. Running patch multiple times and locating the files to be patched is pretty tedious :-)

Thanks

[niels_dekker]

Documentation update regarding array support, to be applied to libs/utility/swap.html Version 2, including a note by Joseph Gauterin

[Dave Abrahams]

Again, please submit a single patch that covers all the files added or modified.

I see that Jamfile.v2 is there, but like the docs need to be linked into the rest of the Boost documentation it needs to be linked into the top level testing Jamfile.

[niels_dekker]

Single patch that covers all the files added or modified, including Jamfile.v2

[niels_dekker]

Hereby submitted a single patch that covers all array-swap related changes to ​https://svn.boost.org/svn/boost/sandbox/swap

I wouldn't mind doing the commit to the sandbox myself, now that Joseph has also taken a look. (I do have SVN write access.)

Also I wouldn't mind adding a link, href="swap.html", to /utility/index.html once you've moved boost::swap the trunk. (But I guess that's beyond the scope of this ticket.)

I guess the contents of sandbox/swap/libs/utility/swap/test/Jamfile.v2 need to be copied into trunk/libs/utility/test/Jamfile.v2 right? Or would you prefer to have "swap" as a separate test suit?

[Dave Abrahams]

Replying to niels_dekker:

Hereby submitted a single patch that covers all array-swap related changes to ​https://svn.boost.org/svn/boost/sandbox/swap

I wouldn't mind doing the commit to the sandbox myself, now that Joseph has also taken a look. (I do have SVN write access.)

Also I wouldn't mind adding a link, href="swap.html", to /utility/index.html once you've moved boost::swap the trunk. (But I guess that's beyond the scope of this ticket.)

No, that's exactly what I meant by "something to link the documentation into the rest of the Boost docs"

I guess the contents of sandbox/swap/libs/utility/swap/test/Jamfile.v2 need to be copied into trunk/libs/utility/test/Jamfile.v2 right? Or would you prefer to have "swap" as a separate test suit?

Either way is OK. If you can anticipate "swap" needing a few more tests, I'd vote for the separate suite, otherwise, just integrate it into the existing Jamfile. But what I'm asking for is a patch against the trunk, so your patch would include the modifications to that Jamfile.

If you have commit access on the trunk, well then, please go ahead and commit it yourself.

[niels_dekker]

Replying to dave:

But what I'm asking for is a patch against the trunk, so your patch would include the modifications to that Jamfile.

Sorry, I still don't get it. How can I create an array-swap patch for the trunk if "swap" itself isn't yet in the trunk?

Isn't it easier to just apply the patch to the sandbox, beforehand? So shall I apply the patch to the sandbox?

If you can anticipate "swap" needing a few more tests, I'd vote for the separate suite,

Okay, let's have it as separate suite! Especially because I would like to have new boost::swap_ranges and boost::iter_swap functions added to a future version of boost/swap, which would need extra tests, of course...

[niels_dekker]

So I just applied the array-swap patch to the sandbox (changeset [47003]), and fixed some small typo's ([47004]). Please double check.

Is there anything more to be done, before moving sandbox/swap to the trunk? An extra editorial pass over the docs?

FYI, sandbox/swap consists of:

  boost/swap.hpp
  boost/utility/swap.hpp
  libs/utility/swap.html
  libs/utility/swap/test/Jamfile.v2
  libs/utility/swap/test/*.cpp (13 files)

[anonymous]

Replying to niels_dekker:

Sorry, I still don't get it. How can I create an array-swap patch for the trunk if "swap" itself isn't yet in the trunk?

Simple:

1. Check out a clean copy of the trunk.
2. Integrate the stuff you put in the sandbox into your local copy of the trunk.
3. cd $BOOST_ROOT in your trunk copy
4. svn diff

The result is your patch

[Dave Abrahams]

Replying to niels_dekker:

Is there anything more to be done, before moving sandbox/swap to the trunk?

Please prepare the patch against trunk as described above. That includes making modifications to status/Jamfile.v2 (or whatever) that integrates the suite, and modifications to the existing webpages that link to the swap docs.

An extra editorial pass over the docs?

Well, it still says "is available via argument dependent" with no "lookup," so it probably wouldn't hurt :-)

[niels_dekker]

Patch, adding "swap" to status/Jamfile.v2 and libs/utility/index.html

[niels_dekker]

Replying to dave:

Please prepare the patch against trunk as described above. That includes making modifications to status/Jamfile.v2 (or whatever) that integrates the suite, and modifications to the existing webpages that link to the swap docs.

I got the impression that a SVN merge cannot be part of a patch. (But I could be wrong. Am I?) Anyway, the modifications to status/Jamfile.v2 and libs/utility/index.html are included with the attached patch, status_and_utility_index.patch Please double check.

Well, it still says "is available via argument dependent" with no "lookup," so it probably wouldn't hurt :-)

Fixed: [47032]

Note: Joseph Gauterin just mailed me that he would like to do the move to the trunk. Which is perfectly fine by me, because it's his boost::swap in the first place.

[joseph.gauterin]

I've attached a patch to be applied to the trunk.

As well as the moving the contents of sandbox/swap to the correct places on the trunk, it alters the follwoing files: /libs/libraries - adds sqap to the list of libraries /libs/maintainers - added my details as maintainer of boost::swap /libs/utility/index.html - added link to swap documentation /status/Jamfile.v2 - integrated utility/swap test suite

Dave - I don't have SVN write access to the trunk, so will you please apply the patch.

Thanks, Joe.

[Dave Abrahams]

I'm sorry, but the patch doesn't apply cleanly. It's partly due to [43633#file2], which appears to be an at-least-partly-bogus change (John?) and partly because it chokes on the changes to status/Jamfile.v2 for reasons I don't understand:

patching file Jamfile.v2
Hunk #1 FAILED at 112.
Hunk #2 FAILED at 126.
2 out of 2 hunks FAILED -- saving rejects to file Jamfile.v2.rej

[Dave Abrahams]

Hmm, maybe this isn't John's problem. I don't know why iterator adaptors are referenced by the utility/ docs. However, I also don't understand why he added generator_iterator in this change

[Dave Abrahams]

OK, sorry, my mistake. I needed -p0 in my patch command, and I was able to apply it just fine. However, the svn pre-commit hook failed. Since Joseph is going to be the maintainer of this stuff (very nice, thorough tests, Joseph!) I just gave him commit privileges on trunk. Joseph, the ball is in your court.

[John Maddock]

Replying to dave:

Hmm, maybe this isn't John's problem. I don't know why iterator adaptors are referenced by the utility/ docs. However, I also don't understand why he added generator_iterator in this change

I believe the generator_iterator.htm was an unreferenced file showing up in the inspection report - I added a link to it that's all.

John.

[joseph.gauterin]

I've committed the change now.

Joe.

[anonymous]

And why this library was moved to the trunk without any formal review?

[Dave Abrahams]

It is going to be a part of the existing "utility" library.