-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[libclang/python] Bump minimum compatibility to Python 3.6 #77228
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
[libclang/python] Bump minimum compatibility to Python 3.6 #77228
Conversation
This simply removes the Python 2 arm of this conditional logic and unindents the Python 3 logic.
In Python3, inheriting from the object class is implicit.
The LLVM minimum python version is 3.6. The os.fspath function is always available in Python 3.6, so we remove the unnecessary fallback. https://docs.python.org/3/library/os.html#os.fspath Issues llvm#76664.
The LLVM minimum python version is 3.6. The abstract base classes were copied from collections to collections.abc in python 3.3. I believe the usage of ABC from the original "collections" module or "collections.abc" module were valid between 3.3 and 3.7, but was eventually dropped in 3.8. So, I believe this method of import has been valid since 3.3. https://docs.python.org/dev/library/collections.abc.html#module-collections.abc Issues llvm#76664.
@llvm/pr-subscribers-clang Author: Craig Hesling (linux4life798) ChangesRemove Python 2 support and clean up code that conditions based on version. Issue #76664. Full diff: https://github.com/llvm/llvm-project/pull/77228.diff 2 Files Affected:
diff --git a/clang/bindings/python/README.txt b/clang/bindings/python/README.txt
index 44c715e5de56f7..3e509662144fac 100644
--- a/clang/bindings/python/README.txt
+++ b/clang/bindings/python/README.txt
@@ -10,7 +10,7 @@ runner. For example:
--
$ env PYTHONPATH=$(echo ~/llvm/clang/bindings/python/) \
CLANG_LIBRARY_PATH=$(llvm-config --libdir) \
- python -m unittest discover -v
+ python3 -m unittest discover -v
tests.cindex.test_index.test_create ... ok
...
diff --git a/clang/bindings/python/clang/cindex.py b/clang/bindings/python/clang/cindex.py
index d780ee353a133c..754f03d718e882 100644
--- a/clang/bindings/python/clang/cindex.py
+++ b/clang/bindings/python/clang/cindex.py
@@ -66,81 +66,50 @@
import clang.enumerations
+import collections.abc
import os
-import sys
-
-if sys.version_info[0] == 3:
- # Python 3 strings are unicode, translate them to/from utf8 for C-interop.
- class c_interop_string(c_char_p):
- def __init__(self, p=None):
- if p is None:
- p = ""
- if isinstance(p, str):
- p = p.encode("utf8")
- super(c_char_p, self).__init__(p)
- def __str__(self):
- return self.value
-
- @property
- def value(self):
- if super(c_char_p, self).value is None:
- return None
- return super(c_char_p, self).value.decode("utf8")
-
- @classmethod
- def from_param(cls, param):
- if isinstance(param, str):
- return cls(param)
- if isinstance(param, bytes):
- return cls(param)
- if param is None:
- # Support passing null to C functions expecting char arrays
- return None
- raise TypeError(
- "Cannot convert '{}' to '{}'".format(type(param).__name__, cls.__name__)
- )
-
- @staticmethod
- def to_python_string(x, *args):
- return x.value
- def b(x):
- if isinstance(x, bytes):
- return x
- return x.encode("utf8")
+# Python 3 strings are unicode, translate them to/from utf8 for C-interop.
+class c_interop_string(c_char_p):
+ def __init__(self, p=None):
+ if p is None:
+ p = ""
+ if isinstance(p, str):
+ p = p.encode("utf8")
+ super(c_char_p, self).__init__(p)
-elif sys.version_info[0] == 2:
- # Python 2 strings are utf8 byte strings, no translation is needed for
- # C-interop.
- c_interop_string = c_char_p
-
- def _to_python_string(x, *args):
- return x
-
- c_interop_string.to_python_string = staticmethod(_to_python_string)
+ def __str__(self):
+ return self.value
- def b(x):
- return x
+ @property
+ def value(self):
+ if super(c_char_p, self).value is None:
+ return None
+ return super(c_char_p, self).value.decode("utf8")
+ @classmethod
+ def from_param(cls, param):
+ if isinstance(param, str):
+ return cls(param)
+ if isinstance(param, bytes):
+ return cls(param)
+ if param is None:
+ # Support passing null to C functions expecting char arrays
+ return None
+ raise TypeError(
+ "Cannot convert '{}' to '{}'".format(type(param).__name__, cls.__name__)
+ )
-# 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
+ @staticmethod
+ def to_python_string(x, *args):
+ return x.value
-# 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.
-try:
- fspath = os.fspath
-except AttributeError:
- def fspath(x):
+def b(x):
+ if isinstance(x, bytes):
return x
+ return x.encode("utf8")
# ctypes doesn't implicitly convert c_void_p to the appropriate wrapper
@@ -202,7 +171,7 @@ def __init__(self, enumeration, message):
### Structures and Utility Classes ###
-class CachedProperty(object):
+class CachedProperty:
"""Decorator that lazy-loads the value of a property.
The first time the property is accessed, the original property function is
@@ -392,7 +361,7 @@ def __repr__(self):
return "<SourceRange start %r, end %r>" % (self.start, self.end)
-class Diagnostic(object):
+class Diagnostic:
"""
A Diagnostic is a single instance of a Clang diagnostic. It includes the
diagnostic severity, the message, the location the diagnostic occurred, as
@@ -433,7 +402,7 @@ def spelling(self):
@property
def ranges(self):
- class RangeIterator(object):
+ class RangeIterator:
def __init__(self, diag):
self.diag = diag
@@ -449,7 +418,7 @@ def __getitem__(self, key):
@property
def fixits(self):
- class FixItIterator(object):
+ class FixItIterator:
def __init__(self, diag):
self.diag = diag
@@ -468,7 +437,7 @@ def __getitem__(self, key):
@property
def children(self):
- class ChildDiagnosticsIterator(object):
+ class ChildDiagnosticsIterator:
def __init__(self, diag):
self.diag_set = conf.lib.clang_getChildDiagnostics(diag)
@@ -532,7 +501,7 @@ def from_param(self):
return self.ptr
-class FixIt(object):
+class FixIt:
"""
A FixIt represents a transformation to be applied to the source to
"fix-it". The fix-it should be applied by replacing the given source range
@@ -547,7 +516,7 @@ def __repr__(self):
return "<FixIt range %r, value %r>" % (self.range, self.value)
-class TokenGroup(object):
+class TokenGroup:
"""Helper class to facilitate token management.
Tokens are allocated from libclang in chunks. They must be disposed of as a
@@ -603,7 +572,7 @@ def get_tokens(tu, extent):
yield token
-class TokenKind(object):
+class TokenKind:
"""Describes a specific type of a Token."""
_value_map = {} # int -> TokenKind
@@ -642,7 +611,7 @@ def register(value, name):
### Cursor Kinds ###
-class BaseEnumeration(object):
+class BaseEnumeration:
"""
Common base class for named enumerations held in sync with Index.h values.
@@ -2059,7 +2028,7 @@ def from_cursor_result(res, fn, args):
return res
-class StorageClass(object):
+class StorageClass:
"""
Describes the storage class of a declaration
"""
@@ -2353,7 +2322,7 @@ def argument_types(self):
container is a Type instance.
"""
- class ArgumentsIterator(collections_abc.Sequence):
+ class ArgumentsIterator(collections.abc.Sequence):
def __init__(self, parent):
self.parent = parent
self.length = None
@@ -2608,7 +2577,7 @@ def __ne__(self, other):
# a void*.
-class ClangObject(object):
+class ClangObject:
"""
A helper for Clang objects. This class helps act as an intermediary for
the ctypes library and the Clang CIndex library.
@@ -2656,8 +2625,8 @@ class _CXUnsavedFile(Structure):
}
-class CompletionChunk(object):
- class Kind(object):
+class CompletionChunk:
+ class Kind:
def __init__(self, name):
self.name = name
@@ -2747,7 +2716,7 @@ def isKindResultType(self):
class CompletionString(ClangObject):
- class Availability(object):
+ class Availability:
def __init__(self, name):
self.name = name
@@ -2849,7 +2818,7 @@ def results(self):
@property
def diagnostics(self):
- class DiagnosticsItr(object):
+ class DiagnosticsItr:
def __init__(self, ccr):
self.ccr = ccr
@@ -3003,13 +2972,13 @@ def from_source(
if hasattr(contents, "read"):
contents = contents.read()
contents = b(contents)
- unsaved_array[i].name = b(fspath(name))
+ unsaved_array[i].name = b(os.fspath(name))
unsaved_array[i].contents = contents
unsaved_array[i].length = len(contents)
ptr = conf.lib.clang_parseTranslationUnit(
index,
- fspath(filename) if filename is not None else None,
+ os.fspath(filename) if filename is not None else None,
args_array,
len(args),
unsaved_array,
@@ -3040,7 +3009,7 @@ def from_ast_file(cls, filename, index=None):
if index is None:
index = Index.create()
- ptr = conf.lib.clang_createTranslationUnit(index, fspath(filename))
+ ptr = conf.lib.clang_createTranslationUnit(index, os.fspath(filename))
if not ptr:
raise TranslationUnitLoadError(filename)
@@ -3159,7 +3128,7 @@ def diagnostics(self):
Return an iterable (and indexable) object containing the diagnostics.
"""
- class DiagIterator(object):
+ class DiagIterator:
def __init__(self, tu):
self.tu = tu
@@ -3193,7 +3162,7 @@ def reparse(self, unsaved_files=None, options=0):
if hasattr(contents, "read"):
contents = contents.read()
contents = b(contents)
- unsaved_files_array[i].name = b(fspath(name))
+ unsaved_files_array[i].name = b(os.fspath(name))
unsaved_files_array[i].contents = contents
unsaved_files_array[i].length = len(contents)
ptr = conf.lib.clang_reparseTranslationUnit(
@@ -3217,7 +3186,11 @@ def save(self, filename):
"""
options = conf.lib.clang_defaultSaveOptions(self)
result = int(
- conf.lib.clang_saveTranslationUnit(self, fspath(filename), options)
+ conf.lib.clang_saveTranslationUnit(
+ self,
+ os.fspath(filename),
+ options,
+ )
)
if result != 0:
raise TranslationUnitSaveError(result, "Error saving TranslationUnit.")
@@ -3261,12 +3234,12 @@ def codeComplete(
if hasattr(contents, "read"):
contents = contents.read()
contents = b(contents)
- unsaved_files_array[i].name = b(fspath(name))
+ unsaved_files_array[i].name = b(os.fspath(name))
unsaved_files_array[i].contents = contents
unsaved_files_array[i].length = len(contents)
ptr = conf.lib.clang_codeCompleteAt(
self,
- fspath(path),
+ os.fspath(path),
line,
column,
unsaved_files_array,
@@ -3300,7 +3273,9 @@ class File(ClangObject):
@staticmethod
def from_name(translation_unit, file_name):
"""Retrieve a file handle within the given translation unit."""
- return File(conf.lib.clang_getFile(translation_unit, fspath(file_name)))
+ return File(
+ conf.lib.clang_getFile(translation_unit, os.fspath(file_name)),
+ )
@property
def name(self):
@@ -3328,7 +3303,7 @@ def from_result(res, fn, args):
return res
-class FileInclusion(object):
+class FileInclusion:
"""
The FileInclusion class represents the inclusion of one source file by
another via a '#include' directive or as the input file for the translation
@@ -3377,7 +3352,7 @@ def __init__(self, enumeration, message):
Exception.__init__(self, "Error %d: %s" % (enumeration, message))
-class CompileCommand(object):
+class CompileCommand:
"""Represents the compile command used to build a file"""
def __init__(self, cmd, ccmds):
@@ -3409,7 +3384,7 @@ def arguments(self):
yield conf.lib.clang_CompileCommand_getArg(self.cmd, i)
-class CompileCommands(object):
+class CompileCommands:
"""
CompileCommands is an iterable object containing all CompileCommand
that can be used for building a specific file.
@@ -3460,7 +3435,7 @@ def fromDirectory(buildDir):
errorCode = c_uint()
try:
cdb = conf.lib.clang_CompilationDatabase_fromDirectory(
- fspath(buildDir), byref(errorCode)
+ os.fspath(buildDir), byref(errorCode)
)
except CompilationDatabaseError as e:
raise CompilationDatabaseError(
@@ -3474,7 +3449,7 @@ def getCompileCommands(self, filename):
build filename. Returns None if filename is not found in the database.
"""
return conf.lib.clang_CompilationDatabase_getCompileCommands(
- self, fspath(filename)
+ self, os.fspath(filename)
)
def getAllCompileCommands(self):
@@ -3865,7 +3840,7 @@ def register(item):
register(f)
-class Config(object):
+class Config:
library_path = None
library_file = None
compatibility_check = True
@@ -3880,7 +3855,7 @@ def set_library_path(path):
"any other functionalities in libclang."
)
- Config.library_path = fspath(path)
+ Config.library_path = os.fspath(path)
@staticmethod
def set_library_file(filename):
@@ -3891,7 +3866,7 @@ def set_library_file(filename):
"any other functionalities in libclang."
)
- Config.library_file = fspath(filename)
+ Config.library_file = os.fspath(filename)
@staticmethod
def set_compatibility_check(check_status):
|
This should just bump to 3.8 IMO. Anything below that is end-of-life and a waste of time to support. |
I agree and my personal plan is still to bump the version to 3.7 or 3.8, but it was more important to me to make progress towards adding any usable type annotations than it is to raise the min python version across llvm or just within libclang. I figured this incremental change was safe, couldn't be blocked based on min version, and doesn't introduce any syntax that has been obsoleted in later python. Feel free to checkout #77219, too. Let me know if you think there is something I could do to help progress the transition to python min version 3.8 (or 3.7). |
Looks sensible, but I'm not experienced enough with migration from Python 2 to 3 to approve. |
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.
Thanks all! @boomanaiden154 or @tru, could you merge this PR? |
Remove Python 2 support and clean up code that conditions based on version.
Issue #76664.