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

[BUG] Multidimensional std::array initialization #568

Open
jmafc opened this issue Aug 2, 2023 · 9 comments · May be fixed by #927
Open

[BUG] Multidimensional std::array initialization #568

jmafc opened this issue Aug 2, 2023 · 9 comments · May be fixed by #927
Labels
bug Something isn't working

Comments

@jmafc
Copy link

jmafc commented Aug 2, 2023

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 jmafc added the bug Something isn't working label Aug 2, 2023
@JohelEGP
Copy link
Contributor

JohelEGP commented Aug 2, 2023

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

@JohelEGP
Copy link
Contributor

JohelEGP commented Aug 2, 2023

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
Copy link
Contributor

JohelEGP commented Aug 2, 2023

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
Copy link
Contributor

JohelEGP commented Aug 2, 2023

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
Copy link

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
Copy link
Contributor

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]
}

@JohelEGP
Copy link
Contributor

JohelEGP commented Sep 18, 2023

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
Copy link
Contributor

JohelEGP commented Oct 5, 2023

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
Copy link
Contributor

JohelEGP commented Jan 9, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants