hsutter / cppfront

A personal experimental C++ Syntax 2 -> Syntax 1 compiler
Other
5.24k stars 224 forks source link

[BUG] Cannot deduce a braced-init-list (perhaps "parens-init-list" in Cpp2-speak) #1020

Closed bluetarpmedia closed 3 months ago

bluetarpmedia commented 3 months ago

Describe the bug Cannot deduce a C++ braced-init-list (written with parens in Cpp2). Instead I have to explicitly write out the type.

To Reproduce I'm trying to translate this C++ code:

#include <initializer_list>
#include <iostream>

int main()
{
    const auto ints = {11, 22, 33, 44, 55};
    for (auto i : ints) {
        std::cout << i << " ";
    }
}

into Cpp2:

main: () -> int = {
    ints: const _ = (11, 22, 33, 44, 55);

    for ints do (i) {
        std::cout << "(i)$ ";
    }
}

But the relevant line lowers to:

auto const ints {11, 22, 33, 44, 55};

And the C++ compiler reports this error:

error: initializer for variable 'ints' with type 'const auto' contains multiple expressions
    2 |     auto const ints {11, 22, 33, 44, 55}; 
      |     ~~~~~~~~~~~~~~~      ^

Repro on Godbolt

Changing the declaration to the following works as desired:

ints: const std::initializer_list<int> = (11, 22, 33, 44, 55);
DyXel commented 3 months ago

I think the general solution would be to emit the = as gcc suggests (for declarations / initializations only):

error: direct-list-initialization of 'auto' requires exactly one element [-fpermissive]
note: for deduction to 'std::initializer_list', use copy-list-initialization (i.e. add '=' before the '{')

so auto const ints {11, 22, 33, 44, 55}; in your example, becomes auto const ints = {11, 22, 33, 44, 55};.

Finally, thanks to CTAD, std::initializer_list const ints = {11, 22, 33, 44, 55}; would work as well (this one would work even without the =).

bluetarpmedia commented 3 months ago

Interestingly, clang rejects the std::initializer_list const ints = {11, 22, 33, 44, 55}; CTAD version.

#include <initializer_list>
#include <vector>
int main()
{
    std::vector const vec = {11, 22, 33, 44, 55};             // OK
    std::initializer_list const ints = {11, 22, 33, 44, 55};  // Error
}

Repro on Godbolt with 3 major compilers.

Some discussion on whether its valid.

hsutter commented 3 months ago

I'm trying to translate this C++ code:

#include <initializer_list>
#include <iostream>

int main()
{
    const auto ints = {11, 22, 33, 44, 55};
    for (auto i : ints) {
        std::cout << i << " ";
    }
}

into Cpp2:

main: () -> int = {
    ints: const _ = (11, 22, 33, 44, 55);

    for ints do (i) {
        std::cout << "(i)$ ";
    }
}

Actually the Cpp2 equivalent is even simpler:

main: () = {
    // this works fine today
    for (11, 22, 33, 44, 55) do (i) {
        std::cout << "(i)$ ";
    }
}

Would that answer your immediate question/need?

Unless you really want ints to be a variable of type initializer_list<int> that you're going to use again in other ways? I had not intended to support deducing initializer_list because I didn't think there were many real use cases for wanting an object of initializer_list type.

Also, deducing initializer_list has been a source of questions in the past, and the Cpp1 status is oddly inconsistent about it... I understand why, but it was an odd history and an odd final state, and I've never been fully convinced it was needed at all.

bluetarpmedia commented 3 months ago

Unless you really want ints to be a variable of type initializer_list that you're going to use again in other ways?

Yeah, that was the intention. I tried to minimise the repro code to describe the problem but should've linked to the full context. I was translating this code example from cppreference:

#include <barrier>
#include <iostream>
#include <string>
#include <syncstream>
#include <thread>
#include <vector>

int main()
{
    const auto workers = {"Anil", "Busara", "Carl"};  // <--- Deduced initializer_list

    auto on_completion = []() noexcept
    {
        // locking not needed here
        static auto phase =
            "... done\n"
            "Cleaning up...\n";
        std::cout << phase;
        phase = "... done\n";
    };

    std::barrier sync_point(std::ssize(workers), on_completion);  // <--- Used here

    auto work = [&](std::string name)
    {
        std::string product = "  " + name + " worked\n";
        std::osyncstream(std::cout) << product;
        sync_point.arrive_and_wait();

        product = "  " + name + " cleaned\n";
        std::osyncstream(std::cout) << product;
        sync_point.arrive_and_wait();
    };

    std::cout << "Starting...\n";
    std::vector<std::jthread> threads;
    threads.reserve(std::size(workers));      // <--- Used here
    for (auto const& worker : workers)        // <--- Used here
        threads.emplace_back(work, worker);
}

This is a work in progress but I'm slowly copying and converting the examples from cppreference (under their permissive license) here: cpp2reference.com

hsutter commented 3 months ago

Yeah, that was the intention.

OK, thanks.

I'm slowly copying and converting the examples from cppreference (under their permissive license) here: cpp2reference.com

🤯 Mind blown 🤯

AbhinavK00 commented 3 months ago

In the given example, why not use std::array? I mean, I know you're converting the examples but cpp2 does not need std::initializer_list, it has uniform initialization which just works. Any use of std::initializer_list as a container/range can be replaced by std::array or std::vector instead and cpp2 does not expose it in initialization.

It'll be better if std::initializer_list remains merely an implementation detail and isn't taught with cpp2.

bluetarpmedia commented 3 months ago

In the given example, why not use std::array?

Good point, I’ll update it to use std::array.

I guess the main motivation for this issue is to find a terse Cpp2 equivalent for:

const auto workers = {"Anil", "Busara", "Carl"};

Because I think someone coming from C++ would try and reach for:

workers:= ("Anil", "Busara", "Carl");

AbhinavK00 commented 3 months ago

I guess the main motivation for this issue is to find a terse Cpp2 equivalent

There have been discussions and suggestions about built-in arrays in cpp2.

Maybe something like:

workers:= ["Anil", "Busara", "Carl"];

This could transpile to something like a cpp2::array and replace the uses cases of both std::array and std::initializer_list, kill two birds with one stone!

hsutter commented 3 months ago

Or, doing some zen-of-cpp2 thinking here, what about using defaults?

If the declaration is of the form

name := (expression, list);

we know there is no explicit type on the right-hand side... I'm pretty sure there's no way to smuggle one in, because Cpp2 deliberately doesn't have the comma operator which would be the main way I would think of to force it in Cpp1.

So, since we know the RHS is a list, we could do the "deduction" ourselves as a language default, and emit std::array and let the type and size be deduced. (Alternatively we could also emit std::initializer_list but I'd rather not unless there's a compelling need.)

But this is a late-night thought and I could be missing something. What do you think?

bluetarpmedia commented 3 months ago

Yes, I like that. So if:

workers:= ("Anil", "Busara", "Carl");

defaults to std::array, could this:

bag:= (11, "cat", 3.14);

default to std::tuple?

hsutter commented 3 months ago

I don't think changing the type based on the list contents is easy, for the compiler or the human. Feels a bit magical. For example, if it deduces to tuple we can't for-each over it.

If you want tuple, just ask? bag: std::tuple = (1, "xyzzy");

But I suppose that works here too: workers: std::array = ("Anil", "Busara", "Carl");

Given that, perhaps the following is fine, which works today? (And has no magic... this is WYSIWYG code.)

main: () = {
    workers: std::array = ("Anil", "Busara", "Carl");
    for workers do (e)
        std::cout << e << "\n";

    bag: std::tuple = (11, "cat", 3.14);
    std::cout << std::get<0>(bag) << "\n";
    std::cout << std::get<1>(bag) << "\n";
    std::cout << std::get<2>(bag) << "\n";
}

And we could still add the default to std::array... I could see that as a default.

AbhinavK00 commented 3 months ago

So, since we know the RHS is a list, we could do the "deduction" ourselves as a language default, and emit std::array and let the type and size be deduced.

That is a nice solution but maybe we we should try for something that solves other issues too like #542 and #568 (athough they both can be solved by #927 ).

DyXel commented 3 months ago

So, since we know the RHS is a list, we could do the "deduction" ourselves as a language default, and emit std::array and let the type and size be deduced.

That sounds like a reasonable assumption to make , I kind of like it, however...

(Alternatively we could also emit std::initializer_list but I'd rather not unless there's a compelling need.)

... I have seen that std::initializer_list is indeed a bit weird, but is ask the opposite, is there a reason it shouldn't be supported in Cpp2? To me {val1, val2, ...} is a std::initializer_list , not an std::array, not a std::vector, and the Cpp2 equivalent is (val1, val2, ...), so to me the expression name := (expression, list); naturally deduces to a std::initializer_list, which can be then used for other purposes where they have the support. Is there a reason why supporting it could cause problems to Cpp2's implementation or harm/confuse users?

I totally agree with @AbhinavK00 , we should try to solve other issues along this one if possible.

gregmarr commented 3 months ago

What is the cost (time and storage) difference between these two?

std::initializer_list il = {"Anil", "Busara", "Carl"};
std::array arr = {"Anil", "Busara", "Carl"};

There is also a difference if you pass them to template functions, as array has the size as a template parameter, so you get a different function for each array size and element type, where for initializer_list, they only vary by element type.

void output(auto &container)
{
  for (auto &str : container)
  {
    std::cout << str << "\n";
  }
}

int main()
{
  std::initializer_list il3 = {"Anil", "Busara", "Carl"};
  std::initializer_list il4 = {"Anil", "Busara", "Carl", "Dan"};
  std::array arr3 = {"Anil", "Busara", "Carl"};
  std::array arr4 = {"Anil", "Busara", "Carl", "Dan"};
  output(il3);
  output(il4);
  output(arr3);
  output(arr4);
}

This results in two versions of output for array and one version for initializer_list. Hopefully they'll all be collapsed or inlined, but maybe not.

hsutter commented 3 months ago

Here's another case... should the user be able to expect they can modify the result? to be able to subscript?

main: () = {
    x : std::initializer_list = ( 1, 2, 3);
    x[0] = 0;       // error, initializer_list has no operator[]
    x.begin()* = 0; // error, can't assign to a const object
    for x do (e) std::cout << e;
}

Whereas:

main: () = {
    x : std::array = ( 1, 2, 3);
    x[0] = 0;       // ok
    x.begin()* = 0; // ok
    for x do (e) std::cout << e; // prints 023
}
hsutter commented 3 months ago

I think this is convincing me that array is the right answer. It is the C++ "stack array" type, which is a sensible default. And initializer_list really is designed to be primarily used as an initializer list for a non-deduced type.

DyXel commented 3 months ago

I guess it doesn't hurt sticking with std::array for now and see if somebody has some legitimate use cases for std::initializer_list.

gregmarr commented 3 months ago

Would it make sense for x := (1, 2, 3) to be initializer_list<int> and then the user can do x : std::array = (1, 2, 3); if they need to modify it?

How difficult would it be for cppfront to detect these patterns and suggest changing the type to std::array?

    x := (1, 2, 3);
    x[0] = 0;       // error, initializer_list has no operator[]
    x.begin()* = 0; // error, can't assign to a const object
hsutter commented 3 months ago

After trying it out and running regressions, there are a few conflicts, such as z := (move x); in a few test cases. While these seem to be test-style code, requiring such code to change to std::move just for initializers loses some generality, and would require getting rid of the diagnostic to make std::move an error. I'm not sure the usefulness justifies it.

I think I do prefer this which already works:

ints: const std::array = (11, 22, 33, 44, 55);

It's WYSIWYG/magic-free, and I think reasonably easy to type?

Thanks again! I'm going to close this without change for now, but that still leaves open the door to deduction in the future.