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

Change logic for how we determine how many bits a ptr has #1091

Closed
wants to merge 1 commit into from

Conversation

DmitriyMusatkin
Copy link
Contributor

@DmitriyMusatkin DmitriyMusatkin commented Feb 13, 2024

Issue #, if available:
#932

Description of changes:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@@ -15,9 +15,9 @@
AWS_PUSH_SANE_WARNING_LEVEL

/* The number of bits in a size_t variable */
#if SIZE_MAX == UINT32_MAX
#if UINTPTR_MAX == UINT32_MAX
Copy link
Contributor

Choose a reason for hiding this comment

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

Tsk, tsk, that other library isn't defining SIZE_MAX to spec:

https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf

Each instance of these macros shall be replaced by a constant expression suitable for use
in #if preprocessing directive

Copy link
Contributor

Choose a reason for hiding this comment

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

But seriously, if SIZE_MAX isn't defined, and so that other library is defining it, what makes us think UINTPTR_MAX is defined?

What environment is that user in?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

linked the issue. artery library used to always override SIZE_MAX when compiled using C++ with non-preprocessor friendly value.

since we only support c99+ and its been ages for all compilers to add those defines to their header, i would not be too concerned with UINTPTR_MAX missing - if its missing then there are likely myriad of other things we dont support.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why can't this be a cmake feature check rather than in-source adhoc #if?

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.

3 participants