tcbrindle / flux

A C++20 library for sequence-orientated programming
https://tristanbrindle.com/flux/
Boost Software License 1.0
472 stars 29 forks source link

Suggestion: add a header containing preprocessor macros. #140

Closed marvin-littlewood closed 8 months ago

marvin-littlewood commented 8 months ago

Hi Tristan,

Thank you for developing flux... I think functional style C++ is the way to go in a multi-core world and IMHO, the technique has the potential to increase the chances of correct programs.

I was able to build module_test.cpp successfully per your post.

Encouraged, I took the word-count.cpp example and replaced #include <flux> with import flux; and discovered that the build failed due to identifier FLUX_FWD not being defined.

The solution was to copy the macro into word-count.cpp. According to this post, preprocessor macros don't play well with modules so one solution seems to be to create a macro file to be included as well as the import.

FWIW, to my surprise, I found this worked using clang++-17:

namespace flux {
    inline auto forward(auto x) { return static_cast<decltype(x)&&>(x); }
} // flux

I can't tell whether it meets all your requirements, but it would be nice to get rid of as many preprocessor macros as possible!

Marv

tcbrindle commented 8 months ago

Hi Marvin,

Thanks for your interest in Flux!

You're right that macros don't work with modules, so we can't use the FLUX_FWD macro with just import flux;. It should possible however to just #include the file include/flux/core/macros.hpp to get FLUX_FWD back.

(Although having quickly looked at it, I think the usage in word-count.cpp is actually unnecessary, as it's perfectly fine to pass an lvalue sequence to for_each.)

FWIW, to my surprise, I found this worked using clang++-17:

namespace flux {
    inline auto forward(auto x) { return static_cast<decltype(x)&&>(x); }
} // flux

Unfortunately this is going to result in an attempted copy whenever it's passed an lvalue. The "proper" macro-less way to do forwarding is to say

std::forward<decltype(seq)>(seq)

but this is very longwinded, hence the macro! (And unless you're using a very recent compiler, heavy use of it can be bad for compile times too.)

I think for the foreseeable future we'll continue using FLUX_FWD internally within the library itself, but recommend that external users prefer std::forward for module compatibility. One day I'd like to convert the examples to using modules, so I should probably go through them and change/remove uses of the FWD macro.

tcbrindle commented 8 months ago

@marvin-littlewood I'm going to close this issue as we do already do what's suggested in the title -- the include/core/macros.hpp header. If you have any more questions or suggestions then please let me know.