Skip to content

<xkeycheck.h>: Fix circular inclusion for Header Units #1692

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.

Sign up for GitHub

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

Merged
StephanTLavavej merged 1 commit into from
Feb 26, 2021
Merged

<xkeycheck.h>: Fix circular inclusion for Header Units #1692

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion stl/inc/xkeycheck.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@
#pragma once
#ifndef _XKEYCHECK_H
#define _XKEYCHECK_H
#include <yvals_core.h>

// xkeycheck.h assumes that it's being included by yvals_core.h in a specific order.
Copy link
Contributor

@CaseyCarter CaseyCarter Feb 24, 2021

Choose a reason for hiding this comment

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

Instead of assuming, should we #error if _YVALS_CORE_H_ isn't defined? I'm inclined to not check, since (1) as you say elsewhere no user should ever include this and there's no reason to include from elsewhere in the STL, and (2) if someone does somehow manage to include this before yvals_core.h the effect will simply be to disable the contents (_STL_COMPILER_PREPROCESSOR won't be defined). There's no danger of silent bad codegen.

The flip side of this coin is, if this file is only supposed to be included from one place ever, why not inline its contents and delete this file instead of forcing the preprocessor to inline it into yvals_core.h on every compile?

(This is an "approve with suggestions" comment: it's worth discussion, but I certainly won't be bothered if we merge this as is.)

Copy link
Member Author

@StephanTLavavej StephanTLavavej Feb 24, 2021

Choose a reason for hiding this comment

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

I'm inclined to not check

Same. I would add (3) this wouldn't enforce the particular ordering we care about (early enough to protect most of yvals_core.h from macros, but after the definitions of _STL_COMPILER_PREPROCESSOR and _HAS_CXXmeow).

The flip side of this coin is, if this file is only supposed to be included from one place ever, why not inline its contents and delete this file instead of forcing the preprocessor to inline it into yvals_core.h on every compile?

We could do that (as we recently did for <cmath>), but xkeycheck.h is rather large (almost 600 lines, 20 KB) and extremely boring (it should do nothing for builds that aren't doomed); yvals_core.h is already big enough (almost 1400 lines, 67 KB) and contains lots of important stuff.

What I really want is for the compilers to reward keyword macros with doom.

// Nothing else should include xkeycheck.h.

#if _STL_COMPILER_PREPROCESSOR

#if defined(__cplusplus) && !defined(_ALLOW_KEYWORD_MACROS) && !defined(__INTELLISENSE__)
Expand Down