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

Add more default flags for C++ files with MSVC #1003

Closed
paul-reilly opened this issue Oct 22, 2020 · 2 comments
Closed

Add more default flags for C++ files with MSVC #1003

paul-reilly opened this issue Oct 22, 2020 · 2 comments
Milestone

Comments

@paul-reilly
Copy link
Contributor

/EHsc

/EHsc is added as a default flag in CMake-produced ninja and msbuild files :

from Ninja output

/nologo /TP /DWIN32 /D_WINDOWS /GR /EHsc /Zi /Ob0 /Od /RTC1 -MDd /showIncludes /FS -c

from msbuild output

/Zi /nologo /W1 /WX- /diagnostics:column /Od /Ob0 /D WIN32 /D _WINDOWS /D "CMAKE_INTDIR=\"Debug\"" /D _MBCS /Gm- /EHsc /RTC1 /MDd /GS /fp:precise /Zc:wchar_t /Zc:forScope /Zc:inline /GR /Gd /TP /errorReport:queue

I have had errors with vcpkg libraries where I have forgot to specify an exception handling flag with xmake too. I think this is would be a sensible default flag to add.

/permissive-

I personally think that /permissive- would be a good default too since writing non-conformant code is not the usual use case and not having this causes errors with conforming code, eg with if (x == 1 and b == 2), the C++ and keyword is not permitted.

@waruqi
Copy link
Member

waruqi commented Oct 23, 2020

Usually xmake will add as few default flags as possible to ensure that users are flexible enough to configure. However, in the new version, users have been able to override the default flags (#978), so adding /EHsc as the default flags is also acceptable.

I have added it in the dev branch, you can update to dev and try it.

However, I still won't add too many default flags. Therefore, I will not add /permissive- yet unless it is really necessary.

@waruqi waruqi added this to the v2.3.9 milestone Oct 23, 2020
@paul-reilly
Copy link
Contributor Author

Thanks, that's working!

/permissive- isn't really necessary, it's default in VS2017+ but maybe more time needs to pass before it's a good default for everybody.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants