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

command line flag --std is redundant... and incorrect #72

Closed
joeroback opened this issue Dec 23, 2016 · 6 comments
Closed

command line flag --std is redundant... and incorrect #72

joeroback opened this issue Dec 23, 2016 · 6 comments

Comments

@joeroback
Copy link

I do not see the purpose of the command line flag --std... I use CMake to determine the C++ standard, c++11, gnu++11, c++14, gnu++14, etc. Yet, cmake-js seems to just append a standard... regardless of whether cmake has discovered one. Example

add_library(${PROJECT_NAME} SHARED ${SOURCES})
set_target_properties(${PROJECT_NAME} PROPERTIES CXX_STANDARD 14 CXX_STANDARD_REQUIRED ON)

Will result in -std=gnu++14 on Linux, yet cmake-js will put -std=c++11 by default in CMAKE_CXX_FLAGS... and even if you pass --std gnu++14 it duplicates it on the command line.

Can there be an option to not do this and let CMake handle it?

@unbornchikken
Copy link
Member

Yeah, but CXX_STANDARD is available from CMake 3.1. However, there should be an option for turn std coercion argument of, std=auto. And I can make this to default on the next semver major update. What do you say?

@joeroback
Copy link
Author

yea, whatever you see fit. As long as I can have an option where it allows me to have CMake decide I am happy. Thanks!

I just think having it in general defeats the purpose of using CMake, which is supposed to be in charge of CXXFLAGS. If I was using CMake < 3.1, and I wasn't using cmake-js, I'd have to put it in CMakeLists.txt anyway, so I really don't see the purpose of this option.

@Qix-
Copy link

Qix- commented Sep 14, 2018

It shouldn't be included at all. This is what the following is for:

if (("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang") OR ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU"))
	set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11")
endif ()

Please don't put opinionated functionality that isn't absolutely necessary to build Node addons in Cmake.js. It only hurts users trying to move away from an overly opinionated system (gyp).

@unbornchikken
Copy link
Member

Yeah, I get it, you are right. I'm gonna remove std option in the next major release.

@Qix-
Copy link

Qix- commented Sep 14, 2018

❤️ Thanks @unbornchikken.

@unbornchikken
Copy link
Member

Fixed as of v4.0.0.

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

3 participants