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

Proposal: Become C++11 code base, Remove Support for VS 2013 builds #2549

Closed
ianwjhalliday opened this issue Feb 16, 2017 · 18 comments
Closed

Comments

@ianwjhalliday
Copy link
Collaborator

TL;DR: Remove VS 2013 support in ChakraCore so we can use the full C++11 feature set, following suit with the rest of the C++ community.

Proposal

Drop support for ChakraCore build using VS 2013 tools.

Require VS 2015 toolset.

Proposed Plan

Remove VS2013 support as part of v2.0 release of ChakraCore.

Maintain VS2013 support in v1.x release branches.

Impact

No impact on JSRT consumers since JSRT is a C API.

No impact on Nuget consumers since prebuilt binaries provided.

Impact users that build ChakraCore 2.0+ from source. Require them to upgrade their MSVC build toolset to minimum VS 2015.

Motivation

Supporting Motivation

Node.js has dropped VS2013 support in favor of using C++11 features. Node.js v6.8 (Oct 12 2016) and v7.x (upcoming) do not support VS2013, only VS2015.

ICU v58 (released Oct 24 2016) does not support VS2013, requires VS2015 or cygwin for Windows builds.

BlueJeans are not dependent on VS2013, they are using VS2015.3

Users who wish to compile ChakraCore can easily do so with access to VS2015 MSVC toolset for free from the Microsoft Visual C++ Build Tools 2015

C++ 11/14/17 compiler support reference table

@dilijev
Copy link
Contributor

dilijev commented Feb 17, 2017

I definitely support this.

FWIW, very little work is required on our part to follow this proposal. We already mainly use VS 2015 (dev14). We simply drop the dev12 part of the legacy configuration and switch to dev14 on Windows 7 for Legacy build & test. AFAIK, dev14 is already configured on the Win 7 CI machines. @mmitche can confirm.

Other things this would enable, although none are necessary to go forward with this:

  • Dropping the _u macro for all unicode strings and use unicode string literals as supported by the standard for char16_t, i.e. u"..." instead of _u("...").
  • Using constexpr in places where it may be helpful.

I'm sure there's more, but that's just off the top of my head.

@Cellule
Copy link
Contributor

Cellule commented Feb 17, 2017

Same, I definitely support this!
More features and less work. From what I understand, the reason we kept it was to support some customers that still supported VS 2013, which from what you mentioned is not needed anymore.

@dilijev
Copy link
Contributor

dilijev commented Feb 17, 2017

I opened a PR to master-ci to remove dev12 support from legacy (#2552) and a PR to update the Wiki (microsoft/ChakraCore-wiki#25). We can wait to merge them until there's consensus on this issue.

@obastemur
Copy link
Collaborator

I also support this but I would go for this after GCC support. Our Clang requirement (3.7+) is a pain on some of the distros and raising the compiler bar even more is something I would like to avoid.

@dilijev
Copy link
Contributor

dilijev commented Feb 21, 2017

Is the default GCC in most distros not C++11 compliant?

@obastemur
Copy link
Collaborator

Is the default GCC in most distros not C++11 compliant?

It's not strictly about C++11 compliance (that is another issue though..)

It's more about introducing some new things and ending up with compiler bar is raised up even more.

@dilijev
Copy link
Contributor

dilijev commented Feb 24, 2017

So basically you're saying that for now the VS 2013 support keeps us from raising the feature bar higher. Your concern is if we drop the support requirement, we could potentially move to using features that are supported in VS 2015 and Clang but not default versions of other toolchains like GCC?

How far away is GCC support?

@obastemur
Copy link
Collaborator

So basically you're saying that for now the VS 2013 support keeps us from raising the feature bar higher.

What I'm saying is, better don't change our codebase dramatically until ChakraCore compiles with GCC. I'm working on that update only on free time for now (around 30% is complete).. After 2.0 release period, I guess it will be a easier to estimate the process.

Not sure if we expect any tangible gain from dropping VS2013 sooner?

@dilijev
Copy link
Contributor

dilijev commented Mar 10, 2017

@obastemur I agree we should wait until we have a successful build in GCC and CI support for the same.

@obastemur
Copy link
Collaborator

I will not hold this any longer. GCC latest support version has decent amount of C++11 features available. As long as we use stuff from C++11, it won't do any harm on any potential GCC work.

@obastemur
Copy link
Collaborator

/cc @Penguinwizzard

@Penguinwizzard
Copy link
Contributor

I'm definitely in support of this move. The last thing that may be an issue is people who depend on building ChakraCore for Windows 7; I asked on the gitter to see if there's any (no response yet), and I'll leave the PR to make this change open for at least a week in the hopes that anyone who needs it will respond there.

@digitalinfinity
Copy link
Contributor

@matthargett @rozele does React Native for Windows need ChakraCore to compile with VS 2013?

@dilijev
Copy link
Contributor

dilijev commented Sep 28, 2017

@curtisman Do you have any further concerns?

@matthargett
Copy link

no, we only support VS2015.3 and 2017, and will be deprecating 2015 support soon. go for C++14 as far as I’m concerned! ;)

@Penguinwizzard
Copy link
Contributor

PR is open at #3855.

@dilijev
Copy link
Contributor

dilijev commented Oct 17, 2017

At this point, are we keeping this issue open to track additional work items?

@Penguinwizzard
Copy link
Contributor

Nope.

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

8 participants