use a repeat expression for arrays with a single initializer

[folkertdev]

translate

unsigned short data[DATA_SIZE] = {0};

to

let mut data: [libc::c_ushort; 65535] = [0 as libc::c_int as libc::c_ushort; 65535];

instead of a literal with 65535 zeros.

I also simplified the logic. Although https://github.com/GaloisInc/C2Rust/issues/40 is a dead link I believe the second commit is correct.

[folkertdev]

Looks like you need to update your CI to use a newer ubuntu version (we usually use ubuntu-latest, but maybe there is a reason you pin a specific version?)

This is a scheduled Ubuntu 20.04 retirement. Ubuntu 20.04 LTS runner will be removed on 2025-04-15. For more details, see actions/runner-images#11101

[kkysen]

Looks like you need to update your CI to use a newer ubuntu version (we usually use ubuntu-latest, but maybe there is a reason you pin a specific version?)

This is a scheduled Ubuntu 20.04 retirement. Ubuntu 20.04 LTS runner will be removed on 2025-04-15. For more details, see actions/runner-images#11101

I forget exactly why but the latest Ubuntu might've broke something in CI (it did for rav1d recently).

[folkertdev]

Ok well maybe 22.04 still works, or maybe latest got updated with a fix? (I can try that too)

I'm also not sure what's up with the azure runners, but it doesn't look like the changes in the PR would cause those errors:

https://dev.azure.com/immunant/c2rust/_build/results?buildId=4343&view=logs&j=f7d03aff-a499-5cb4-81d5-3cd2462ec94c&t=2246cd89-1657-535f-9a56-0b87376126d6&l=282

Maybe pinning once_cell to a slightly older version works there?

[thedataking]

I'm also not sure what's up with the azure runners

I am going to switch us off the Azure runners as soon as I get the time to do so. If you've tested locally and are convinced your change is not the cause of the breakage, that's good enough for me. Feel free to merge without CI passing. And thanks for contributing this improvement!

[thedataking]

Quick note @folkertdev : I would like to avoid PR that changes the transpiler AND the CI. Those should be separate PRs.

(Testing on this PR is fine but don't feel obliged to try to fix the CI)

[folkertdev]

ah, I'll split that out then. I'd just like CI to check my work, not everything runs locally.

[folkertdev]

I misinterpreted what C actually does earlier. The usage of

int foo[] = { 0 }

is extremely widespread, but at least with modern compilers it is equivalent in practice to

int foo[] = { }

Anyway, that is not at all similar to int foo[16] = { 1 } , which does not repeat the 1 16 times, but instead zeroes out the remainder of the array.

So this has now been fixed to still detect the extremely common { 0 } scenario, but leave other incomplete initializers alone.

---

I ran test file locally and diffed the outputs. I'm not able to run any of the tests though, e.g. this fails

> ./scripts/test_translator.py --only-directories="arrays" tests
arrays:
 [ FAILED ] translate incomplete_arrays.c
incomplete_arrays.c:5:8: error: unknown type name 'uint32_t'
    5 | extern uint32_t SOME_INTS[];
      |        ^
warning: c2rust: Encountered unsupported BuiltinType kind 489 for type <dependent type>

That file does #include <stdint.h>, so I'm not sure what the problem is. Anyway, in terms of testing that is kind of the best I can do I think?

[kkysen]

@folkertdev, I merged your CI PR #1219 now, so you can rebase this now if you want and hopefully more CI works now. I didn't want to rebase for your in case it messes anything up you might've been working on.

[folkertdev]

Well I was playing with more complicated zero initializers (e.g. struct {short; int y;} arr6[4] = { 0 };, but I don't really know enough about C (or C++, and the AST you get from clang seems to use a bunch of C++ terminology) to be sure that the logic is correct.

So let's merge this when CI is happy, and then we'll see if the more complex case comes up.

[folkertdev]

Btw, wouldn't something like cargo insta (or some other snapshot testing solution) be really useful for testing the output of c2rust-transpile? I've basically been doing that manually. Unless I'm missing something, the current tests really only test that the c2rust-transpile does not panic (and maybe that it generates rust that compiles?). What it generates exactly does not appear to be tracked?

[bungcip]

@folkertdev some test like

c2rust/tests/items/src/test_fn_attrs.rs

Line 24 in 71c95a2

assert!(src.contains("#[inline(always)]\nunsafe extern \"C\" fn rust_always_inline_static"));

appear to test the output (basically just include_str! and check if string contains suitable output)

[kkysen]

Could you add to test_arrays.rs that these arrays translate as expected?

[folkertdev]

What actually runs the logic in test_arrays.rs? just a cargo test does not seem to do it, the functions in that file appear to never be actually called, and they are not marked #[test].

[kkysen]

We specifically want zero initialization, not necessarily default initialization, although they are usually the same, and seems to be for all cases in fn implicit_default_expr. It seems like it doesn't handle structs/unions/enums, though, which fn zero_initializer does, so we probably want to use both and clarify the naming/docs to ensure it's specifically zero.

[kkysen]

Btw, wouldn't something like cargo insta (or some other snapshot testing solution) be really useful for testing the output of c2rust-transpile? I've basically been doing that manually. Unless I'm missing something, the current tests really only test that the c2rust-transpile does not panic (and maybe that it generates rust that compiles?). What it generates exactly does not appear to be tracked?

Yes, it definitely would. I've used it in newer tests like for c2rust-pdg and it worked great, but we haven't had time to change our existing transpiler tests. It would be super helpful to have the translated output checked in and reviewable.