[Draft] Full removal of nall::vector#2200
[Draft] Full removal of nall::vector#2200rasky wants to merge 18 commits intoares-emulator:masterfrom
Conversation
rasky
force-pushed
the
nall7
branch
14 times, most recently
from
7fbf2dc to
c37d072
Compare
There was a problem hiding this comment.
Pull Request Overview
This draft PR demonstrates the complete removal of nall::vector, replacing it with std::vector throughout the codebase. This represents a significant modernization effort to use standard library containers instead of custom implementations.
- Complete replacement of nall::vector with std::vector across all modules
- Updating method calls from nall-specific APIs to standard library equivalents
- Adding necessary vector-helpers.hpp to provide compatibility functions
- Removing all nall::vector implementation files and dependencies
Reviewed Changes
Copilot reviewed 290 out of 290 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| nall/nall/vector-helpers.hpp | New helper functions to replace vector-specific operations like split/merge |
| tools/mame2bml/mame2bml.cpp | Updated vector usage and method calls |
| tools/genius/genius.hpp | Changed vector types to std::vector |
| nall/nall/vector.hpp | Removed entire nall::vector implementation |
| mia/mia.cpp | Updated media list to use std::vector |
| desktop-ui/emulator/emulator.hpp | Changed emulator collections to std::vector |
| ares/*/system/system.cpp | Updated enumerate functions to return std::vector |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
tools/genius/genius.cpp
Outdated
There was a problem hiding this comment.
The ternary operator creates an unnecessary temporary string object. Consider using an empty string literal instead: modified ? "*" : \"\"
| setTitle({modified ? "*" : string{}, name, " [", games.size(), "] - genius"}); | |
| setTitle({modified ? "*" : "", name, " [", games.size(), "] - genius"}); |
nall/nall/windows/registry.cpp
Outdated
There was a problem hiding this comment.
This pattern of extracting front/back elements and erasing them appears multiple times. Consider creating helper functions like take_front() and take_back() to reduce code duplication.
tests/m68000/m68000.cpp
Outdated
There was a problem hiding this comment.
The lambda captures by reference but creates temporary string objects. Consider using emplace_back instead of push_back to construct the string in place: errors.emplace_back(std::forward<decltype(p)>(p)...);
nall/nall/string/eval/node.hpp
Outdated
There was a problem hiding this comment.
The while loop repeatedly calls push_back which may cause multiple reallocations. Consider using resize for better performance: if(index >= link.size()) link.resize(index + 1, nullptr);
| auto set_link(u32 index, Node* node) -> void { | |
| while(index >= link.size()) link.push_back(nullptr); | |
| if(index >= link.size()) link.resize(index + 1, nullptr); |
rasky
force-pushed
the
nall7
branch
2 times, most recently
from
6e0e056 to
70fd1ee
Compare
genius can't build with Qt as it uses ComboEdit, but it is currently enabled in the ubuntu-ci-qt as it inherits the base ubuntu-ci (which is GTK-based).
- Replace all nall::vector declarations with std::vector in desktop-ui - Update API calls: .append() -> .push_back(), .reset() -> .clear() - Replace .takeFirst()/.takeLast() with .front()/.back() + .erase()/.pop_back() - Convert .find() to std::ranges::find() and std::ranges::find_if() - Replace .sort() with std::ranges::sort() - Fix boolean conversions: if(!vector) -> if(vector.empty()) - Migrate all emulator-specific files and core desktop-ui components - Use direct assignments between std::vector instead of manual loops - Maintain compatibility with nall::split() by using auto type deduction
|
Superseded by #2218 |
This draft PR shows the whole road to nall::vector removal. It will be submitted as smaller PRs one at a time until we reach the end, so this just shows the final version of the series.