hpx icon indicating copy to clipboard operation
hpx copied to clipboard

Start using C++17 features unconditionally

Open msimberg opened this issue 4 years ago • 3 comments

Now that we require C++17 we can start using C++17 features more liberally without having to worry about fallbacks for older standards. Below is a grab bag of things we could start changing (please feel free to add to the list, or even better simply make the change if there's something missing). Keep in mind that we don't need to change every instance possible in one go, so don't be afraid to just make a local change while you're working on some piece of code (as we do right now with e.g. typedef to using). Small steps are better than no steps.

  • [x] Replace the HPX_INLINE_CONSTEXPR_VARIABLE macro with inline constexpr
  • [x] Use if constexpr for dispatching where appropriate (e.g. std::true/false_type overloads: https://github.com/STEllAR-GROUP/hpx/blob/03309bea58bcc18aff2864900c77e31116158b1b/libs/parallelism/futures/include/hpx/futures/future.hpp#L51-L84) (#6083)
  • [x] Fold expressions (#6091, #6101)
  • [x] Replace various type traits with std counterparts (e.g. always_void -> void_t) (#6082)
  • [ ] Replace various utilities with std counterparts (e.g. hpx::invoke -> std::invoke? @K-ballo I suspect you know best which ones are actually reasonable to change and for which ones we actually would want to keep our own implementation)
    • [x] Deprecate hpx::apply (#6084)
    • [ ] Rename hpx::invoke_fused to hpx::apply
  • [x] Replace various attributes hidden behind macros directly with the attributes themselves (e.g. [[nodiscard]]) (#5818)
  • [x] Remove empty messages from static_asserts that don't have a meaningful message (#6092)
  • [ ] Use _t and _v variants of various type traits where appropriate, nested namespace definitions (e.g. namespace hpx::execution::experimental { })
    • [x] #6106
    • [x] #6107
    • [x] #6109
    • [x] #6111
    • [x] #6112
    • [x] #6113
    • [x] #6115
    • [x] #6118
    • [x] #6121
    • [x] #6129
    • [x] #6131
    • [x] #6336
  • [x] Make use of CTAD where appropriate (both variable definitions and for our own classes)
  • [x] Use std::string_view instead of boost::string_ref (#6093)

msimberg avatar Aug 05 '21 09:08 msimberg

What about the hpx::apply <--> std::apply semantic mismatch? Should we start deprecating hpx::apply for some other name (hpx::fire_n_forget/hpx::post?) so we can substitute our hpx::util::invoke_fused with std::apply without causing undue confusion?

hkaiser avatar Aug 05 '21 12:08 hkaiser

  • Replace various utilities with std counterparts (e.g. hpx::invoke -> std::invoke)

std::invoke implementations are considerably heavier at build time. It makes sense to replace hpx::invoke in examples and other user facing bits. In implementation code HPX_INVOKE should be used instead, to additionally reduce the stack noise in debug builds.

K-ballo avatar Aug 05 '21 12:08 K-ballo

What about the hpx::apply <--> std::apply semantic mismatch? Should we start deprecating hpx::apply for some other name (hpx::fire_n_forget/hpx::post?) so we can substitute our hpx::util::invoke_fused with std::apply without causing undue confusion?

Yes, I think we'll have to start deprecating it. I'm not sure what would be the best name for it. Another alternative would be to redirect users to hpx::execution::experimental::execute as an alternative, but it may be too early. That would require a customization for executors (just forward to post) and execute would have to become variadic.

std::invoke implementations are considerably heavier at build time. It makes sense to replace hpx::invoke in examples and other user facing bits. In implementation code HPX_INVOKE should be used instead, to additionally reduce the stack noise in debug builds.

Thanks, that makes sense. I suppose if that's the case we could at least putting invoke and friends in detail.

msimberg avatar Aug 06 '21 07:08 msimberg