bloomberg / clang-p2996

Experimental clang support for WG21 P2996 (Reflection).
https://github.com/bloomberg/clang-p2996/tree/p2996/P2996.md
51 stars 8 forks source link

Splice operator with fold expression and parameter pack indexing #67

Closed jasiolpn closed 2 months ago

jasiolpn commented 2 months ago

Hello First of all thank you for incredible work you have done. I am very excited about reflection features in C++.

I was playing a little with C++26 features implemented in Clang. One of them is pack indexing. I decided to check how it works with reflection. I wrote simple program (without reflection fatures) which works as expected:

struct A
{
    auto sayHello() const noexcept -> void
    {
        std::cout << "hello from A!\n";
    }
};

struct B
{
    auto sayOtherHello() const noexcept -> void
    {
        std::cout << "hello from B!\n";
    }
};

template <std::size_t ACount, std::size_t BCount, auto... Sayers>
struct Greeter
{
    auto operator()() const noexcept -> void
    {
        [] <auto... Indexes> (std::index_sequence<Indexes...>) {
            ((Sayers...[Indexes].sayHello()), ...);
        }(std::make_index_sequence<ACount>{});

        [] <auto... Indexes> (std::index_sequence<Indexes...>) {
            ((Sayers...[Indexes + ACount].sayOtherHello()), ...);
        }(std::make_index_sequence<BCount>{});
    }
};

int main()
{
    Greeter<2, 4, A{}, A{}, B{}, B{}, B{}, B{}>{}();

    return 0;
}

Output:

hello from A!
hello from A!
hello from B!
hello from B!
hello from B!
hello from B!

But if I change Sayers parameters of struct Greeter to be reflections it fails to compile:

struct A
{
    auto sayHello() const noexcept -> void
    {
        std::cout << "hello from A!\n";
    }
};

struct B
{
    auto sayOtherHello() const noexcept -> void
    {
        std::cout << "hello from B!\n";
    }
};

template <std::size_t ACount, std::size_t BCount, auto... Sayers>
struct Greeter
{
    auto operator()() const noexcept -> void
    {
        [] <auto... Indexes> (std::index_sequence<Indexes...>) {
            (([: Sayers...[Indexes] :].sayHello()), ...);
        }(std::make_index_sequence<ACount>{});

        [] <auto... Indexes> (std::index_sequence<Indexes...>) {
            (([: Sayers...[Indexes + ACount] :].sayOtherHello()), ...);
        }(std::make_index_sequence<BCount>{});
    }
};

int main()
{
    using namespace std::meta;

    Greeter<
        2, 4,
        reflect_value(A{}), reflect_value(A{}),
        reflect_value(B{}), reflect_value(B{}), reflect_value(B{}), reflect_value(B{})
    >{}();

    return 0;
}

Compiler output:

error: reflection operand must be a named entity
.../cpp/pack_indexig_fold_expressions/main.cpp:26:9: note: in instantiation of function template specialization 'Greeter<2, 4, (reflection), (reflection), (reflection), (reflection), (reflection), (reflection)>::operator()()::(anonymous class)::operator()<0UL, 1UL>' requested here
   26 |         [] <auto... Indexes> (std::index_sequence<Indexes...>) {
      |         ^
.../cpp/pack_indexig_fold_expressions/main.cpp:42:8: note: in instantiation of member function 'Greeter<2, 4, (reflection), (reflection), (reflection), (reflection), (reflection), (reflection)>::operator()' requested here
   42 |     >{}();
      |        ^
error: reflection operand must be a named entity
.../cpp/pack_indexig_fold_expressions/main.cpp:30:9: note: in instantiation of function template specialization 'Greeter<2, 4, (reflection), (reflection), (reflection), (reflection), (reflection), (reflection)>::operator()()::(anonymous class)::operator()<0UL, 1UL, 2UL, 3UL>' requested here
   30 |         [] <auto... Indexes> (std::index_sequence<Indexes...>) {
      |         ^
/home/marcin/Development/cpp/pack_indexig_fold_expressions/main.cpp:42:8: note: in instantiation of member function 'Greeter<2, 4, (reflection), (reflection), (reflection), (reflection), (reflection), (reflection)>::operator()' requested here
   42 |     >{}();
      |        ^
2 errors generated.

However code compiles if I will manually expand fold expressions:

struct A
{
    auto sayHello() const noexcept -> void
    {
        std::cout << "hello from A!\n";
    }
};

struct B
{
    auto sayOtherHello() const noexcept -> void
    {
        std::cout << "hello from B!\n";
    }
};

template <std::size_t ACount, std::size_t BCount, auto... Sayers>
struct Greeter
{
    auto operator()() const noexcept -> void
    {
        [: Sayers...[0] :].sayHello();
        [: Sayers...[1] :].sayHello();
        [: Sayers...[2] :].sayOtherHello();
        [: Sayers...[3] :].sayOtherHello();
        [: Sayers...[4] :].sayOtherHello();
        [: Sayers...[5] :].sayOtherHello();
    }
};

int main()
{
    using namespace std::meta;

    Greeter<
        2, 4,
        reflect_value(A{}), reflect_value(A{}),
        reflect_value(B{}), reflect_value(B{}), reflect_value(B{}), reflect_value(B{})
    >{}();

    return 0;
}

Output:

hello from A!
hello from A!
hello from B!
hello from B!
hello from B!
hello from B!

I believe that second code snippet should be valid and there is a bug in a compiler.

Best regards! Marcin Nowak

katzdm commented 2 months ago

Hey Marcin! I believe you ran into a corner case that we learned about this week during Core wording review. There are three contexts in the language, that we know of, when an id-expression can be a prvalue: 1) when making a non-type template parameter of non-class type, 2) when naming an enumerator, and as we found out this week, 3) pack index expressions. I think we need some special handling for the last case.

jasiolpn commented 2 months ago

Thank you for the reply. I developed workaround for my problem which works, but it needs some boilerplate code, is less readable and (in my opinion most important) is not efficient as it should be:

template <std::size_t ACount, std::size_t BCount, auto... Sayers>
struct Greeter
{
    auto operator()() const noexcept -> void
    {
        const auto tuple = std::make_tuple([: Sayers :]...);

        [&tuple] <auto... Indexes> (std::index_sequence<Indexes...>) {
            std::apply(
                [] (auto&& ...sayers) -> void {
                    ((sayers...[Indexes].sayHello()), ...);
                },
                tuple
            );
        }(std::make_index_sequence<ACount>{});

        [&tuple] <auto... Indexes> (std::index_sequence<Indexes...>) {
            std::apply(
                [] (auto&& ...sayers) -> void {
                    ((sayers...[Indexes + ACount].sayOtherHello()), ...);
                },
                tuple
            );
        }(std::make_index_sequence<BCount>{});
    }
};

It requires instantiation of std::tuple so we are losing constexprness of the Sayers arguments.

Anyway I hope problems with wording will be resolved soon!

katzdm commented 2 months ago

~Hey Marcin! I believe you ran into a corner case that we learned about this week during Core wording review. There are three contexts in the language, that we know of, when an id-expression can be a prvalue: 1) when making a non-type template parameter of non-class type, 2) when naming an enumerator, and as we found out this week, 3) pack index expressions. I think we need some special handling for the last case.~

Lol I think the reason is unrelated to this. Hope to have a fix shortly, though.

katzdm commented 2 months ago

Turned out to be a bug in substitution of reflection non-type template arguments - the bug was introduced when I removed support for the ^cast-expression form that was supported up until P2996R3. Thanks for the report! This should work on Godbolt tomorrow.