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

Fix eaassert.h include guard #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

IncludeGuardian
Copy link

@IncludeGuardian IncludeGuardian commented Mar 26, 2023

eaassert.h does not have a strict enough include guard to trigger the multiple inclusion optimization on most compilers.

The #if defined(EA_PRAGMA_ONCE_SUPPORTED) check occurs before any include of eacompilertraits.h (where EA_PRAGMA_ONCE_SUPPORTED is defined) so the preprocessor sees the #pragma once only if eaassert.h is included after another header that includes eacompilertraits.h.

The traditional include guard is also not strict enough to trigger the multiple-inclusion optimization as the only tokens that can appear outside of the #ifdef/#endif pair are whitespace, comments, and the null directive # (for certain compilers
https://gcc.gnu.org/onlinedocs/cppinternals/Guard-Macros.html). However, the previously mentioned EA_PRAGMA_ONCE_SUPPORTED check disables this optimization.

This commit moves the #pragma once code after the include directive where EA_PRAGMA_ONCE_SUPPORTED would be defined. It also tidies up the traditional include guard so that it satisfies the stricter rules mentioned above.

This was issues was found with IncludeGuardian 0.0.7.

`eassert.h` does not have a strict enough include guard to trigger the multiple
inclusion optimization on most compilers.

The `#if defined(EA_PRAGMA_ONCE_SUPPORTED)` check occurs before any include of
`eacompilertraits.h` (where `EA_PRAGMA_ONCE_SUPPORTED` is defined) so the
preprocessor sees the `#pragma once` only if `eassert.h` is included **after**
another header that includes `eacompilertraits.h`.

The traditional include guard is also not strict enough to trigger the
multiple-inclusion optimization as the only tokens that can appear outside of
the `#ifdef`/`#endif` pair are whitespace, comments, and the null directive `#`
(for certain compilers
https://gcc.gnu.org/onlinedocs/cppinternals/Guard-Macros.html).  However, the
previously mentioned `EA_PRAGMA_ONCE_SUPPORTED` check disables this
optimization.

This commit moves the `#pragma once` code after the include directive where
`EA_PRAGMA_ONCE_SUPPORTED` would be defined.  It also tidies up the traditional
include guard so that it satisfies the stricter rules mentioned above.

This was issues was found with IncludeGuardian 0.0.7.
@IncludeGuardian
Copy link
Author

The full output from IncludeGuardian (https://includeguardian.io) with this particular issue highlighted can be found here https://gist.github.com/IncludeGuardian/a3716704237848e258471471b058879a#file-eastl-yaml-L160-L161

@IncludeGuardian
Copy link
Author

@grojo-ea Would anyone be able to take a look at this?

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.

1 participant