header-units.json: Comment out version, yvals.h, yvals_core.h #1781
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reported by @olgaark and confirmed by @cdacamar.
Header units are perfectly capable of emitting macros. However, the IDE's build system can be configured to perform "dependency scanning" when translating
#include
toimport
. During this scanning phase, the compiler and build system are effectively assuming that anything named inheader-units.json
won't emit macros that control the inclusion of other headers. (This is because we haven't built the header units yet - we're scanning dependencies to determine what header units to build in what order - and we're trying to avoid preprocessor-expanding anything named inheader-units.json
as that would defeat the purpose.)This means that
yvals.h
andyvals_core.h
shouldn't be marked as eligible for automatic translation of#include
toimport
inheader-units.json
, because every STL header starts by including one of these two, and expects that they'll define macros that control whether other headers should be included. (Beginning with_STL_COMPILER_PREPROCESSOR
guarding everything, also_HAS_CXX17
/_HAS_CXX20
and the Standard feature-test macros.) (Note that_HAS_CXX17
/_HAS_CXX20
are defined byvcruntime.h
included byyvals_core.h
.)Finally, while the Standard specifies that
<version>
is importable, and this is indeed the case (import <version>;
works just fine), we should also comment it out inheader-units.json
. While we never include<version>
ourselves, its only purpose in life is to provide Standard feature-test macros, so users relying on dependency scanning and automatic translation of#include
toimport
would run into the same problem if they#include <version>
and then guard other includes with feature-test macros.(If users are including other Standard headers to get feature-test macros to control the inclusion of other headers, it seems unavoidable that they'll have problems with dependency scanning and automatic translation - but using
<version>
is a superior strategy to begin with.)