Skip to content

Update atomlist#270

Merged
MatthieuDartiailh merged 1 commit intonucleic:mainfrom
frmdstryr:update-atomlist
May 8, 2026
Merged

Update atomlist#270
MatthieuDartiailh merged 1 commit intonucleic:mainfrom
frmdstryr:update-atomlist

Conversation

@frmdstryr
Copy link
Copy Markdown
Contributor

This fixes a leak in validate_sequence, updates insert & pop to use vectorcalls, and cleans up some outdated version guards.

Since full_validate already returns a new reference it does not need an extra incref, it just gets stolen by the list. PySequence_List returns a populated copy of the items. PyList_SET_ITEM is intended to be used with PyList_New so it does not discard the old item meaning the old item was leaked.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 8, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.52%. Comparing base (e78a716) to head (c6155d7).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #270   +/-   ##
=======================================
  Coverage   97.52%   97.52%           
=======================================
  Files          24       24           
  Lines        1093     1093           
  Branches      167      167           
=======================================
  Hits         1066     1066           
  Misses         13       13           
  Partials       14       14           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread atom/src/atomlist.cpp Outdated
return 0;
PyList_SET_ITEM( templist.get(), i, cppy::incref( val ) );
PyList_SET_ITEM( templist.get(), i, val );
Py_DECREF( b ); // Release old reference
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PyList_GET_ITEM return a borrowed reference but PyList_SET_ITEM does not decrement the ref count of the replaced object. So this is correct but really deserves a comment.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Added a comment there to clarify.

@MatthieuDartiailh MatthieuDartiailh merged commit b2b0595 into nucleic:main May 8, 2026
20 of 21 checks passed
@frmdstryr
Copy link
Copy Markdown
Contributor Author

Thanks!

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