Fix #9386: use variant instead of unique_ptr to prevent compilers failing on the code generation #9391
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Don't forget to update |
Rename the zero-parameter NetworkValidateClientName to NetworkValidateOurClientName to make it clearer it is performed on our client name, and to make it a non-overloaded function to aid with the variant being added a few commits later
One UpdateServiceInterval has two parameters to update the service interval for a vehicle type, the other for all vehicle types at once. Rename the latter to help with function resolution for the introduction of variants.
… the saveload macros This to help variant's constructor to be able to resolve the constructor of the setting
… actual SettingDesc
…gDesc constructor This as using std::string causes much more variables to be tracked, potentially causing problemes for certain compilers in certain situations
…ants instead of new + unique_ptr With std::variant all memory can be figured out at compile time, so the compiler needs to keep track of fewer elements. It also saves out a unique_ptr and its memory management, over a slight impact for resolving a setting.
|
Reminder we shouldn't forget to update https://github.com/OpenTTD/website/blob/master/_data/download-descriptions.yml if this lands. |
TrueBrain
approved these changes
Jun 26, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Motivation / Problem
In the words of TB:
Description
Fixes #9386 by using
std::variantto forego the need to generate code for anewand as such needing to bother withdeleteat a later stage.The forwarding constructor of
std::variantis quite picky (at least on MSVC). It does not like to resolve overloaded functions, so some functions needed to be renamed to be resolvable. Furthermore the 'naked' initializer forSaveLoadwas causing issues at some places, so effectively 'name' it by addingSaveLoadto the SLE* macros.Finally resolving from a variant to a type is not super trivial when you do not know the exact type, for this
std::visitis used which helps but including that in a simpleiteratoris beyond my C++-foo, so fell back to a helper function to convert the type of the 'auto loop' to the needed/wantedconst SettingDesc *.Limitations
std::visitrequires Mac OS 10.14 or higher which has been out-of-support for half a year... and that causes deprecations warnings that I can't solve.Checklist for review
Some things are not automated, and forgotten often. This list is a reminder for the reviewers.