[PATCH] D56429: fix python3 compability issue
jstasiak marked an inline comment as done. jstasiak added inline comments. Comment at: bindings/python/clang/cindex.py:3001 +contents = b(contents) +unsaved_files_array[i].name = b(fspath(name)) +unsaved_files_array[i].contents = contents Nitpicking: the description only mentions changes related to file contents but this modification (`fspath(name)` -> `b(fspath(name))`) likely fixes a different issue, it may be worth mentioning this in the commit message. Unless this extra `b()` call is not strictly necessary here but added for consistency? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56429/new/ https://reviews.llvm.org/D56429 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D56341: [python] Make the collections import future-proof
jstasiak added a comment. Thanks. If you could commit this on my behalf I'd appreciate it (I don't have commit access). Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56341/new/ https://reviews.llvm.org/D56341 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D56341: [python] Make the collections import future-proof
jstasiak created this revision. jstasiak added reviewers: mgorny, michaelplatings, serge-sans-paille. Herald added subscribers: cfe-commits, arphaman. On Python 3.7 the old code raises a warning: DeprecationWarning: Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated, and in 3.8 it will stop working class ArgumentsIterator(collections.Sequence): On Python 3.8 it wouldn't work anymore. Repository: rC Clang https://reviews.llvm.org/D56341 Files: bindings/python/clang/cindex.py Index: bindings/python/clang/cindex.py === --- bindings/python/clang/cindex.py +++ bindings/python/clang/cindex.py @@ -64,7 +64,6 @@ # o implement additional SourceLocation, SourceRange, and File methods. from ctypes import * -import collections import clang.enumerations @@ -123,6 +122,14 @@ def b(x): return x +# Importing ABC-s directly from collections is deprecated since Python 3.7, +# will stop working in Python 3.8. +# See: https://docs.python.org/dev/whatsnew/3.7.html#id3 +if sys.version_info[:2] >= (3, 7): +from collections import abc as collections_abc +else: +import collections as collections_abc + # We only support PathLike objects on Python version with os.fspath present # to be consistent with the Python standard library. On older Python versions # we only support strings and we have dummy fspath to just pass them through. @@ -2181,7 +2188,7 @@ The returned object is iterable and indexable. Each item in the container is a Type instance. """ -class ArgumentsIterator(collections.Sequence): +class ArgumentsIterator(collections_abc.Sequence): def __init__(self, parent): self.parent = parent self.length = None Index: bindings/python/clang/cindex.py === --- bindings/python/clang/cindex.py +++ bindings/python/clang/cindex.py @@ -64,7 +64,6 @@ # o implement additional SourceLocation, SourceRange, and File methods. from ctypes import * -import collections import clang.enumerations @@ -123,6 +122,14 @@ def b(x): return x +# Importing ABC-s directly from collections is deprecated since Python 3.7, +# will stop working in Python 3.8. +# See: https://docs.python.org/dev/whatsnew/3.7.html#id3 +if sys.version_info[:2] >= (3, 7): +from collections import abc as collections_abc +else: +import collections as collections_abc + # We only support PathLike objects on Python version with os.fspath present # to be consistent with the Python standard library. On older Python versions # we only support strings and we have dummy fspath to just pass them through. @@ -2181,7 +2188,7 @@ The returned object is iterable and indexable. Each item in the container is a Type instance. """ -class ArgumentsIterator(collections.Sequence): +class ArgumentsIterator(collections_abc.Sequence): def __init__(self, parent): self.parent = parent self.length = None ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54120: [python] Support PathLike filenames and directories
jstasiak updated this revision to Diff 173505.
jstasiak added a comment.
That's fair, changed string to just x, should be obvious from the context what
x is.
Thank you for the review. As I don't have commit access I'd like to ask you to
commit this on my behalf.
https://reviews.llvm.org/D54120
Files:
bindings/python/clang/cindex.py
bindings/python/tests/cindex/test_cdb.py
bindings/python/tests/cindex/test_code_completion.py
bindings/python/tests/cindex/test_translation_unit.py
bindings/python/tests/cindex/util.py
Index: bindings/python/tests/cindex/util.py
===
--- bindings/python/tests/cindex/util.py
+++ bindings/python/tests/cindex/util.py
@@ -1,5 +1,15 @@
# This file provides common utility functions for the test suite.
+import os
+HAS_FSPATH = hasattr(os, 'fspath')
+
+if HAS_FSPATH:
+from pathlib import Path as str_to_path
+else:
+str_to_path = None
+
+import unittest
+
from clang.cindex import Cursor
from clang.cindex import TranslationUnit
@@ -68,8 +78,13 @@
return cursors
+skip_if_no_fspath = unittest.skipUnless(HAS_FSPATH,
+"Requires file system path protocol / Python 3.6+")
+
__all__ = [
'get_cursor',
'get_cursors',
'get_tu',
+'skip_if_no_fspath',
+'str_to_path',
]
Index: bindings/python/tests/cindex/test_translation_unit.py
===
--- bindings/python/tests/cindex/test_translation_unit.py
+++ bindings/python/tests/cindex/test_translation_unit.py
@@ -20,6 +20,8 @@
from clang.cindex import TranslationUnit
from .util import get_cursor
from .util import get_tu
+from .util import skip_if_no_fspath
+from .util import str_to_path
kInputsDir = os.path.join(os.path.dirname(__file__), 'INPUTS')
@@ -36,6 +38,17 @@
yield t.name
+@contextmanager
+def save_tu_pathlike(tu):
+"""Convenience API to save a TranslationUnit to a file.
+
+Returns the filename it was saved to.
+"""
+with tempfile.NamedTemporaryFile() as t:
+tu.save(str_to_path(t.name))
+yield t.name
+
+
class TestTranslationUnit(unittest.TestCase):
def test_spelling(self):
path = os.path.join(kInputsDir, 'hello.cpp')
@@ -89,6 +102,22 @@
spellings = [c.spelling for c in tu.cursor.get_children()]
self.assertEqual(spellings[-1], 'x')
+@skip_if_no_fspath
+def test_from_source_accepts_pathlike(self):
+tu = TranslationUnit.from_source(str_to_path('fake.c'), ['-Iincludes'], unsaved_files = [
+(str_to_path('fake.c'), """
+#include "fake.h"
+int x;
+int SOME_DEFINE;
+"""),
+(str_to_path('includes/fake.h'), """
+#define SOME_DEFINE y
+""")
+])
+spellings = [c.spelling for c in tu.cursor.get_children()]
+self.assertEqual(spellings[-2], 'x')
+self.assertEqual(spellings[-1], 'y')
+
def assert_normpaths_equal(self, path1, path2):
""" Compares two paths for equality after normalizing them with
os.path.normpath
@@ -135,6 +164,16 @@
self.assertTrue(os.path.exists(path))
self.assertGreater(os.path.getsize(path), 0)
+@skip_if_no_fspath
+def test_save_pathlike(self):
+"""Ensure TranslationUnit.save() works with PathLike filename."""
+
+tu = get_tu('int foo();')
+
+with save_tu_pathlike(tu) as path:
+self.assertTrue(os.path.exists(path))
+self.assertGreater(os.path.getsize(path), 0)
+
def test_save_translation_errors(self):
"""Ensure that saving to an invalid directory raises."""
@@ -167,6 +206,22 @@
# Just in case there is an open file descriptor somewhere.
del tu2
+@skip_if_no_fspath
+def test_load_pathlike(self):
+"""Ensure TranslationUnits can be constructed from saved files -
+PathLike variant."""
+tu = get_tu('int foo();')
+self.assertEqual(len(tu.diagnostics), 0)
+with save_tu(tu) as path:
+tu2 = TranslationUnit.from_ast_file(filename=str_to_path(path))
+self.assertEqual(len(tu2.diagnostics), 0)
+
+foo = get_cursor(tu2, 'foo')
+self.assertIsNotNone(foo)
+
+# Just in case there is an open file descriptor somewhere.
+del tu2
+
def test_index_parse(self):
path = os.path.join(kInputsDir, 'hello.cpp')
index = Index.create()
@@ -185,6 +240,19 @@
with self.assertRaises(Exception):
f = tu.get_file('foobar.cpp')
+@skip_if_no_fspath
+def test_get_file_pathlike(self):
+"""Ensure tu.get_file() works appropriately with PathLike filenames."""
+
+tu = get_tu('int foo();')
+
+f = tu.get_file(str_to_path('t.c'))
+self.assertIsInstance(f, File)
+self.assertEqual(f.name, 't.c')
+
+with
[PATCH] D54120: [python] Support PathLike filenames and directories
jstasiak marked 2 inline comments as done. jstasiak added a comment. Thanks for the feedback, you're right those were things worth improving, I updated the code. https://reviews.llvm.org/D54120 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54120: [python] Support PathLike filenames and directories
jstasiak updated this revision to Diff 173496.
jstasiak added a comment.
Tests are skipped using unittest skipping mechanism now and pathlib imported
only when necessary.
https://reviews.llvm.org/D54120
Files:
bindings/python/clang/cindex.py
bindings/python/tests/cindex/test_cdb.py
bindings/python/tests/cindex/test_code_completion.py
bindings/python/tests/cindex/test_translation_unit.py
bindings/python/tests/cindex/util.py
Index: bindings/python/tests/cindex/util.py
===
--- bindings/python/tests/cindex/util.py
+++ bindings/python/tests/cindex/util.py
@@ -1,5 +1,15 @@
# This file provides common utility functions for the test suite.
+import os
+HAS_FSPATH = hasattr(os, 'fspath')
+
+if HAS_FSPATH:
+from pathlib import Path as str_to_path
+else:
+str_to_path = None
+
+import unittest
+
from clang.cindex import Cursor
from clang.cindex import TranslationUnit
@@ -68,8 +78,13 @@
return cursors
+skip_if_no_fspath = unittest.skipUnless(HAS_FSPATH,
+"Requires file system path protocol / Python 3.6+")
+
__all__ = [
'get_cursor',
'get_cursors',
'get_tu',
+'skip_if_no_fspath',
+'str_to_path',
]
Index: bindings/python/tests/cindex/test_translation_unit.py
===
--- bindings/python/tests/cindex/test_translation_unit.py
+++ bindings/python/tests/cindex/test_translation_unit.py
@@ -20,6 +20,8 @@
from clang.cindex import TranslationUnit
from .util import get_cursor
from .util import get_tu
+from .util import skip_if_no_fspath
+from .util import str_to_path
kInputsDir = os.path.join(os.path.dirname(__file__), 'INPUTS')
@@ -36,6 +38,17 @@
yield t.name
+@contextmanager
+def save_tu_pathlike(tu):
+"""Convenience API to save a TranslationUnit to a file.
+
+Returns the filename it was saved to.
+"""
+with tempfile.NamedTemporaryFile() as t:
+tu.save(str_to_path(t.name))
+yield t.name
+
+
class TestTranslationUnit(unittest.TestCase):
def test_spelling(self):
path = os.path.join(kInputsDir, 'hello.cpp')
@@ -89,6 +102,22 @@
spellings = [c.spelling for c in tu.cursor.get_children()]
self.assertEqual(spellings[-1], 'x')
+@skip_if_no_fspath
+def test_from_source_accepts_pathlike(self):
+tu = TranslationUnit.from_source(str_to_path('fake.c'), ['-Iincludes'], unsaved_files = [
+(str_to_path('fake.c'), """
+#include "fake.h"
+int x;
+int SOME_DEFINE;
+"""),
+(str_to_path('includes/fake.h'), """
+#define SOME_DEFINE y
+""")
+])
+spellings = [c.spelling for c in tu.cursor.get_children()]
+self.assertEqual(spellings[-2], 'x')
+self.assertEqual(spellings[-1], 'y')
+
def assert_normpaths_equal(self, path1, path2):
""" Compares two paths for equality after normalizing them with
os.path.normpath
@@ -135,6 +164,16 @@
self.assertTrue(os.path.exists(path))
self.assertGreater(os.path.getsize(path), 0)
+@skip_if_no_fspath
+def test_save_pathlike(self):
+"""Ensure TranslationUnit.save() works with PathLike filename."""
+
+tu = get_tu('int foo();')
+
+with save_tu_pathlike(tu) as path:
+self.assertTrue(os.path.exists(path))
+self.assertGreater(os.path.getsize(path), 0)
+
def test_save_translation_errors(self):
"""Ensure that saving to an invalid directory raises."""
@@ -167,6 +206,22 @@
# Just in case there is an open file descriptor somewhere.
del tu2
+@skip_if_no_fspath
+def test_load_pathlike(self):
+"""Ensure TranslationUnits can be constructed from saved files -
+PathLike variant."""
+tu = get_tu('int foo();')
+self.assertEqual(len(tu.diagnostics), 0)
+with save_tu(tu) as path:
+tu2 = TranslationUnit.from_ast_file(filename=str_to_path(path))
+self.assertEqual(len(tu2.diagnostics), 0)
+
+foo = get_cursor(tu2, 'foo')
+self.assertIsNotNone(foo)
+
+# Just in case there is an open file descriptor somewhere.
+del tu2
+
def test_index_parse(self):
path = os.path.join(kInputsDir, 'hello.cpp')
index = Index.create()
@@ -185,6 +240,19 @@
with self.assertRaises(Exception):
f = tu.get_file('foobar.cpp')
+@skip_if_no_fspath
+def test_get_file_pathlike(self):
+"""Ensure tu.get_file() works appropriately with PathLike filenames."""
+
+tu = get_tu('int foo();')
+
+f = tu.get_file(str_to_path('t.c'))
+self.assertIsInstance(f, File)
+self.assertEqual(f.name, 't.c')
+
+with self.assertRaises(Exception):
+f = tu.get_file(str_to_path('foobar.cpp'))
+
def
[PATCH] D54120: [python] Support PathLike filenames and directories
jstasiak created this revision.
jstasiak added reviewers: mgorny, jbcoe.
Herald added a subscriber: arphaman.
Python 3.6 introduced a file system path protocol (PEP 519[1]). The standard
library APIs accepting file system paths now accept path objects too. It could
be useful to add this here as well for convenience.
[1] https://www.python.org/dev/peps/pep-0519
Repository:
rC Clang
https://reviews.llvm.org/D54120
Files:
bindings/python/clang/cindex.py
bindings/python/tests/cindex/test_cdb.py
bindings/python/tests/cindex/test_code_completion.py
bindings/python/tests/cindex/test_translation_unit.py
bindings/python/tests/cindex/util.py
Index: bindings/python/tests/cindex/util.py
===
--- bindings/python/tests/cindex/util.py
+++ bindings/python/tests/cindex/util.py
@@ -1,5 +1,15 @@
# This file provides common utility functions for the test suite.
+import os
+HAS_FSPATH = hasattr(os, 'fspath')
+
+try:
+import pathlib
+except ImportError:
+str_to_path = None
+else:
+str_to_path = pathlib.Path
+
from clang.cindex import Cursor
from clang.cindex import TranslationUnit
@@ -72,4 +82,6 @@
'get_cursor',
'get_cursors',
'get_tu',
+'HAS_FSPATH',
+'str_to_path',
]
Index: bindings/python/tests/cindex/test_translation_unit.py
===
--- bindings/python/tests/cindex/test_translation_unit.py
+++ bindings/python/tests/cindex/test_translation_unit.py
@@ -20,6 +20,8 @@
from clang.cindex import TranslationUnit
from .util import get_cursor
from .util import get_tu
+from .util import HAS_FSPATH
+from .util import str_to_path
kInputsDir = os.path.join(os.path.dirname(__file__), 'INPUTS')
@@ -36,6 +38,17 @@
yield t.name
+@contextmanager
+def save_tu_pathlike(tu):
+"""Convenience API to save a TranslationUnit to a file.
+
+Returns the filename it was saved to.
+"""
+with tempfile.NamedTemporaryFile() as t:
+tu.save(str_to_path(t.name))
+yield t.name
+
+
class TestTranslationUnit(unittest.TestCase):
def test_spelling(self):
path = os.path.join(kInputsDir, 'hello.cpp')
@@ -89,6 +102,22 @@
spellings = [c.spelling for c in tu.cursor.get_children()]
self.assertEqual(spellings[-1], 'x')
+if HAS_FSPATH:
+def test_from_source_accepts_pathlike(self):
+tu = TranslationUnit.from_source(str_to_path('fake.c'), ['-Iincludes'], unsaved_files = [
+(str_to_path('fake.c'), """
+#include "fake.h"
+int x;
+int SOME_DEFINE;
+"""),
+(str_to_path('includes/fake.h'), """
+#define SOME_DEFINE y
+""")
+])
+spellings = [c.spelling for c in tu.cursor.get_children()]
+self.assertEqual(spellings[-2], 'x')
+self.assertEqual(spellings[-1], 'y')
+
def assert_normpaths_equal(self, path1, path2):
""" Compares two paths for equality after normalizing them with
os.path.normpath
@@ -135,6 +164,16 @@
self.assertTrue(os.path.exists(path))
self.assertGreater(os.path.getsize(path), 0)
+if HAS_FSPATH:
+def test_save_pathlike(self):
+"""Ensure TranslationUnit.save() works with PathLike filename."""
+
+tu = get_tu('int foo();')
+
+with save_tu_pathlike(tu) as path:
+self.assertTrue(os.path.exists(path))
+self.assertGreater(os.path.getsize(path), 0)
+
def test_save_translation_errors(self):
"""Ensure that saving to an invalid directory raises."""
@@ -167,6 +206,17 @@
# Just in case there is an open file descriptor somewhere.
del tu2
+# We can also pass the filename as Path-like object
+if HAS_FSPATH:
+tu2 = TranslationUnit.from_ast_file(filename=str_to_path(path))
+self.assertEqual(len(tu2.diagnostics), 0)
+
+foo = get_cursor(tu2, 'foo')
+self.assertIsNotNone(foo)
+
+# Just in case there is an open file descriptor somewhere.
+del tu2
+
def test_index_parse(self):
path = os.path.join(kInputsDir, 'hello.cpp')
index = Index.create()
@@ -185,6 +235,19 @@
with self.assertRaises(Exception):
f = tu.get_file('foobar.cpp')
+if HAS_FSPATH:
+def test_get_file_pathlike(self):
+"""Ensure tu.get_file() works appropriately with PathLike filenames."""
+
+tu = get_tu('int foo();')
+
+f = tu.get_file(str_to_path('t.c'))
+self.assertIsInstance(f, File)
+self.assertEqual(f.name, 't.c')
+
+with self.assertRaises(Exception):
+f = tu.get_file(str_to_path('foobar.cpp'))
+
def test_get_source_location(self):
"""Ensure
