Skip to content

Conversation

apaatsio
Copy link

Save the users a Google search and tell them what compiler version they need.

See: #448

Save the users a Google search and tell them what compiler version they need.

See: nodejs#448
@rvagg
Copy link
Member

rvagg commented Jan 28, 2016

This would need an MSVC version too for completeness

@kkoopa
Copy link
Collaborator

kkoopa commented Jan 28, 2016

I do not like it. Listing versions explicitly is too much detail. There can
still be discrepancies, compiler bugs, vendors not applying the right patches,
wrong flags and so on. Not to mention that no version of msvc truly has proper
C++11 support yet.

The code which produces the error message mostly relies on feature detection,
and it is merely there as convenience to limit the number of frivolous issues
due to not grokking the compiler error which would be produced otherwise. What
will happen when the next compiler comes along and is not listed?

On Thursday 28 January 2016 02:29:44 Antti Ahti wrote:

Save the users a Google search and tell them what compiler version they
need.

See: #448
You can view, comment on, or merge this pull request online at:

#536

-- Commit Summary --

  • Add required compiler versions in the error msg

-- File Changes --

M nan.h (2)

-- Patch Links --

https://github.com/nodejs/nan/pull/536.patch
https://github.com/nodejs/nan/pull/536.diff


Reply to this email directly or view it on GitHub:
#536

@apaatsio
Copy link
Author

Yeah, it's a trade-off between "more useful error message for most users" and "not so useful but accurate error message"

@ranisalt
Copy link
Contributor

Clang 3.3 has full C++11 support already, but Nan probably can be compiled with Clang 3.1 since patches 3.2 and 3.3 are not very used (source)

@agnat
Copy link
Collaborator

agnat commented Oct 15, 2016

I'm with @kkoopa on this. We (try to) point the user in a clear direction. Maintaining a list of working compilers is beyond our scope.

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.

5 participants