Skip to content

Conversation

@jackpoz
Copy link
Member

@jackpoz jackpoz commented Oct 13, 2019

Changes proposed:

  • Build: Enable and require c++17

Target branch(es): 3.3.5/master

  • 3.3.5
  • master

Issues addressed: None

Tests performed: (Does it build, tested in-game, etc.)

  • Built, logged ingame

Known issues and TODO list: (add/remove lines as needed)

  • VS, GCC and Clang versions should be increased to whichever version has a working C++17 with no big known issues

@Aokromes
Copy link
Member

Conflicting files
dep/argon2/CMakeLists.txt

@jackpoz
Copy link
Member Author

jackpoz commented Dec 28, 2019

Cleaned up the PR with only the changes required for C++17 after some CMake bits have been merged to 3.3.5 already

@jackpoz
Copy link
Member Author

jackpoz commented Feb 4, 2020

anyone against merging this PR ?

@ghost
Copy link

ghost commented Feb 4, 2020

Maybe replace the label [Feedback-FixOutdatedMissingWIP] with [Feedback-PatchFix] before merging, just to signify its current state.

Otherwise, this PR has been available for quite a while, so if anybody with an opinion has not reviewed or tested this yet, they are unlikely to respond before this gets merged anyway.

@jackpoz
Copy link
Member Author

jackpoz commented Feb 6, 2020

This PR is planned to be merged this weekend. Speak before that if you don't want that (and maybe provide some valid reasons about it)

@Lumber-Jack-2
Copy link
Contributor

so compiling with Microsoft Visual Studio/2017/Community/VC will still all wotk smooth?

@Aokromes
Copy link
Member

Aokromes commented Feb 6, 2020

so compiling with Microsoft Visual Studio/2017/Community/VC will still all wotk smooth?

https://community.trinitycore.org/topic/14655-gcc-63-visual-studio-2017-156-end-of-life/

@Lumber-Jack-2
Copy link
Contributor

aok wrote: Since we are targeting C++17 compilers, GCC 6.3, clang 4 and Visual Studio 2017 under version 15.7 will become unsupported after summer

perfect thanks!

@jackpoz
Copy link
Member Author

jackpoz commented Feb 6, 2020

@Rushor this PR raises VC++ required version to 19.16 which means VS 2017 15.9+ https://docs.microsoft.com/en-us/visualstudio/releasenotes/vs2017-relnotes#-visual-studio-2017-version-1590 as previous VS 2017 versions had C++17 bugs.

I would suggest to update to latest VS 2017 or to latest VS 2019 to get latest fixes from the VS team.

@Lumber-Jack-2
Copy link
Contributor

alright - thank you very much!

@Warpten
Copy link
Contributor

Warpten commented Feb 7, 2020

Nitpick: also set Zc:__cplusplus on Windows to get proper value for the language version (it previously always reported as c++98)

(Yes i know _MSVC_LANG is a thing but that's not standard)

@jackpoz
Copy link
Member Author

jackpoz commented Feb 8, 2020

I would prefer to change that flag in another PR, maybe removing _MSVC_VER checks, and testing it.

@Warpten
Copy link
Contributor

Warpten commented Feb 8, 2020

Aight go (might want to toggle __cplusplus on our code only since deps might rely on C++98 value checks)

@jackpoz jackpoz merged commit 726d5e9 into TrinityCore:3.3.5 Feb 8, 2020
@jackpoz jackpoz deleted the 3.3.5-cpp17 branch February 9, 2020 11:26
jackpoz added a commit to jackpoz/TrinityCore that referenced this pull request Feb 15, 2020
@ghost ghost mentioned this pull request Feb 28, 2020
jackpoz added a commit to jackpoz/TrinityCore that referenced this pull request Jun 2, 2020
Shauren pushed a commit that referenced this pull request Dec 21, 2021
* Build: Enable and require c++17

* Build: Raise Visual Studio version from 2017 15.2 to 2017 15.9

* Build: Raise GCC version from 6.3.0 to 7.1.0

* Reduce branch differences

* Fix build after latest merge

* Cleanup after latest merge

(cherry picked from commit 726d5e9)
idairfguido added a commit to idairfguido/TrinityCore that referenced this pull request Jan 21, 2025
idairfguido pushed a commit to idairfguido/TrinityCore that referenced this pull request Feb 12, 2025
* Build: Enable and require c++17

* Build: Raise Visual Studio version from 2017 15.2 to 2017 15.9

* Build: Raise GCC version from 6.3.0 to 7.1.0

* Reduce branch differences

* Fix build after latest merge

* Cleanup after latest merge

(cherry picked from commit 726d5e9)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants