Add `ArrayVecCopy`

[tbu-]

Also add infrastructure to generate src/arrayvec_copy.rs. It works by replacing a couple of strings in src/arrayvec.rs.

Check that the transformation is up-to-date in CI.

Fixes #32. Closes #261.

[bluss]

hi, intriguing. I'm skeptical of the maintainability, but it could work, it's a nice accomplishment and not a bad flow you have set up. The more sed patterns needed the worse of a solution this is, I guess? In particular it's interesting how hard it will be to add something to ArrayVec after this change.

[tbu-]

The more sed patterns needed the worse of a solution this is, I guess?

Yeah. :( I wanted to document the sed patterns but couldn't find a way to do that easily.

They are, in that order: 1: rename ArrayVec → ArrayVecCopy 2: rename ArrayVecVisitor → ArrayVecCopyVisitor 3, 4: transform impl<T> to impl<T: Copy> 5, 6: transform struct Name<T> to struct Name<T: Copy> 7, 8: transform fn name<T> to fn name<T: Copy> 9: change all const fn to normal fn (see last commit).

[tbu-]

In particular it's interesting how hard it will be to add something to ArrayVec after this change.

Theoretically, it should be as simple as making the change, then deleting src/arrayvec_copy.rs and running ./generate_arrayvec_copy. If you don't do that, you'll get an error in the check-generated CI check, but everything else will continue to work.

[bluss]

It's promising.

I'm suggesting this creative way of not having to have a patch at all. Instead insert our own directives into the code using comments, and use cfg(never_true) to remove some items from ArrayVecCopy.

https://github.com/tbu-/arrayvec/pull/2

[bluss]

It all came together very nicely. How to add Copy-specific features is going to be figured out when that bridge needs to be crossed.

I think the PR is really good now except that ArrayVecCopy is not actually Copy :laughing: which I hope we take with some humor. How do I know, well I'm just adding a basic test for that before merging..

I'm pushing a fix for that and a basic test now.

[bluss]

Have to think more about the module structure. Maybe ArrayVecCopy should just live inside arrayvec::copy. It would be somewhat consistent when the iterator structs need to have that namespace anyway.

[tbu-]

I think the PR is really good now except that ArrayVecCopy is not actually Copy 😆 which I hope we take with some humor.

Oh my god…

How do I know, well I'm just adding a basic test for that before merging..

Thanks for remembering to add tests, which I conveniently forgot…

Maybe ArrayVecCopy should just live inside arrayvec::copy.

It'd make it more inconvenient to import ArrayVecCopy. The standard library also exports HashMap inside of std::collections (but also in std::collections::hash). I skipped exporting ArrayVecCopy in arrayvec::copy so that we have canonical import paths.

[bluss]

Maybe the editing here is missing "the outside perspective from cargo doc" part of the review. Generate doc, look at it, we see stuff we need to add -> module doc for copy and main module doc needs to mention ArrayVecCopy because it's strange if it doesn't.

I'm still leaning towards that ArrayVecCopy should be down in its copy module, it's more consistent that way, and we have the chance to do it right now. copy is such a short name it feels easy to do.

[tbu-]

I'm still leaning towards that ArrayVecCopy should be down in its copy module, it's more consistent that way, and we have the chance to do it right now. copy is such a short name it feels easy to do.

Can you explain what your proposed module structure is?

1. Everything related to ArrayVecCopy in the copy module and nothing outside of it.
2. Everything related to ArrayVecCopy in the copy module but also ArrayVecCopy in the root.
3. Everything related to ArrayVecCopy in the copy module except ArrayVecCopy, which is solely in the root.

If I understand you correctly, you propose 1. I implemented it as 3.

I think the choice of 1 would lead to more (duplicate!) typing because the name specifies "copy" twice: arrayvec::copy::ArrayVecCopy. Perhaps we should instead call it arrayvec::copy::ArrayVec in this case?

I'd prefer 2 or 3 over 1, I think the standard library does 2, and I'd do 3 so that we only have one possible import path for ArrayVecCopy.

[bluss]

I would prefer (1) or (2). I think (1) is on balance the better choice? I don't mind the duplicate Copy at all, I think it should be there.

[tbu-]

Implemented (1).

[tbu-]

This is asymmetrical towards ArrayString, that's also not in a submodule.

[kristoff3r]

Hi, are there any plans for merging this? I've used this branch for a bit and it's very useful.

[afgTheCat]

Hi, just chiming in. This feature would be helpful for me as well.