-
Notifications
You must be signed in to change notification settings - Fork 14.9k
release/21.x: [libclang/python] Return None instead of null cursors from Token.cursor (#163183) #163961
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
Conversation
From #163183 |
@llvm/pr-subscribers-clang Author: Jannick Kremer (DeinAlptraum) ChangesBackport 38a5282 Full diff: https://github.com/llvm/llvm-project/pull/163961.diff 3 Files Affected:
diff --git a/clang/bindings/python/clang/cindex.py b/clang/bindings/python/clang/cindex.py
index 824674309d262..5ce7b5781bcb4 100644
--- a/clang/bindings/python/clang/cindex.py
+++ b/clang/bindings/python/clang/cindex.py
@@ -3853,6 +3853,8 @@ def cursor(self):
cursor._tu = self._tu
conf.lib.clang_annotateTokens(self._tu, byref(self), 1, byref(cursor))
+ if cursor.is_null():
+ return None
return cursor
diff --git a/clang/bindings/python/tests/cindex/test_tokens.py b/clang/bindings/python/tests/cindex/test_tokens.py
index b6c1fc8b83600..6658579c63835 100644
--- a/clang/bindings/python/tests/cindex/test_tokens.py
+++ b/clang/bindings/python/tests/cindex/test_tokens.py
@@ -53,3 +53,9 @@ def test_token_extent(self):
self.assertEqual(extent.start.offset, 4)
self.assertEqual(extent.end.offset, 7)
+
+ def test_null_cursor(self):
+ """Ensure that the cursor property converts null cursors to None"""
+ tu = get_tu("int i = 5;")
+ tokens = list(tu.get_tokens(extent=tu.cursor.extent))
+ self.assertEqual(tokens[-1].cursor, None)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 43529b0f28c3d..364ea632b40cb 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -94,6 +94,7 @@ Clang Frontend Potentially Breaking Changes
Clang Python Bindings Potentially Breaking Changes
--------------------------------------------------
+- Return ``None`` instead of null cursors from ``Token.cursor``
- ``Cursor.from_location`` now returns ``None`` instead of a null cursor.
This eliminates the last known source of null cursors.
- Almost all ``Cursor`` methods now assert that they are called on non-null cursors.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
I get |
No, it's up to the release managers to merge it. |
…or (llvm#163183) Since llvm#138103 , the `Cursor` class throws an error when any of its methods is called on a null cursor. Simultaneously, we adapted all methods to return `None` instead of a null cursor, so users should not encounter these. We have overlooked one way to end up with null cursors, namely the `Token.cursor` property, which may return null cursors under some circumstances. Fixes llvm#163180
841cbb2
to
ceeb930
Compare
@DeinAlptraum (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR. |
Return |
Backport 38a5282