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

Lazily initialize cstring cache. #4866

Closed
wants to merge 1 commit into from
Closed

Conversation

asl
Copy link
Contributor

@asl asl commented Aug 12, 2024

The cstrings could be statically constructed (e.g. via cstring literals) but this requires cstring cache to be already alive.

The order of static ctors is not defined, so we need to ensure cstring cache is already there on the first use

The cstrings could be statically constructed (e.g. via cstring literals)
but this requires cstring cache to be already alive.

The order of static ctors is not defined, so we need to ensure
cstring cache is already there on the first use

Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info>
@asl asl added the core Topics concerning the core segments of the compiler (frontend, midend, parser) label Aug 12, 2024
@asl asl requested review from vlstill, qobilidop and fruffy August 12, 2024 18:26
@asl
Copy link
Contributor Author

asl commented Aug 12, 2024

Should be more complete solution compared to #4865

@asl asl added the run-sanitizer Use this tag to run a Clang+Sanitzers CI run. label Aug 12, 2024
static CacheType *g_cache = nullptr;
if (!g_cache) g_cache = new CacheType;

return *g_cache;
Copy link
Contributor

@ChrisDodd ChrisDodd Aug 12, 2024

Choose a reason for hiding this comment

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

static locals are defined to be initialized the first time the containing function is called, so there's no need to do this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hrm, you are right, this is a local, not global. Then I need to check what was going on for the original case...

@asl
Copy link
Contributor Author

asl commented Aug 12, 2024

Closing for now to investigate the original case again (see #4866 (comment) for more information). Will reopen with updated information what was going on.

@asl asl closed this Aug 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Topics concerning the core segments of the compiler (frontend, midend, parser) run-sanitizer Use this tag to run a Clang+Sanitzers CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants