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

Switch to using the standards-conforming preprocessor for C++ #4407

Draft
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

eric-hughes-tiledb
Copy link
Contributor

@eric-hughes-tiledb eric-hughes-tiledb commented Oct 10, 2023

MSVC by default uses its traditional, but non-conforming, preprocessor. The occasion for the switch is the new __VA_OPT__ preprocessor macro in C++20, which allows a standard way of handling commas with __VA_ARGS__.

REQUIRES: No longer running windows-19 in CI. It looks like there is incompatibility between the strict preprocessor and that version of the compiler libraries.


TYPE: NO_HISTORY
DESC: Switch to using the standards-conforming preprocessor for C++

@eric-hughes-tiledb
Copy link
Contributor Author

This should be rebased after #4405 merges to see if it alleviates anything.

Copy link
Member

@ihnorton ihnorton left a comment

Choose a reason for hiding this comment

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

As long as this passes with all current dependencies, it seems safe (used by LLVM and OpenJDK; not used by Chromium or Arrow) -- but please make sure this does not bleed into the public C or C++ APIs.

@KiterLuc KiterLuc force-pushed the eh/msvc-conforming-preprocessor branch from 756e5ea to 1693277 Compare October 11, 2023 15:41
MSVC by default uses its traditional, but non-conforming, preprocessor. The occasion for the switch is the new `__VA_OPT__` preprocessor macro in C++20, which allows a standard way of handling commas with `__VA_ARGS__`.
@eric-hughes-tiledb
Copy link
Contributor Author

It looks as though this strict preprocessor option is not compatible with VS 2019; there's warning inside the compiler's include files that's being escalated to an error. I doubt it's worth developing a workaround.

There's a different error in VS 2022. This is from our dependency clipp not working with the strict preprocessor.

Changing the description to match these observations.

@teo-tsirpanis
Copy link
Member

4eda19c fixes the clipp incompatibility, feel free to cherry-pick.

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.

None yet

3 participants