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

avoid using already defined thread_local as variable name #667

Merged
merged 1 commit into from
Jan 6, 2025

Conversation

brechtsanders
Copy link
Contributor

Building p11-kit 0.25.5 with GCC15 on MinGW-w64 failed because thread_local is already defined for this platform.

Resolved by changing the variable name from thread_local to threadlocal.

Building p11-kit 0.25.5 with GCC15 on MinGW-w64 failed because `thread_local` is already defined for this platform.

Resolved by changing the variable name from `thread_local` to `threadlocal`.
@kalvdans
Copy link

kalvdans commented Jan 3, 2025

GCC 15 has not been released yet, could you confirm the compiler version? [EDIT] Reproduced in godbolt with GCC trunk: https://godbolt.org/z/P1j1vfeWM

Copy link

@kalvdans kalvdans left a comment

Choose a reason for hiding this comment

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

thread_local was added as a keyword in C23:

https://en.wikipedia.org/wiki/C23_(C_standard_revision)#Keywords

Instead of specifying -std=c17 I agree renaming a variable is best. (I have no merge rights)

@kalvdans
Copy link

kalvdans commented Jan 3, 2025

Thinking more about it, we should instead (or in addition to the rename) make sure P11_TLS_KEYWORD gets defined on GCC15.

@brechtsanders
Copy link
Contributor Author

brechtsanders commented Jan 3, 2025

Yes, I was testing a snapshot version of GCC15 as it will default to -std=c23 causing some changes that break a lot of software builds.

See also: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118283

Copy link
Member

@ueno ueno left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@coveralls
Copy link

Coverage Status

coverage: 69.528%. remained the same
when pulling f2acbe8 on brechtsanders:patch-1
into fa9ec0e on p11-glue:master.

@ueno ueno merged commit fd7ad39 into p11-glue:master Jan 6, 2025
14 checks passed
@brechtsanders brechtsanders deleted the patch-1 branch January 6, 2025 05:26
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.

4 participants