hsutter / cppfront

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

[BUG] Multidimensional std::array initialization #568

Open jmafc opened 1 year ago

jmafc commented 1 year ago

Describe the bug Initialization of a two-dimensional std::array appears to be broken.

To Reproduce Steps to reproduce the behavior:

  1. Sample code
    main: () = {
    board: std::array<std::array<u8, 3>, 3> = (
        ('O', 'X', 'O'),
        (' ', 'X', 'X'),
        ('X', 'O', 'O')
    );
    i: i32 = 0;
    while (i < 3) {
        j: i32 = 0;
        while (j < 3) {
            std::cout << board[i][j];
            j++;
        }
        std::cout << '\n';
        i++;
    }
    }
  2. Command lines including which C++ compiler you are using

cppfront compiler v0.2.1 Build 8724:0846 g++ (Debian 12.2.0-14) 12.2.0

$ cppfront test.cpp2 -p
test.cpp2... ok (all Cpp2, passes safety checks)

$ c++ -std=c++20 -I ~/src/include test.cpp
  1. Expected result - what you expected to happen
OXO
 XX
XOO
  1. Actual result/error
    
    OXO
**Additional context**
`cppfront` translates the `board` declaration to:
```c++
   std::array<std::array<cpp2::u8,3>,3> board {
        ('O', 'X', 'O'), 
        (' ', 'X', 'X'), 
        ('X', 'O', 'O')}; 

i.e., it only replaces the outer parentheses by braces and g++ accepts that silently, although adding -Wall does emit warning: left operand of comma operator has no effect [-Wunused-value] for each element in the array. Examining the array after initialization in gdb shows:

$1 = {_M_elems = {{_M_elems = "OXO"}, {_M_elems = "\000\000"}, {
      _M_elems = "\000\000"}}}

which explains the actual output.

BTW, is there currently a way to do the equivalent of constexpr std::size_t ROWS = 3? I used ROWS: const i32 = 3 but I don't think that equivalent, plus if I'm not mistaken the const is superfluous. Also, I tried to write a for loop to iterate but couldn't quite get the syntax right. Finally, I tried replacing the std::array by std::vector and cppfront -p accepted it as valid, but then g++ did not like the parenthetical initializations.

JohelEGP commented 1 year ago

See #542. Currently, you only get braced-init-list in a few select places.

JohelEGP commented 1 year ago

BTW, is there currently a way to do the equivalent of constexpr std::size_t ROWS = 3? I used ROWS: const i32 = 3 but I don't think that equivalent, plus if I'm not mistaken the const is superfluous.

I do ROWS: constant<3> = ():

constant: <V: _> type == std::integral_constant<decltype(V), V>;

Also, I tried to write a for loop to iterate but couldn't quite get the syntax right.

It's for range next expression do (element) statement or for range do (element) statement. You can find how to extract the grammar at #358.

JohelEGP commented 1 year ago

This is how you can make it work today (https://cpp2.godbolt.org/z/815eovG3T):

main: () = {
    board: std::array<std::array<u8, 3>, 3> = (
        :std::array<u8, 3> = ('O', 'X', 'O'),
        :std::array<u8, 3> = (' ', 'X', 'X'),
        :std::array<u8, 3> = ('X', 'O', 'O')
    );
}
JohelEGP commented 1 year ago

I was just trying to cook up a hack to get Cppfront to lower a braced-init-list. Multidimensional std::array seems to be an outlier in that it requires an extra braced-init-list. See https://cpp2.godbolt.org/z/nYbcbP37T.

#include <array>
#define INIT(...) {__VA_ARGS__}
main: () = {
    board: std::array<std::array<u8, 3>, 3> = (INIT(
        INIT('O', 'X', 'O'),
        INIT(' ', 'X', 'X'),
        INIT('X', 'O', 'O')
    ));
}
AbhinavK00 commented 1 year ago

One way could be to implement a feature similar to P2163 in cpp2. cpp2 does not have the notion of braced-init list, native tuples could replace that while giving more features. Another alternative could be to implement simple array literals like other languages have, also recommended in #424.

farmerpiki commented 12 months ago

this also happens with boost::json tag_invoke_v2nonrepresentable: (copy : boost::json::value_from_tag, inout jv: boost::json::value, v: file_data) = { file_time_tp: = std::chrono::time_point_cast( v.timestamp - std::filesystem::file_time_type::clock::now() + std::chrono::system_clock::now()); file_time_t: std::time_t = std::chrono::system_clock::to_time_t(file_time_tp);

timestamp_str: std::string = fmt::format("{:%FT%T}", fmt::localtime(file_time_t));
jv = (("directory_name", v.directory_name),
("extension", v.extension),
("size", v.size),
("timestamp", timestamp_str));

}

this is the cpp version of the function I was trying to replace void tag_invoke(boost::json::value_from_tag, boost::json::value &jv, file_data const &v) { auto file_time_tp = std::chrono::time_point_cast( v.timestamp - std::filesystem::file_time_type::clock::now() + std::chrono::system_clock::now()); std::time_t file_time_t = std::chrono::system_clock::to_time_t(file_time_tp);

std::string timestamp_str = fmt::format("{:%FT%T}", fmt::localtime(file_time_t)); jv = {{"directory_name", v.directory_name}, {"extension", v.extension}, {"size", v.size}, {"timestamp", timestamp_str}}; }

I get these warnings instead, the code compiles though which I think is dangerous! main2.cpp2:269:10: warning: left operand of comma operator has no effect [-Wunused-value] jv = { ("directory_name", v.directory_name), ^~~~ main2.cpp2:270:3: warning: left operand of comma operator has no effect [-Wunused-value] ("extension", v.extension), ^~~ main2.cpp2:271:3: warning: left operand of comma operator has no effect [-Wunused-value] ("size", v.size), ^~ main2.cpp2:272:3: warning: left operand of comma operator has no effect [-Wunused-value] ("timestamp", std::move(timestamp_str)) }; ^~~ main2.cpp2:409:1: warning: non-void function does not return a value [-Wreturn-type] }

JohelEGP commented 12 months ago

Yeah, that's double balanced parentheses, where the outer one is an initializer list, and the inner one is a primary-expression. This can be confusing due to Cpp2's overloading of parenthesis and lack of distinct braced-init-list syntax (#542). This particular formulation is worth making ill-formed when non-last expressions are not discarded (e.g. ((_ = a, b)) is OK).

JohelEGP commented 11 months ago

Ah, but requiring _ = a in (_ = a, b) would prevent you from invoking a comma operator. I know that Cpp2 won't allow authoring operator,, but what about calling it?

JohelEGP commented 8 months ago

There's also the option of lowering all expression lists within an initializer as braced-init-lists. If you really wanted to use the comma operator, you can defer it to a IIFE: :std::array<int, 2> = (x, :() (y, z)$;()).