Skip to content

[BUG] forward on non-deducible parameter is move #572

Open
Open
[BUG] forward on non-deducible parameter is move#572
Labels
bugSomething isn't working

Description

@JohelEGP
JohelEGP
opened on Aug 5, 2023

Title: forward on non-deducible parameter is move.

Description:

I inadvertently wrote the function from: <Number> (forward q: quantity<Number>),
thinking that would make q a forwarding parameter for any quantity.
Cppfront recognizes q's type as non-deducible,
and doesn't make q a forwarding parameter.
The end result is that the generated code is the same as using move instead of forward.

Minimal reproducer (https://cpp2.godbolt.org/z/qTnYan8f4):

quantity: @struct <Number: type> type = {
  number: Number;
}
from: <Number> (forward q: quantity<Number>) = _ = q;
main: () = {
  q: quantity<i32> = ();
  q.from();
  _ = q;
}
Commands: 
cppfront main.cpp2
clang++17 -std=c++23 -stdlib=libc++ -lc++abi -pedantic-errors -Wall -Wextra -Wconversion -fsanitize=undefined -Werror=unused-result -I . main.cpp

Expected result: A diagnostic indicating that q can't be made a forwarding parameter.

Actual result and error:

Cpp2 lowered to Cpp1: 
//=== Cpp2 type declarations ====================================================


#include "cpp2util.h"

template<typename Number> class quantity;
  

//=== Cpp2 type definitions and function declarations ===========================

template<typename Number> class quantity {
  public: Number number; 
};
template<typename Number> auto from(quantity<Number>&& q) -> void;
auto main() -> int;
  

//=== Cpp2 function definitions =================================================


template<typename Number> auto from(quantity<Number>&& q) -> void { (void) CPP2_FORWARD(q);  }
auto main() -> int{
  quantity<cpp2::i32> q {}; 
  CPP2_UFCS_0(from, q);
  (void) std::move(q);
}
Output: 
main.cpp2:7:15: error: no matching function for call to 'from'
    7 |   CPP2_UFCS_0(from, q);
      |               ^~~~
raw.githubusercontent.com/hsutter/cppfront/main/include/cpp2util.h:727:16: note: expanded from macro 'CPP2_UFCS_0'
  727 |         return FUNCNAME(CPP2_FORWARD(obj)); \
      |                ^~~~~~~~
main.cpp2:7:3: note: in instantiation of function template specialization 'main()::(anonymous class)::operator()<quantity<int> &>' requested here
    7 |   CPP2_UFCS_0(from, q);
      |   ^
raw.githubusercontent.com/hsutter/cppfront/main/include/cpp2util.h:729:2: note: expanded from macro 'CPP2_UFCS_0'
  729 | }(PARAM1)
      |  ^
main.cpp2:4:32: note: candidate function [with Number = int] not viable: expects an rvalue for 1st argument
    4 | template<typename Number> auto from(quantity<Number>&& q) -> void { (void) CPP2_FORWARD(q);  }
      |                                ^    ~~~~~~~~~~~~~~~~~~~~

See also:

Activity

  1. JohelEGP
    added
    bugSomething isn't working
    on Aug 5, 2023

  2. JohelEGP commented on Aug 5, 2023

    @JohelEGP
    JohelEGP
    on Aug 5, 2023
    ContributorAuthor

    In my current use case, I don't care about Number.
    So what I really want to say is:

    from: (forward q: quantity) = _ = q;

    But that treats quantity as a concrete type, and lowers to:

    requires (std::is_same_v<CPP2_TYPEOF(q), quantity>)

    That ends up with this error:

    main.cpp2:5:50: error: type/value mismatch at argument 2 in template parameter list for 'template<class _Tp, class _Up> constexpr const bool std::is_same_v<_Tp, _Up>'
main.cpp2:5:50: note:   expected a type, got 'quantity'

    What I want is this behavior from P2392's §2.1:
    1691262036

    So given appropriate cpp::is overloads for the sublist of:
    1691262555

    I want

    from: (forward q: quantity) = _ = q;
from: (forward q: quantity<float>) = _ = q;

    to respectively translate to

    from: (forward q) requires cpp2::is<quantity, CPP2_TYPEOF(q)>() = _ = q;
from: (forward q) requires cpp2::is<quantity<float>, CPP2_TYPEOF(q)>() = _ = q;

    With the following two overloads,
    a forward parameter can now work if its parameter type names a template (https://cpp2.godbolt.org/z/8zavMvfE8):

    template <template <typename...> class C, typename T>
consteval auto is() -> bool {
    if constexpr (requires(T t) { []<typename... Ts>(C<Ts...> const&) { }(t); }) {
        return true;
    }
    return false;
}
template <typename T, typename U>
consteval auto is() -> bool {
    return std::is_same_v<T, U>;
}

  3. JohelEGP commented on Aug 5, 2023

    @JohelEGP
    JohelEGP
    on Aug 5, 2023
    ContributorAuthor

    Expected result: A diagnostic indicating that q can't be made a forwarding parameter.

    An alternative expected result of
    from: <Number> (forward q: quantity<Number>)
    is for q to be a forwarding parameter
    and have it deduce Number.

    This formulation does that (https://compiler-explorer.com/z/hq1W6PTMP):

    template<typename q_t, typename Number = typename decltype([]<typename Number>(const quantity<Number>&) { return std::type_identity<Number>{}; }(std::declval<q_t>()))::type> auto from(q_t&& q) -> void;
template<typename q_t, typename Number> auto from(q_t&& q) -> void { (void) (CPP2_FORWARD(q).number = Number());  }

    The error messages do leave a lot to be desired (uncomment the call with std::variant).

  4. JohelEGP commented on Aug 5, 2023

    @JohelEGP
    JohelEGP
    on Aug 5, 2023
    ContributorAuthor

    In my current use case, I don't care about Number.

    Actually, I do have an use case:

        export wide: <U, N> (forward qty: quantity<length, U, N>) -> _ = :quantity = (:width = (), qty.number, :U = ());
    export tall: <U, N> (forward qty: quantity<length, U, N>) -> _ = :quantity = (:height = (), qty.number, :U = ());

    I call those on prvalues, so I didn't notice the parameters were rvalues and not forwarding.

  5. hsutter commented on Aug 11, 2023

    @hsutter
    hsutter
    on Aug 11, 2023
    Owner

    Thanks! I've read through the use cases a few times, and I think the part of this that is a feature extension can be deferred to the future. But I think the original ask was this:

    Expected result: A diagnostic indicating that q can't be made a forwarding parameter.

    If there's a better diagnostic we can emit for erroneous code, could we have a PR for that? Otherwise we can close this I think?

    Thanks!

  6. JohelEGP commented on Aug 11, 2023

    @JohelEGP
    JohelEGP
    on Aug 11, 2023
    ContributorAuthor

    A PR would be trivial after #534 is fixed.
    What main has right now isn't enough to detect a forward parameter with non-deducible type.

  7. JohelEGP commented on Oct 2, 2024

    @JohelEGP
    JohelEGP
    on Oct 2, 2024
    ContributorAuthor

    The comments #572 (comment) and #572 (comment)
    are be pretty much the other part of P2481,
    the paper mentioned in commit 36c5a9e.

    Cppfront implements from P2481

    • the fully generic part with f: (forward x: _) = { ... }, and
    • the type_of constrained part with f: (forward x: of_my_type) = { ... }.

    What's missing in Cppfront from P2481
    is actually making the parameter be of that type_of
    (with its deduced constness and reference).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions