-
-
Notifications
You must be signed in to change notification settings - Fork 957
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
Remove AutoDeleteSmallVector and AutoFreeSmallVector #7453
Conversation
9f34079
to
804f003
Compare
There's a slight work-around for clang-3.8 included (extra |
d5b5385
to
6a51ce0
Compare
Now with more Also, why the heck is GitHub displaying the commits in a mostly random order? |
Oh good, this was on my TODO list |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use of noexcept is interesting. I've seen varying opinions on how worthwhile it actually is...
AutoDeleteSmallVector<LanguageStrings *> compiled_strings; ///< The compiled strings per language, first must be English/the master language!. | ||
StringList string_names; ///< The names of the compiled strings. | ||
std::vector<std::unique_ptr<LanguageStrings>> raw_strings; ///< The raw strings per language, first must be English/the master language!. | ||
std::vector<std::shared_ptr<LanguageStrings>> compiled_strings; ///< The compiled strings per language, first must be English/the master language!. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how difficult would it be to "fix" the use of shared_ptr here?
or, perhaps the various places that "own" the pointer could be documented?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very easily, it is only GameStrings::cur_language
that points to one of the compiled string entries. We store a raw pointer now and just assume it's never going to be invalidated; still possible.
In pure though, raw pointers are bad and should never be used. Of course, the codebase is far, far, far from that point now. So, philosophical question time: do the "right" thing, or the pragmatic thing?
…ror. The raw_strings vector may not include NULLs as no consumer can deal with it.
…r instead of heap allocated. This reduces memory allocations and heap fragmentation.
… in text layout code.
…ing AutoDeleteSmallVector obsolete. DropDownListItem are strongly managed using std::unique_ptr to ensure leak-free handling. Appropriate use of move-semantics make intent a lot clearer than parameter comments and allows the compiler to generate copy-free code for most situations.
…ngs instead of an AutoFreeSmallVector.
The last use was for storing a list of memory blocks. As the way these lists are accessed is very specific, it is easier to just write an explicit destructor instead of trying to exactly match the behaviour.
Rebased to drop the already applied OSX commit. |
Does the change from returning a |
In the literal sense: No. In the less literal sense: Many current compilers will perform copy elision where possible, which means even the move assignment is likely to be optimized out. |
Remove the last remains of SmallVector. For the replacement,
std::unique_ptr
takes care of the memory management. Use of references and move semantics makes ownership clear and reduces potentials for memory leaks. Current compilers should be able to optimize most of the copy/moves out, resulting in very little runtime overhead.This got a bit bigger than expected and picked up some unrelated fixes I saw when in the area.
Commits 41b6916, 0266dcb, and b27d0af could be split out if needed.