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 using std::filesystem when not available #2317

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

dimateos
Copy link

Ref #2316

I simply removed the _MSC_VER check.

Feel free to edit or discard.
Cheers!

@paulhoux
Copy link
Collaborator

The Debug target seems to have a few errors. If you can fix those, I'll run the tests again.

@dimateos
Copy link
Author

Seems like MSVC does not define __cplusplus according to the picked std=c++XX prior to 2017.
Moreover, due to legacy code expecting it to still be 199711L this is disabled by default and requires /Zc:__cplusplus.

ref : https://devblogs.microsoft.com/cppblog/msvc-now-correctly-reports-__cplusplus/

Instead, we can additionally check _MSVC_LANG which always reports the c++ std version
Basically, when /Zc:__cplusplus is enabled: __cplusplus == _MSVC_LANG

ref: https://learn.microsoft.com/en-us/cpp/preprocessor/predefined-macros?view=msvc-170

I added the explanation to the file too

@dimateos
Copy link
Author

dimateos commented Nov 16, 2023

Well there were a few more issues, like the source .cpp not added to the solution (thats why I missed updating it). Basically the project compiled in c17 because ghc was not being used, otherwise it would have failed at the linker stage when using the lib i.e. in the samples.

Now I tested both and succesfully compiled cinder with c17 and c14, plus also a few samples with either c++ using the output lib :)

@paulhoux paulhoux requested a review from andrewfb November 16, 2023 16:28
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