Skip to content

[unsafe] Fix non-const global initializers and [unsafe, refcount] order of global initialization - #366

Merged
nunoplopes merged 11 commits into
Cpp2Rust:masterfrom
lucic71:global-initialized-with-non-const
Sep 17, 2026
Merged

nunoplopes merged 11 commits into
Cpp2Rust:masterfrom
lucic71:global-initialized-with-non-const

Conversation

@lucic71

@lucic71 lucic71 commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Rust requires initializers of global variables to be const. C++ does not require that.

To fix this tension, wrap global variables in std::cell::LazyCell so that initialization does not depend on the constness of the initializer.

Another problem that this PR fixes is the moment when global variables are initialized in both refcount and unsafe. thread_local in refcount and the new LazyCell in unsafe initialize the variable on the first usage. This is wrong if the constructors of the global variables have side effects that must be visible in the program, for example:

#include <cassert>

static int total = 0;

struct S {
  S(int x) { total += x; }
};

// there is no explicit usage of a and b inside the program other than the initialization
// however the constructor has side effects that are visible on the total variable
// so the initialization has to happen before entering main, not lazily as thread_local
// and LazyCell normally do
static S a(1);
static S b(10);

int main() {
  assert(total == 11);
  return 0;
}

To fix this, I added a new function that runs before main and forces the initialization of global variables: __cpp2rust_init_globals().

@nunoplopes

Copy link
Copy Markdown
Contributor

It's weird that LazyCell is not used in any new test case, only in existing test cases, that didn't need it because they were working?

Also, the new function call could be skipped if unneeded to reduce test case churn.

@lucic71

lucic71 commented Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

It's weird that LazyCell is not used in any new test case, only in existing test cases, that didn't need it because they were working?

There are new tests: tests/unit/global_non_const_init.cpp and tests/unit/global_init_side_effect.cpp

Also, the new function call could be skipped if unneeded to reduce test case churn.

Yes, I think that could be done

@lucic71

lucic71 commented Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Also, the new function call could be skipped if unneeded to reduce test case churn.

Yes, I think that could be done

Not emitting the empty function would equally churn the tests because main would have to be moved at the end of each file. That's the only place where we know if a global needs initialization or not.

@nunoplopes

Copy link
Copy Markdown
Contributor

Also, the new function call could be skipped if unneeded to reduce test case churn.

Yes, I think that could be done

Not emitting the empty function would equally churn the tests because main would have to be moved at the end of each file. That's the only place where we know if a global needs initialization or not.

You can check if there are any globals with non-const initializers when emitting main.

The new tests don't have LazyCell, which is very weird.

Comment on lines +20 to +21
pub static mut a_1: std::cell::LazyCell<S> = std::cell::LazyCell::new(|| unsafe { S::S({ 1 }) });
pub static mut b_2: std::cell::LazyCell<S> = std::cell::LazyCell::new(|| unsafe { S::S({ 10 }) });

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LazyCell is used in the new tests here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I forgot to mention explicitly is that LazyCell is only for unsafe. Refcount continues to use thread_local Value

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then don't generate the function call in refcount.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refcount also needs that. Thread locals are initialized lazily. We need them to be initialized before main starts

Comment on lines +52 to +63
pub static mut default_ctor_7: std::cell::LazyCell<Ctor> =
std::cell::LazyCell::new(|| unsafe { Ctor::Ctor1() });
pub static mut arg_ctor_8: std::cell::LazyCell<Ctor> =
std::cell::LazyCell::new(|| unsafe { Ctor::Ctor2({ 7 }) });
pub static mut str_9: std::cell::LazyCell<Vec<libc::c_char>> =
std::cell::LazyCell::new(|| unsafe {
{
let s = c"abc".as_ptr();
std::slice::from_raw_parts(s, (0..).take_while(|&i| *s.add(i) != 0).count() + 1)
.to_vec()
}
});

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LazyCell is used in the new tests here

@lucic71

lucic71 commented Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

Also, the new function call could be skipped if unneeded to reduce test case churn.

Yes, I think that could be done

Not emitting the empty function would equally churn the tests because main would have to be moved at the end of each file. That's the only place where we know if a global needs initialization or not.

You can check if there are any globals with non-const initializers when emitting main.

This does not work if globals are declared after main in both single-TU or multi-TU. We still need to move main at the end of the file which will equally churn the tests.

I propose always emitting the function and generating an empty body if no globals need initialization. This keeps the converter simpler.

@lucic71 lucic71 changed the title Fix non-const global initializers and order of global initialization [unsafe] Fix non-const global initializers and [unsafe, refcount] order of global initialization Sep 15, 2026
@nunoplopes

Copy link
Copy Markdown
Contributor

Branch with conflicts

@nunoplopes

Copy link
Copy Markdown
Contributor

Also, the new function call could be skipped if unneeded to reduce test case churn.

Yes, I think that could be done

Not emitting the empty function would equally churn the tests because main would have to be moved at the end of each file. That's the only place where we know if a global needs initialization or not.

You can check if there are any globals with non-const initializers when emitting main.

This does not work if globals are declared after main in both single-TU or multi-TU. We still need to move main at the end of the file which will equally churn the tests.

I propose always emitting the function and generating an empty body if no globals need initialization. This keeps the converter simpler.

Can't you traverse the AST and look for globals?

@lucic71

lucic71 commented Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

Also, the new function call could be skipped if unneeded to reduce test case churn.

Yes, I think that could be done

Not emitting the empty function would equally churn the tests because main would have to be moved at the end of each file. That's the only place where we know if a global needs initialization or not.

You can check if there are any globals with non-const initializers when emitting main.

This does not work if globals are declared after main in both single-TU or multi-TU. We still need to move main at the end of the file which will equally churn the tests.
I propose always emitting the function and generating an empty body if no globals need initialization. This keeps the converter simpler.

Can't you traverse the AST and look for globals?

I can, but I usually avoided traversing the AST more than once

@nunoplopes
nunoplopes merged commit 104c483 into Cpp2Rust:master Sep 17, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants