Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add class template argument deduction (CTAD) for template user defined types (UDT) #369

Closed

Conversation

filipsajdak
Copy link
Contributor

Currently, after compiling the following code:

value: <T: type, Y: type, Z: type> type = {
    public data1: T;
    public data2: Y;
    public data3: Z;

    operator=: (out this, t: T, inout y: Y, z: Z) = {
        data1 = t;
        data2 = y;
        data3 = z;
    }
    operator=: (out this, t: * const T, y: *Y, move z: Z) = {
        data1 = t*;
        data2 = y*;
        data3 = z;
    }
}

main: () = {
    s := std::string("lvalue string");
    v: value = (42, s, :std::string = "temporary string");
    std::cout << "(v.data1)$, (v.data2)$, (v.data3)$" << std::endl;

    s = "will be moved";
    w: value = (v.data1&, v.data2&, move s);
    std::cout << "(w.data1)$, (w.data2)$, (w.data3)$" << std::endl;
}

We will get the following error:

../tests/ctad.cpp2... ok (all Cpp2, passes safety checks)

../tests/ctad.cpp2:20:11: error: no viable constructor or deduction guide for deduction of template arguments of 'value'
    value v {42, s, std::string{"temporary string"}};
          ^
../tests/ctad.cpp2:6:13: note: candidate template ignored: couldn't infer template argument 'T'
    public: value(cpp2::in<T> t, Y& y, cpp2::in<Z> z);
            ^
../tests/ctad.cpp2:11:13: note: candidate template ignored: couldn't infer template argument 'T'
    public: value(cpp2::in<T const*> t, cpp2::in<Y*> y, Z&& z);
            ^
../tests/ctad.cpp2:1:52: note: candidate function template not viable: requires 1 argument, but 3 were provided
template<typename T, typename Y, typename Z> class value {
                                                   ^
1 error generated.

After this change cppfront generates Class template argument deduction (CTAD) for template class that has constructors with the same argument type list as template arguments (order matters). cppfront will add the following cpp1 code to Cpp2 type definitions and function declarations section:

template<typename T, typename Y, typename Z> value(T const &, Y&, Z const &) -> value<T, Y, Z>;
template<typename T, typename Y, typename Z> value(T const* const &, Y* const &, Z&&) -> value<T, Y, Z>;

And will make the code compile and run successfuly:

42, lvalue string, temporary string
42, lvalue string, will be moved

All regression tests pass.

Comment on lines 4954 to 5089
case passing_style::out :
case passing_style::inout : arg_type += "&"; break;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See https://godbolt.org/z/1dcWs4za1.

Suggested change
case passing_style::out :
case passing_style::inout : arg_type += "&"; break;
case passing_style::out : arg_type.insert(0, "cpp2::deferred_init<") += ">*"; break;
case passing_style::inout : arg_type += "&"; break;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, thank you. It seems there is only a need to avoid cpp2::in<T> as it is the only not deducible type. So, I guess I can make it simpler to reuse the cpp2 way of passing except for passing in args.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One comment about data1 = (t = 42); in the following code:

 operator=: (out this, out t: T, inout y: Y, z: Z) = {
        data1 = (t = 42);
        data2 = y;
        data3 = z;
    }

I think we shall not support that kind of style of coding - there is very good alternative that works and is more clear about the intentions:

    operator=: (out this, out t: T, inout y: Y, z: Z) = {
        t = 42;
        data1 = t;
        data2 = y;
        data3 = z;
    }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Copy link
Contributor

@JohelEGP JohelEGP May 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did data1 = (t = 42); work before? Now it errors with:

build/_cppfront/main.cpp:30:18: error: cannot initialize a member subobject of type 'int' with an rvalue of type 'void'
        : data1{ t.construct(42) }
                 ^~~~~~~~~~~~~~~

@filipsajdak filipsajdak force-pushed the fsajdak-add-ctad-for-udt branch 5 times, most recently from 95688de to 9721b18 Compare April 19, 2023 16:36
@filipsajdak filipsajdak force-pushed the fsajdak-add-ctad-for-udt branch 3 times, most recently from f186f11 to 41f6749 Compare April 24, 2023 06:41
@hsutter
Copy link
Owner

hsutter commented May 20, 2023

Thanks! As I look at the examples, it strikes me that this should be (but isn't) falling into passing by T const& (not in<T>) when T is a template parameter type, which I did specifically for deduction.

It seems we should bypass in<T> in two more places.

  1. For a constructor parameter when T is a template parameter on the class, pass T const& for CTAD deduction. We do that already for all functions when T is a template parameter on the function, for general deduction.
  2. For a parameter * <anything>, pass <anything> * by value (not in<T>). We know it's a pointer, and should be passed by value.

If we did those two things:

  • would that address these examples, as well as being generally useful?
  • would that replace the need for this PR?

@hsutter hsutter self-assigned this May 20, 2023
@hsutter hsutter added the question - further information requested Further information is requested label May 20, 2023
@JohelEGP
Copy link
Contributor

That's indeed a general improvement.
I tested it by hijacking the cpp2 identifier: https://cpp2.godbolt.org/z/WaM6998T3.

Unfortunately, out parameters also inhibit deduction: https://cpp2.godbolt.org/z/j54j65j9q.

@JohelEGP
Copy link
Contributor

A way to support the deduction of an out parameter
would be by changing the pass-by out syntax out i
from being lowered to an argument intended for a cpp2::out parameter
to being lowered to a value of cpp2::out: https://cpp2.godbolt.org/z/34d5EfazK.

Currently after compiling the following code:
```cpp
value: <T: type, Y: type, Z: type> type = {
    public data1: T;
    public data2: Y;
    public data3: Z;

    operator=: (out this, out t: T, inout y: Y, z: Z) = {
        t = 42;
        data1 = t;
        data2 = y;
        data3 = z;
    }
    operator=: (out this, t: * const T, y: *Y, move z: Z) = {
        data1 = t*;
        data2 = y*;
        data3 = z;
    }
}

main: () = {
    i : int;
    s := std::string("lvalue string");
    v: value = (out i, s, :std::string = "temporary string");
    std::cout << "(v.data1)$, (v.data2)$, (v.data3)$" << std::endl;

    s = "will be moved";
    w: value = (v.data1&, v.data2&, move s);
    std::cout << "(w.data1)$, (w.data2)$, (w.data3)$" << std::endl;
}
```
We will get the following error:
```
../tests/ctad.cpp2... ok (all Cpp2, passes safety checks)

../tests/ctad.cpp2:22:11: error: no viable constructor or deduction guide for deduction of template arguments of 'value'
    value v {&i, s, std::string{"temporary string"}};
          ^
../tests/ctad.cpp2:12:13: note: candidate template ignored: couldn't infer template argument 'T'
    public: value(cpp2::in<T const*> t, cpp2::in<Y*> y, Z&& z);
            ^
../tests/ctad.cpp2:6:13: note: candidate template ignored: could not match 'cpp2::out<T>' against 'cpp2::deferred_init<int> *'
    public: value(cpp2::out<T> t, Y& y, cpp2::in<Z> z);
            ^
../tests/ctad.cpp2:1:52: note: candidate function template not viable: requires 1 argument, but 3 were provided
template<typename T, typename Y, typename Z> class value {
                                                   ^
1 error generated.
```

After this change cppfront generates Class template argument deduction (CTAD)
for template class that has constructors with the same argument type list
as template arguments (order matters). cppfront will add the following
cpp1 code to `Cpp2 type definitions and function declarations` section:
```cpp
template<typename T, typename Y, typename Z> value(cpp2::deferred_init<T>*, Y&, Z const &) -> value<T, Y, Z>;
template<typename T, typename Y, typename Z> value(T const* const &, Y* const &, Z&&) -> value<T, Y, Z>;
```
And will make the code compile and run successfuly:
```
42, lvalue string, temporary string
42, lvalue string, will be moved
```
@filipsajdak filipsajdak force-pushed the fsajdak-add-ctad-for-udt branch from 41f6749 to 955042b Compare May 21, 2023 16:47
hsutter added a commit that referenced this pull request May 21, 2023
@hsutter
Copy link
Owner

hsutter commented May 21, 2023

A way to support the deduction of an out parameter
would be by changing the pass-by out syntax out i
from being lowered to an argument intended for a cpp2::out parameter
to being lowered to a value of cpp2::out: https://cpp2.godbolt.org/z/34d5EfazK.

I'm happy to try that... it creates an extra copy of the out but that's enough for an experiment and can be optimized later.

Pushed: e0125d7

@JohelEGP
Copy link
Contributor

JohelEGP commented May 21, 2023

it creates an extra copy

Please, see #198 (comment).
You can initialize a parameter from a prvalue of a non-copyable non-movable type: https://compiler-explorer.com/z/Y1x5nMT16.
So there should be no extra copy.

hsutter added a commit that referenced this pull request May 21, 2023
…ment thread

1. For a constructor parameter when `T` is a template parameter on the class, pass `T const&` for CTAD deduction. We do that already for all functions when `T` is a template parameter on the function, for general deduction.

2. For a parameter `* <anything>`, pass `<anything> *` by value (not `in<>`). We know it's a pointer, and should be passed by value.
@hsutter
Copy link
Owner

hsutter commented May 21, 2023

OK, done... today's two commits (mentioned above) now apply all three changes discussed in this thread... quoting for convenience:

  1. For a constructor parameter when T is a template parameter on the class, pass T const& for CTAD deduction. We do that already for all functions when T is a template parameter on the function, for general deduction.
  2. For a parameter * <anything>, pass <anything> * by value (not in<T>). We know it's a pointer, and should be passed by value.
  3. changing the pass-by out syntax out i from being lowered to an argument intended for a cpp2::out parameter to being lowered to a value of cpp2::out

With these changes, the motivating example in this PR description now works, and out is more deducible.

Thanks again for the examples and discussion!

Question: Even though the top example now works, I'm leaving this open for @filipsajdak to check and confirm... Filip, do these changes sufficiently address your motivation for this PR, so we should close it? Or are there additional examples that motivate the PR we should consider, and add those examples to this thread and rebase the PR?

@hsutter
Copy link
Owner

hsutter commented May 21, 2023

Please, see #198 (comment).

@JohelEGP Indeed, I've added a comment there asking you to please upgrade it to a whole new issue for argument-side qualifiers, so we don't lose this. I think we're getting to where we understand those well enough to enable more/all of them. Thanks for the reminder!

@filipsajdak
Copy link
Contributor Author

I will check the examples tomorrow and will let you know.

@filipsajdak
Copy link
Contributor Author

I have no other examples... I think we can close it.

zaucy pushed a commit to zaucy/cppfront that referenced this pull request Dec 5, 2023
zaucy pushed a commit to zaucy/cppfront that referenced this pull request Dec 5, 2023
 comment thread

1. For a constructor parameter when `T` is a template parameter on the class, pass `T const&` for CTAD deduction. We do that already for all functions when `T` is a template parameter on the function, for general deduction.

2. For a parameter `* <anything>`, pass `<anything> *` by value (not `in<>`). We know it's a pointer, and should be passed by value.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question - further information requested Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants