codeinred / tuplet Goto Github PK

Hi,

first of all: Great library! I've learned a few C++ tricks here and there while reading the code.

I wanted to try this library as a replacement for std::tuple in my code base to check if it improves (compile) performance. Unfortunately, there are a few minor to major differences to std::tuple that make it difficult to use it as a drop-in replacement.

One case I heavily rely on in my code base is the use of empty tuples.

Example:

See https://godbolt.org/z/ojYM79q3K

tuplet::tuple<> empty_fails;

Motivation

I've written some generic code that stores lambdas in tuples.
And I've got some code that can append other lambdas to this tuple (and a new tuple is returned).
Because it's quite generic, it may happen that one tuple is empty.

How to fix

A quick&dirty fix I've used is to add something like this:

template<>
struct tuple<> {
    constexpr static size_t N = 0;

    template <other_than<tuple> U> 
    constexpr auto& operator=(U&& tup)
    {
        // We can't really assign empty tuples, can we?
        return *this;
    }

    template <typename... U>
    constexpr auto& assign(U&&... )
    {
        // Also not really needed.
        return *this;
    }
};

Accidental (?) reversion of the fix from #21 by the looks of it in 875c5de:

(void(B1::value = static_cast<U&&>(u).B2::value), ...);
was changed back to:
(void(B1::value = static_cast<U&&>(u).identity_t<B2>::value), ...);

Remove identity_t and all is well again, but maybe a better fix is needed if that identity_t is really there for a reason!

Also it appears to manifest only in c++20 mode. At least the tests all compile and work by default, but that seems to be in c++17 mode. Trying to configure cmake with -DCXX_STANDARD=20 caused all sorts of std::format weirdness. I don't really have the time to look in to that; I'm just going to apply the old fix locally as it was working fine but here's a simple test instead:

#include <cassert>
#include <tuplet/tuple.hpp>

int main()
{
	int a = -1;
	tuplet::tie(a) = tuplet::make_tuple(123);
	assert(a == 123);
}

With c++20:

 cl test.cpp /std:c++20 -I./include && ./test.exe
Microsoft (R) C/C++ Optimizing Compiler Version 19.34.31937 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

test.cpp
Microsoft (R) Incremental Linker Version 14.34.31937.0
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:test.exe
test.obj
Assertion failed: a == 123, file test.cpp, line 8

And 17:

 cl test.cpp /std:c++17 -I./include && ./test.exe
Microsoft (R) C/C++ Optimizing Compiler Version 19.34.31937 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

test.cpp
Microsoft (R) Incremental Linker Version 14.34.31937.0
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:test.exe
test.obj

Sadly no movement at all in https://developercommunity.visualstudio.com/t/fold-expressions-unreliable-in-171-with-c20/1676476

Thanks!

It would be great if your library provided some tuplet-config.cmake file so that users only have to call
find_package(tuplet <some version> REQUIRED) in their CMakeLists.txt to be able to find your library, rather that copying and pasting the 'tuplet/tuple.hpp' file.

Anyway, thank you for your work.

Can the helpers std::tuple_size_v and std::tuple_element_t be added?

msvc 19.29.30133

2021-09-29T19:13:28+07:00
Running bench
Run on (4 X 3696 MHz CPU s)
CPU Caches:
L1 Data 32 KiB (x2)
L1 Instruction 32 KiB (x2)
L2 Unified 256 KiB (x2)
L3 Unified 3072 KiB (x1)

gcc 11.2 (wsl)

2021-09-29T19:20:21+07:00
Running build/bench
Run on (4 X 3696 MHz CPU s)
CPU Caches:
L1 Data 32 KiB (x2)
L1 Instruction 32 KiB (x2)
L2 Unified 256 KiB (x2)
L3 Unified 3072 KiB (x1)
Load Average: 1.44, 0.36, 0.12

clang-14 (wsl)
//add -fbracket-depth=10000000

2021-09-29T19:32:17+07:00
Running build/bench
Run on (4 X 3696 MHz CPU s)
CPU Caches:
L1 Data 32 KiB (x2)
L1 Instruction 32 KiB (x2)
L2 Unified 256 KiB (x2)
L3 Unified 3072 KiB (x1)
Load Average: 1.19, 0.56, 0.28

I tried to use tuplet::tuple as a optional drop-in replacement for std::tuple to check if it reduces compilation times for my use case. But compilation failed. I reproduced it on godbolt.

Any ideas? Is it principal limitation of tuplet so, some wrapper function should be called or it is a bug that can be easily fixed?

#include <memory>

#ifdef USE_STD_TUPLE
#include <tuple>

using std::apply;
using std::tie;
using std::tuple;
#else
#include "https://raw.githubusercontent.com/codeinred/tuplet/main/include/tuplet/tuple.hpp"

using tuplet::apply;
using tuplet::tie;
using tuplet::tuple;
#endif

template <typename... DepValues>
class Foo {
   public:
    Foo(const std::shared_ptr<DepValues>&... deps) : m_deps(deps...) {}

   private:
    using dep_holder_t = tuple<std::shared_ptr<DepValues>...>;

    dep_holder_t m_deps;
};

int main() {
    using T = Foo<int, int>;

    T t{std::make_shared<int>(3), std::make_shared<int>(5)};
}

I trying to use tl::expected<tuplet::tuple<...>,...>'s operator= and it failed.

The expected use SFINAE(std::is_assignable<T,U>) in operator= to determine if is unexpected error or expected value. However, the concept for tuplet::tuple is too loose so that it accept every other than type, which cause compile error.

Tightening the concept will fix it.

Hi, thank you for this tuple implementation, I was trying to implement something similar (with little success).

I might have stumbled into a problem with Clang (tried versions 13, 14, and 15).
The issue arises when a tuple is nested into another tuple mixed with other types, like:

tuplet::tuple<int, tuplet::tuple<int>> p;

Here is a godbolt link: https://godbolt.org/z/1e1hsWv84

Since clang crashes there should be a problem on its side (which I have no idea how to solve).
I was wondering if you know any workaround to make them work together.

Thanks!

This is purely an opinion, so take with a grain of salt :)

I find the English-like numeral suffixes to be a little too "cute" and not necessarily helpful to code readability at scale. Especially when considering that, while software is often written in English, is is often written by non-native English speakers. Getting fancy with English numerals with UDL suffixes like _nd vs _rd seems like a recipe for confusion. It probably also increases the risk of unnecessary name clashes with other libraries when a particular function wants to using namespace UDLs from several different places.

I'd recommend dropping all the English-y UDLs and just sticking to _tag.

replace:

template <char... D> constexpr index_t<D...> operator""_tag() { return {}; }
template <char... D> constexpr index_t<D...> operator""_st() { return {}; }
template <char... D> constexpr index_t<D...> operator""_nd() { return {}; }
template <char... D> constexpr index_t<D...> operator""_rd() { return {}; }
template <char... D> constexpr index_t<D...> operator""_th() { return {}; }

with just:

template <char... D> constexpr index_t<D...> operator""_tag() { return {}; }

(Separately, I'm not sure the UDLs are all that helpful anyway; C++ made UDLs painful to use due to the need to always using namespace blah::literals; to access them; I'd generally just fallback to using tag<N> instead of N_tag anyway; it's one character longer, but cuts out an entire using statement.)

Tuplet is available on Conan. Could it also be made available on VCPKG?

tuple_cat can be made far more efficient by decreasing the function instantiations and parameters passed. This has a significant effect for both compile times and for the resultant assembly generated.

A solution I came up with for my own tuplet-like library was to create a tuple of tuples, to build an index LUT for that 'big' tuple, and then to do the entire reconstruction process in a single pass.

A working version of my code is available here: https://godbolt.org/z/5WoWx8Wv6
adapted from my earlier works,
here: https://gist.github.com/mechacrash/b0b941b1bff29beb76811b69186284e6
and here: https://gist.github.com/mechacrash/39c8af5187e5f54ea6747416c1fa95c4

I reimplemented a version that works with tuplet, and supports the earliest versions of the clang, GCC and MSVC compilers that tuplet currently supports (11.X, 10.X, and 19.23 respectfully - this is limited by support for <compare>) and threw together a quick demonstration of the issue,
See a comparison here:

• before, https://godbolt.org/z/dxsjeKxsh
• after, https://godbolt.org/z/5n1MWeo4c

unoptimised
gcc : 123'700 lines of asm -> 7'860 lines of asm
clang : 116'903 lines of asm -> 8'501 lines of asm
msvc : 121'603 lines of asm -> 12'988 lines of asm

optimised (-O3, /O2)
gcc : fails to build (compilation time exceeded) -> 3 lines, fully optimised out
clang : 31'056 lines of asm -> 3 lines, fully optimised out
msvc : 63'944 lines of asm -> 4'158 lines of asm

Apologies for not supplying this as a pull request, just wanted to quickly throw this across and found this was the easiest path. You're more than welcome to use any part of this code however you like though - adapt or discard as you wish!

Seems like there's a few issues with VS 17.1 and fold expressions (well, maybe that's where the issue is - not sure really) and tuplet suffers (assignment tests fail).

Whilst trying to make a minimal standalone example to post to MS, I found it was allowing invalid code: https://developercommunity.visualstudio.com/t/fold-expressions-unreliable-in-171-with-c20/1676476 (as I write, that ticket isn't live yet; stuck in some queue due to all the bugs in the new compiler being reviewed, maybe ... but hopefully will be soon).

I'm guessing once they fix that similar issue, the tuplet assignment will work again. That said, I have a workaround:

https://github.com/lukester1975/tuplet/blob/f18bc731f062524376d993810f3fd673bc02185e/include/tuplet/tuple.hpp#L294-L303

which allows assignment tests to pass again. Branch is here: https://github.com/lukester1975/tuplet/tree/cl17.1-workaround

1. There's a few versions: simplest that I realised after the others was just to remove the identity<B2> and simply use B2. I can't see what it's there for, but might be missing something...
2. If so, the third version void(static_cast<B1&>(*this).value = std::forward<identity_t<B2>>(u).value), ...); keeps it.
3. Note that in this version, only the right-side of the assignment is actually required. I only changed the lhs to a cast to be consistent.
4. There's an additional test for using tie with move as my initial "fix" (none of these versions, though) inadvertently missed a move and this new test caught it.

As an aside, could I ask the reason for the void in (void(B1::value = static_cast<U&&>(u).identity_t<B2>::value), ...); ? I've seen similar in boost. Does it avoid a warning about unused result on some compilers ... or something else?

Happy to make a PR if you want one of the workarounds.

Thanks!