refactor: memcpy -> std::ranges::copy

[maflcko]

Some follow-ups to #305 (review)

[DrahtBot]

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
ACK	ViniciusCestarii

If your review is incorrectly listed, please copy-paste <!--meta-tag:bot-skip--> into the comment that the bot should ignore.

[ViniciusCestarii]

ACK 8226c05 just commented a nit

[ViniciusCestarii]

In "refactor: memcpy -> std::ranges::copy" 8226c05

nit: I believe this #include <ranges> is not necessary here

[maflcko]

Isn't the iwyu CI supposed to catch this? It passes with either version ...

Maybe it could make sense to fix the CI instead, so that all places are fixed and not only the ones caught in review?

[ViniciusCestarii]

The CI runs IWYU and it only audits "the input .cc file and its associated .h files" (e.g. foo.cpp also checks foo.h). type-string.h has no matching .cpp so it isn't audited.

-  set(CMAKE_CXX_INCLUDE_WHAT_YOU_USE "${IWYU_EXECUTABLE};-Xiwyu;--error")
+  set(CMAKE_CXX_INCLUDE_WHAT_YOU_USE "${IWYU_EXECUTABLE};-Xiwyu;--error;-Xiwyu;--check_also=${PROJECT_SOURCE_DIR}/include/mp/*.h")

With the above diff it checks the headers too and can check for type-string.h. (Bitcoin core also uses --check_also for primitive headers):

/home/vinicius/Code/my/libmultiprocess/include/mp/type-string.h should add these lines:
#include <string>     // for string
namespace mp { struct InvokeContext; }

/home/vinicius/Code/my/libmultiprocess/include/mp/type-string.h should remove these lines:
- #include <ranges>  // lines 11-11

It also flag a lot of what there is currently (10+ header files) and can also make false positives: note the #include <variant> // for tuple which doens't make sense for proxy-io.h:

/home/vinicius/Code/my/libmultiprocess/include/mp/proxy-io.h should add these lines:
#include <capnp/capability.h>          // for Capability, CallContext, Capab...
#include <capnp/common.h>              // for Void, word
#include <capnp/message.h>             // for MallocMessageBuilder, ReaderOp...
#include <capnp/rpc-twoparty.capnp.h>  // for VatId, Side, Side_9fd69ebc87b9...
#include <capnp/rpc.h>                 // for RpcSystem, makeRpcClient, make...
#include <kj/async-io.h>               // for LowLevelAsyncIoProvider, Async...
#include <kj/async-prelude.h>          // for ReadyNow
#include <kj/async.h>                  // for TaskSet, Promise, READY_NOW
#include <kj/common.h>                 // for mv, ArrayPtr, KJ_IF_MAYBE, Maybe
#include <kj/exception.h>              // for runCatchingExceptions, Exception
#include <kj/memory.h>                 // for Own, heap
#include <kj/string.h>                 // for KJ_STRINGIFY, StringPtr
#include <list>                        // for _List_iterator, list, _List_co...
#include <tuple>                       // for tuple
#include <utility>                     // for forward, move
#include <variant>                     // for tuple
#include <vector>                      // for vector
namespace mp { class Connection; }
namespace mp { class EventLoop; }
namespace mp { template <typename Interface> struct ProxyClient; }
namespace mp { template <typename Interface> struct ProxyServer; }

So I believe this isn't a one line fix, it would require a one-time PR to manually audit and fix and map the false positives in a mapping file and then enabling --check_also and --mapping_file with the mapping file to handle false positives on the CI. Maybe this is not worth effort.

This is just what I found poking around, if there's a better way id like to hear it.