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

Discontinue support for VS2013 #4031

Closed
peterbell10 opened this issue Sep 16, 2017 · 8 comments
Closed

Discontinue support for VS2013 #4031

peterbell10 opened this issue Sep 16, 2017 · 8 comments

Comments

@peterbell10
Copy link
Member

peterbell10 commented Sep 16, 2017

Proposal

Cuberite should discontinue support for Visual Studio 2013 and require at least Visual Studio 2015.

Motivation

In #3856 @madmaxoft says:

We decided on C++11, which is what VS2013 uses, so that's what we stick with for now.

But VS2013 lacks many C++11 features:

  • noexcept
  • constexpr
  • magic statics: i.e. thread safe static initialisation
  • ref-qualifiers: i.e. the ability to overload member functions by value category.
    e.g. Copy() members could move when called on an r-value.
  • attributes: e.g. NORETURN -> [[noreturn]]
  • thread_local: See FastRandom.cpp for workarounds needed today
  • = default for move construction and move-assignment
  • user defined literals: e.g. std::chrono::milliseconds{100} could become 100_ms
  • unicode string literals: e.g. from ChatColor.cpp
    const char * cChatColor::Color = "\xc2\xa7"; // or in other words: "§" in UTF-8
    // Becomes
    const char * cChatColor::Color = u8"§";
    
  • char{n}_t
  • inline namespace
  • inheriting constructors
  • alignof and alignas
  • extended sizeof
  • std::quick_exit

And that's not mentioning the compiler bugs that make some supported C++11 features hard to use.

@peterbell10
Copy link
Member Author

An important note is that most modern C++ libraries require VS2015.
I was looking into implementing NBT tags in cItem (for #3682) and the cleanest way I can think of is to use a variant. However, with the exception of boost, all the libraries I can find require VS2015.

@madmaxoft
Copy link
Member

While I don't want to cling to VS2013 at any cost, I think it's reasonable not to be over-zealous with the new C++11 / C++14 / ... features. The more abstract ones we use, the less compatibility we have with as many systems as possible, the less programmers are familiar with the features and thus the lesser the possible workforce on Cuberite. Also note that some of these features are not properly implemented even in VS2017.
noexcept and constexpr - we do want them. Magic statics are not properly implemented in even 2017 (not my knowledge, my colleague was working with them extensively). thread_local is a poor man's excuse for not doing multithreading properly. = default would be useful. User defined literals are stupid because of the underscore requirement and we're getting rid of most of chrono anyway. Unicode literals have exactly the one use mentioned, I can live without them now. char{n}_t are not needed, we typedef to Int{n} already. The rest I haven't even heard about, so you can see how high the entry knowledge this would make for new-coming C++ devs.

@madmaxoft
Copy link
Member

As for the NBT tag implementation, I think we agreed on storing it in json directly.

@peterbell10
Copy link
Member Author

peterbell10 commented Sep 18, 2017

I'm not trying to say that cuberite needs to be rewritten to use these features, just that they are missing. The more advanced features might only be used by a library writer but library compatibility shouldn't be overlooked.

Magic statics are not properly implemented in even 2017

Odd, the linked MSDN article says magic statics work in VS2015. I was just going by that.

thread_local is a poor man's excuse for not doing multithreading properly

I think minimising shared (mutable) data should be a goal for any multithreaded program and thread_local is a tool that makes that easier.

Also, could you point me to the NBT discussion?
Edit: https://forum.cuberite.org/thread-2626.html

@madmaxoft
Copy link
Member

The magic statics are implemented in VS2015, but they are broken in subtle ways that interfere with what we do at work. They might work sufficiently for Cuberite, I don't know.

@tigerw
Copy link
Member

tigerw commented Apr 1, 2020

What say you now? It's 2020 baby! We should be on VS2017 at the earliest!

@madmaxoft
Copy link
Member

I'm not using VS2013 anymore; searching for VS2013 gives only 3 results in this repo, all of them in documentation, rather than code. Appveyor CI uses VS2015 (because their VS2017 images are much slower than VS2015 images). So I'd say it's safe to switch to VS2015. If you can make Appveyor build faster with VS2017, we could even switch there.

@peterbell10
Copy link
Member Author

#4716

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