Skip to content
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

Multiple vec fixes that are useful to implement canvas_ity. #11

Merged
merged 1 commit into from
Oct 7, 2024

Conversation

p0nce
Copy link
Contributor

@p0nce p0nce commented Oct 4, 2024

I found quite a bit of oddities in vector still.

  • Remove bizzarre idata() method, normally inout should not contaminate identifiers. Fixed that in strings, I supposed the goal was to not return mutable C string from numem string

  • vec.ptr() as alias to vec.data(), which is now inout. It fixed ref auto opOpAssign(string op = "~")(vector!T other) that uses other.ptr

  • vec.length as alias of vec.size

  • compiler suggested that vec.opDollar be const or inout

  • clear() and remove() will clean-up items in reverse manner just like the destructor, and only if not a basic type.

  • ref inout(T) opIndex. else you can't index vector fields in const methods.

  • BREAKING vec.remove() is fixed (it was not possible to remove zero items with its API without crashing) and is now "delete [start, end)" instead of "delete [start, end]". Changed error recovery that swaps start and end to assertion. That makes the functions more similar in contract to c++ std::vector::erase() and a bit more expected to me. IF the contract doesn't change, at least it should say plainly that this will crash if no items are removed.

@LunaTheFoxgirl
Copy link
Member

One problem with this PR, nogc_delete should only be called if ownsMemory is set at compile time (so that weak_vector doesn't free when it's not supposed to)

…uite a bit of oddity in vector still.

- Remove bizzarre idata() method, normally inout should not contaminate identifiers. Fixed that in strings, I supposed the goal was to not return mutable C string from numem string
- vec.ptr() as alias to vec.data(), which is not inout. It fixed ref auto opOpAssign(string op = "~")(vector!T other) that uses other.ptr
- vec.length as alias of vec.size
- clear() and remove() will clean-up items in reverse manner just like the destructor, and only if not a basic type.
- ref inout(T) opIndex. else you can't index vector fields in const methods

- BREAKING remove is fixed (it was not possible to remove zero items with its API without crashing) and is now "delete [start, end)" instead of "delete [start, end]"
  Changed error recovery that swaps start and end to assertion.
  That makes the functions more similar in contract to c++ std::vector::erase and a bit more expected to me.
  IF the contract doesn't change, at least it should say plainly that this will crash if no items are removed.
@p0nce
Copy link
Contributor Author

p0nce commented Oct 6, 2024

Rebased and fixed that to account for yesterday changes.
EDIT: let me know what you agree or disagree on :)

@LunaTheFoxgirl LunaTheFoxgirl merged commit f184b12 into Inochi2D:main Oct 7, 2024
0 of 5 checks passed
@LunaTheFoxgirl
Copy link
Member

LGTM, merged

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.

2 participants