Skip to content

Conversation

@polter-rnd
Copy link
Contributor

This change replaces the inline definition of all_suites with a static one to avoid static initialization order issues, particularly on Clang + Windows.

This ensures all_suites is initialized before any static test registrations, in accordance with the C++ standard, which guarantees initialization order for static variables within a single translation unit.

However, this imposes a limit on test executable to have only one translation unit. Not sure how to do it better without explicit initialization point and slightly more boilerplate, which this patch avoids for now.

Fixes #52

@jimporter
Copy link
Owner

While I'm fairly sure this is a compiler bug, if we did work around this, I think the right way would be to just revert the relevant part of 7a9adee and use the old implementation:

inline suites_list & all_suites() {
  static suites_list all;
  return all;
}

@polter-rnd
Copy link
Contributor Author

I think it makes sense to revert it then, otherwise it's not usable on Clang/Windows. Shall I update the MR?

@jimporter
Copy link
Owner

As mentioned in #52, I think there's a good chance this is a Clang bug. I'll try to make a minimal reproducer and file this upstream. If it turns out that I'm wrong and Clang's behavior on Windows is standards-compliant, then I'll (partially) revert 7a9adee.

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.

Static initialization order fiasco with inline all_suites on Windows + Clang

2 participants