cppfront icon indicating copy to clipboard operation
cppfront copied to clipboard

[BUG] Multidimensional std::array initialization

Open jmafc opened this issue 1 year ago • 9 comments

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++;
    }
}
  1. 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:

   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.

jmafc avatar Aug 02 '23 13:08 jmafc

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

JohelEGP avatar Aug 02 '23 13:08 JohelEGP

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 avatar Aug 02 '23 13:08 JohelEGP

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 avatar Aug 02 '23 13:08 JohelEGP

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')
    ));
}

JohelEGP avatar Aug 02 '23 13:08 JohelEGP

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.

AbhinavK00 avatar Aug 02 '23 16:08 AbhinavK00

this also happens with boost::json tag_invoke_v2_nonrepresentable: (copy _: boost::json::value_from_tag, inout jv: boost::json::value, v: file_data) = { file_time_tp: = std::chrono::time_point_caststd::chrono::system_clock::duration( 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_caststd::chrono::system_clock::duration( 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] }

farmerpiki avatar Sep 18 '23 10:09 farmerpiki

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 avatar Sep 18 '23 13:09 JohelEGP

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 avatar Oct 05 '23 21:10 JohelEGP

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)$;()).

JohelEGP avatar Jan 09 '24 16:01 JohelEGP