Skip to content

Conversation

@ratijas
Copy link

@ratijas ratijas commented Feb 7, 2024

This is API and ABI breaking change. It optimizes extra allocations for all four methods where returned value is a slice of their input string.

Fixes #55

…tringView

This is API and ABI breaking change. It optimizes extra allocations for
all four methods where returned value is a slice of their input string.
@ratijas
Copy link
Author

ratijas commented Feb 7, 2024

So, I tried to port KDE/Plasma Discover in couple of files, and found it to be quite pointless. Due to the lack of overloaded API elsewhere, I ended up just appending .toString() allocations all over the place, which kinda defeats the purpose. There were 2-3 hunks of code where it legitimately helped though.

Other than that, these methods should probably be added as overloads in addition to existing ones. Imaging passing rvalue QString to one of such methods, and get a QStringView which points to an already destructed temporary QString object? Lifetimes disaster.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Static methods of Transaction could return QStringView instead of allocating a new QString

1 participant