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

squelch C4232 warnings on MSVC builds #504

Merged
merged 1 commit into from
Jan 6, 2024
Merged

Conversation

compnerd
Copy link
Contributor

@compnerd compnerd commented Jan 2, 2024

C4232 appertains to the identity of dllimported functions [1]. The address of dllimport'ed functions are not guaranteed to maintain identity as the address will be the address of the IAT thunk, which is module specific. Two modules which bind to the same implementation may have different addresses. However, since the use of this is for free, it should be relatively safe as we do not expect to perform pointer identity comparisons.

[1] https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4232?view=msvc-170

@compnerd
Copy link
Contributor Author

compnerd commented Jan 2, 2024

With this change, I actually can build with VS2022 and no warnings!

C4232 appertains to the identity of dllimported functions [1]. The
address of dllimport'ed functions are not guaranteed to maintain
identity as the address will be the address of the IAT thunk, which is
module specific.  Two modules which bind to the same implementation may
have different addresses. However, since the use of this is for `free`,
it should be relatively safe as we do not expect to perform pointer
identity comparisons.

[1] https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4232?view=msvc-170
@compnerd compnerd force-pushed the 4232 branch 2 times, most recently from f6ff5da to 891d0a1 Compare January 5, 2024 23:16
@jgm jgm merged commit 820df08 into commonmark:master Jan 6, 2024
12 checks passed
@compnerd compnerd deleted the 4232 branch January 6, 2024 16: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.

2 participants