Does this need a whole new CI job? I know I suggested that in the issue, but it occurs to me that this could be integrated into e.g. the spin lint command.

Also rather than just checking diffs we could do this codebase-wide and add e.g. // noqa comments throughout the codebase, since it is safe to use borrowed references in some situations, but I think having a linter that alerts contributors and reviewers about the problem is good.

If it ends up being really invasive to add the lint opt-outs we can reconsider.

If the lint fails, it should suggest what to do to fix the issue. One of:

• use an API that returns a strong reference

or, add an opt-out to the lint if:

• you are very sure that a lock is held
• you are very sure you have the only reference to the object (e.g. it is newly created in the local scope).

The linter should also probably check all of the functions listed in the free-threaded HOWTO in the CPython docs:

https://docs.python.org/3/howto/free-threading-extensions.html#borrowed-references

Also see here for more details about why we need to do this and why we can't just globally replace all these functions:

https://py-free-threading.github.io/porting-extensions/#cpython-c-api-usage

We probably should also be linting uses of fast accessor macros that don't do any locking.

Also, FWIW, there are probably still lurking issues. I'm aware of Py_SEQUENCE_FAST at least (#28046), I've just had more pressing things to work on and haven't been able to finish that one off. I'm hoping to finish off that work soon, or at least upstream the trivial fixes I have but leave an issue open for the more complicated issues in the array coercion code.

I almost wonder if one could do a custom clang-tidy style check (not if that can that work with .c.src though, I admit)? That might also be useful for other projects then.

Plus that means we have an established pattern for allowing it, just add a // NOLINT: Newly created tuple comment. I think an inline code-comment is vastly preferential to some global allowlist.

This is looking much closer to being mergeable. Let me know if you need a hand with getting the circleci build fixed.

.. or maybe it's unrelated and would go away with a rebase?

@ngoldbaum

1. I think the rebase worked, kind of.
2. The test is set to fail if any of the functions are used but currently I think there's about 92 uses of the functions, here's a link: C API Borrowed Ref Lint
3. Should I assume that all the currently flagged uses of the functions are approved uses? I.e., should I just mark each listed instance as ok // noqa: borrowed-ref OK
4. I think a failing version of this test would cause 4 tests to fail, azure-pipeline numpy, azure-pipeline numpy (ComprehensiveTest Lint), Linux, and Run MyPy/C API Borrowed Ref Lint. I think it's because those jobs run linter.py, is that ok if it functions that way?

Let me know if you'd like a hand with some git surgery :)

@ngoldbaum looks like all the tests are passing now.

1. Re: numpy/_core/src/common/pythoncapi_compat.h, I think I accidentally added this file into this location. So you were right, this was the source of all the additional lines. I removed this from my PR
2. Re: Linter in tools/ci/check_c_api_usage.sh, I changed the ALL_FILES filter to ignore the submodule section. I tested it in my codespace and here, seems to work ok. The line is now:

ALL_FILES=$(find numpy -type f \( -name "*.c" -o -name "*.h" -o -name "*.c.src" -o -name "*.cpp" \) ! -path "*/pythoncapi-compat/*")

Let me know if you need any additional changes. thanks for the help!

Hi all, I'm really sorry for the delay here. Totally my fault for letting it sit for so long.

I'll try to set aside some time to get this up to shape.