[PATCH] D26082: Support for Python 3 in libclang python bindings

[Mathieu Duponchelle via Phabricator via cfe-commits]

MathieuDuponchelle added a comment.

In https://reviews.llvm.org/D26082#645361, @jbcoe wrote:

> I've reverted this change and submitted https://reviews.llvm.org/D28682 to 
> explicitly check the Python version.
>
> I'll pick this up again, I do want Python 3 support.


I'm not entirely sure why you reverted the patch altogether, apart from the 
problem I raised it worked flawlessly, and in any case did not create any 
problems when the bindings were used from python 2.


Repository:
  rL LLVM

https://reviews.llvm.org/D26082



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26082: Support for Python 3 in libclang python bindings

[Jonathan B Coe via Phabricator via cfe-commits]

jbcoe added a comment.

I've reverted this change and submitted https://reviews.llvm.org/D28682 to 
explicitly check the Python version.

I'll pick this up again, I do want Python 3 support.


Repository:
  rL LLVM

https://reviews.llvm.org/D26082



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26082: Support for Python 3 in libclang python bindings

[Jonathan B Coe via Phabricator via cfe-commits]

jbcoe added a comment.

Sadly I can't run valgrind on macOS Sierra.

I'll try and look at the effects of bindings/python/clang/cindex.py L181 in 
more detail today.

Just out of curiosity, will you be using the Python 3 bindings for anything? If 
so what?


Repository:
  rL LLVM

https://reviews.llvm.org/D26082



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26082: Support for Python 3 in libclang python bindings

[Mathieu Duponchelle via Phabricator via cfe-commits]

MathieuDuponchelle added a comment.

In https://reviews.llvm.org/D26082#644402, @jbcoe wrote:

> @MathieuDuponchelle I've set up a docker image with Fedora 23 and reproduced 
> the errors as you reported.
>
> I've no immediate idea what the problem might be. Might be worth looking with 
> address-sanitizer.


Hey, good to know!

Two questions:

- Did you try running the test with valgrind on OSX?
- Doesn't the fact that commenting out this line: 
https://github.com/llvm-mirror/clang/blob/master/bindings/python/clang/cindex.py#L181
 "solve" the problem give you a good starting point?


Repository:
  rL LLVM

https://reviews.llvm.org/D26082



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26082: Support for Python 3 in libclang python bindings

[Jonathan B Coe via Phabricator via cfe-commits]

jbcoe added a comment.

@MathieuDuponchelle I've set up a docker image with Fedora 23 and reproduced 
the errors as you reported.

I've no immediate idea what the problem might be. Might be worth looking with 
address-sanitizer.


Repository:
  rL LLVM

https://reviews.llvm.org/D26082



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26082: Support for Python 3 in libclang python bindings

[Jonathan B Coe via Phabricator via cfe-commits]

jbcoe added a comment.

@MathieuDuponchelle I'm still keen to get to the bottom of this failure. I've 
not had time to reproduce it yet.

I'm thankful for you taking a look at this and reporting the issue. We need to 
get this working well for everyone.


Repository:
  rL LLVM

https://reviews.llvm.org/D26082



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26082: Support for Python 3 in libclang python bindings

[Mathieu Duponchelle via Phabricator via cfe-commits]

MathieuDuponchelle added a comment.

In https://reviews.llvm.org/D26082#621496, @jbcoe wrote:

> Hi Mathieu,
>  What platform did you run the tests on? How did you run them? I don't get 
> any failures when running tests on macOS 10.12.1.


Hey @jbcoe, do you think you can have a look at this issue? Please see my 
earlier comment for a way to reproduce this 
(`LD_LIBRARY_PATH=/home/meh/devel/sandbox/llvm/build/lib valgrind python3 -m 
nose tests.cindex.test_cursor:test_get_template_argument_value` is a valid way 
to reproduce this issue)


Repository:
  rL LLVM

https://reviews.llvm.org/D26082



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26082: Support for Python 3 in libclang python bindings

[Mathieu Duponchelle via Phabricator via cfe-commits]

MathieuDuponchelle added a comment.

Hey, I'm on Linux, Fedora 23 to be specific.


Repository:
  rL LLVM

https://reviews.llvm.org/D26082



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26082: Support for Python 3 in libclang python bindings

[Jonathan B Coe via Phabricator via cfe-commits]

jbcoe added a comment.

Hi Mathieu,
What platform did you run the tests on? How did you run them? I don't get any 
failures when running tests on macOS 10.12.1.

command: `env PYTHONPATH=`pwd` 
LD_LIBRARY_PATH=/Users/jon/DEV/LLVM/build-ninja/lib nosetests-3.4 .`

python: Python 3.5.2 (default, Oct 11 2016, 04:59:56) 
[GCC 4.2.1 Compatible Apple LLVM 8.0.0 (clang-800.0.38)] on darwin

Python installed using homebrew.

I'm keen to get to the bottom of this failure. I've been distracted by other 
things recently.


Repository:
  rL LLVM

https://reviews.llvm.org/D26082



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26082: Support for Python 3 in libclang python bindings

[Mathieu Duponchelle via cfe-commits]

MathieuDuponchelle added a comment.

I get tons of errors when running these tests with python 3 (version 3.4.3) 
with a fresh build of clang and llvm. The number of failures vs errors vary 
wildly between each invocation, the number of total failures + errors stays the 
same, which led me to think there was some sort of memory corruption.

Running the tests under valgrind make them pass, but also shows errors which I 
assume are ctypes-related.

This is valgrind's output for ` 
LD_LIBRARY_PATH=/home/meh/devel/sandbox/llvm/build/lib valgrind python3 -m nose 
tests.cindex.test_cursor:test_get_template_argument_value` as an example

F2616611: valgrind_output.txt 


Repository:
  rL LLVM

https://reviews.llvm.org/D26082



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26082: Support for Python 3 in libclang python bindings

[Mathieu Duponchelle via Phabricator via cfe-commits]

MathieuDuponchelle added a comment.

Hey, for what it's worth when skipping the free at this line: 
https://github.com/llvm-mirror/clang/blob/master/bindings/python/clang/cindex.py#L181
 the issues are gone, I haven't investigated the reason why.


Repository:
  rL LLVM

https://reviews.llvm.org/D26082



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26082: Support for Python 3 in libclang python bindings

[Jonathan B Coe via Phabricator via cfe-commits]

jbcoe added a comment.

Thanks Mathieu.

Tests ran without issues when I submitted my patch but I tested on macOS.

I'll have a look into this and see if I can reproduce the problem.


Repository:
  rL LLVM

https://reviews.llvm.org/D26082



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26082: Support for Python 3 in libclang python bindings

[Jonathan B Coe via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL285909: Support for Python 3 in libclang python bindings 
(authored by jbcoe).

Changed prior to commit:
  https://reviews.llvm.org/D26082?vs=76774=76854#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D26082

Files:
  cfe/trunk/bindings/python/clang/cindex.py
  cfe/trunk/bindings/python/tests/cindex/test_translation_unit.py

Index: cfe/trunk/bindings/python/clang/cindex.py
===
--- cfe/trunk/bindings/python/clang/cindex.py
+++ cfe/trunk/bindings/python/clang/cindex.py
@@ -64,6 +64,7 @@
 
 from ctypes import *
 import collections
+import sys
 
 import clang.enumerations
 
@@ -73,6 +74,33 @@
 # this by marshalling object arguments as void**.
 c_object_p = POINTER(c_void_p)
 
+if sys.version_info[0] > 2:
+# Python 3 strings are unicode, translate them to/from utf8 for C-interop
+# Python 3 replaces xrange with range, we want xrange behaviour
+xrange = range
+
+class c_string_p(c_char_p):
+def __init__(self, p=None):
+if type(p) == str:
+p = p.encode("utf8")
+super(c_char_p, self).__init__(p)
+
+def __str__(self):
+return str(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):
+return cls(param)
+else:
+c_string_p = c_char_p
+
+
 callbacks = {}
 
 ### Exception Classes ###
@@ -147,7 +175,7 @@
 class _CXString(Structure):
 """Helper for transforming CXString results."""
 
-_fields_ = [("spelling", c_char_p), ("free", c_int)]
+_fields_ = [("spelling", c_string_p), ("free", c_int)]
 
 def __del__(self):
 conf.lib.clang_disposeString(self)
@@ -329,7 +357,7 @@
 
 @property
 def spelling(self):
-return conf.lib.clang_getDiagnosticSpelling(self)
+return str(conf.lib.clang_getDiagnosticSpelling(self))
 
 @property
 def ranges(self):
@@ -358,8 +386,8 @@
 
 def __getitem__(self, key):
 range = SourceRange()
-value = conf.lib.clang_getDiagnosticFixIt(self.diag, key,
-byref(range))
+value = str(conf.lib.clang_getDiagnosticFixIt(self.diag, key,
+byref(range)))
 if len(value) == 0:
 raise IndexError
 
@@ -392,20 +420,20 @@
 @property
 def category_name(self):
 """The string name of the category for this diagnostic."""
-return conf.lib.clang_getDiagnosticCategoryText(self)
+return str(conf.lib.clang_getDiagnosticCategoryText(self))
 
 @property
 def option(self):
 """The command-line option that enables this diagnostic."""
-return conf.lib.clang_getDiagnosticOption(self, None)
+return str(conf.lib.clang_getDiagnosticOption(self, None))
 
 @property
 def disable_option(self):
 """The command-line option that disables this diagnostic."""
 disable = _CXString()
 conf.lib.clang_getDiagnosticOption(self, byref(disable))
 
-return conf.lib.clang_getCString(disable)
+return str(conf.lib.clang_getCString(disable))
 
 def format(self, options=None):
 """
@@ -554,8 +582,8 @@
 if value >= len(self.__class__._kinds):
 self.__class__._kinds += [None] * (value - len(self.__class__._kinds) + 1)
 if self.__class__._kinds[value] is not None:
-raise ValueError,'{0} value {1} already loaded'.format(
-str(self.__class__), value)
+raise ValueError('{0} value {1} already loaded'.format(
+str(self.__class__), value))
 self.value = value
 self.__class__._kinds[value] = self
 self.__class__._name_map = None
@@ -572,12 +600,12 @@
 for key, value in self.__class__.__dict__.items():
 if isinstance(value, self.__class__):
 self._name_map[value] = key
-return self._name_map[self]
+return str(self._name_map[self])
 
 @classmethod
 def from_id(cls, id):
 if id >= len(cls._kinds) or cls._kinds[id] is None:
-raise ValueError,'Unknown template argument kind %d' % id
+raise ValueError('Unknown template argument kind %d' % id)
 return cls._kinds[id]
 
 def __repr__(self):
@@ -596,7 +624,7 @@
 @staticmethod
 def get_all_kinds():
 """Return all CursorKind enumeration instances."""
-return filter(None, CursorKind._kinds)
+return [x for x in CursorKind._kinds if x]
 
 def is_declaration(self):
 """Test if this is a declaration kind."""
@@ -1427,9 +1455,9 @@
 def spelling(self):
 """Return the spelling of the 

[PATCH] D26082: Support for Python 3 in libclang python bindings

[Jonathan B Coe via cfe-commits]

jbcoe added inline comments.



Comment at: bindings/python/clang/cindex.py:77
+# Python 3 strings are unicode, translate them to/from utf8 for C-interop
+if type(u"") == str:
+class c_string_p(c_char_p):

mgorny wrote:
> What is the Python3 version you're aiming to support? If I recall correctly, 
> `u""` is allowed (again) since 3.4. And even then, the condition looks weird 
> and makes me think a while before I figure out what it's supposed to mean.
Replaced with a simple version check



Comment at: bindings/python/clang/cindex.py:518
 
-for i in xrange(0, count):
+for i in range(0, count):
 token = Token()

mgorny wrote:
> compnerd wrote:
> > IIRC, `range` and `xrange` did have some performance difference.  This 
> > would slow down the bindings on python 2.  The difference is obviously not 
> > immediately visible unless count is very high.  I wonder if we can do 
> > anything here to detect the python version and dispatch to `xrange` in 
> > python 2.
> The difference is that `range()` used to construct a complete list in Python 
> 2. In Python 3, `xrange()` (that uses iterator) got renamed to `range()`.
> 
> If you want to avoid performance impact (not sure if it's really measurable 
> here), you can do something alike C for loop:
> 
> i = 0
> while i < count:
> #...
> i += 1
> 
> It's not really idiomatic Python though. OTOH, it won't take more lines than 
> the conditional.
I've defined `xrange` to be `range` for python3. A bit confusing but then we 
can use `xrange` uniformly.



Comment at: bindings/python/clang/cindex.py:623
 """Return all CursorKind enumeration instances."""
-return filter(None, CursorKind._kinds)
+return [x for x in CursorKind._kinds if x]
 

mgorny wrote:
> Why are you changing this? The old version seems to be correct for Python3.
`filter` has changed in Python 3 and the replaced code does not behave the same 
as this simple list comprehension. I've not delved deeper into why but see test 
failures without this change.



Comment at: bindings/python/clang/cindex.py:2573
 if len(args) > 0:
-args_array = (c_char_p * len(args))(* args)
+args_array = (c_string_p * len(args))()
+for i,a in enumerate(args):

mgorny wrote:
> I may be wrong but I think you could use a list comprehension here.
> 
> args_array = (c_string_p * len(args))([c_string_p(x) for x in args])
> 
> You can also try without `[]` to avoid the overhead of constructing list, if 
> the function can take an iterator.
I can't get this to work with a comprension or generator, I agree either would 
be an improvement.



Comment at: bindings/python/tests/cindex/test_translation_unit.py:62
 def test_unsaved_files_2():
-import StringIO
+try:
+from StringIO import StringIO

mgorny wrote:
> Could you try inverting this? Python 2.7 already has `io.StringIO`, so that 
> branch is much more likely to work.
Python 2.7's `io.StringIO` needs a unicode string.


https://reviews.llvm.org/D26082



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26082: Support for Python 3 in libclang python bindings

[Jonathan B Coe via cfe-commits]

jbcoe updated this revision to Diff 76774.
jbcoe marked an inline comment as done.
jbcoe added a comment.

Respond to review comments.


https://reviews.llvm.org/D26082

Files:
  bindings/python/clang/cindex.py
  bindings/python/tests/cindex/test_translation_unit.py

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
@@ -59,9 +59,13 @@
 assert spellings[-1] == 'y'
 
 def test_unsaved_files_2():
-import StringIO
+try:
+from StringIO import StringIO
+except:
+from io import StringIO
+
 tu = TranslationUnit.from_source('fake.c', unsaved_files = [
-('fake.c', StringIO.StringIO('int x;'))])
+('fake.c', StringIO('int x;'))])
 spellings = [c.spelling for c in tu.cursor.get_children()]
 assert spellings[-1] == 'x'
 
Index: bindings/python/clang/cindex.py
===
--- bindings/python/clang/cindex.py
+++ bindings/python/clang/cindex.py
@@ -64,6 +64,7 @@
 
 from ctypes import *
 import collections
+import sys
 
 import clang.enumerations
 
@@ -73,6 +74,33 @@
 # this by marshalling object arguments as void**.
 c_object_p = POINTER(c_void_p)
 
+if sys.version_info[0] > 2:
+# Python 3 strings are unicode, translate them to/from utf8 for C-interop
+# Python 3 replaces xrange with range, we want xrange behaviour
+xrange = range
+
+class c_string_p(c_char_p):
+def __init__(self, p=None):
+if type(p) == str:
+p = p.encode("utf8")
+super(c_char_p, self).__init__(p)
+
+def __str__(self):
+return str(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):
+return cls(param)
+else:
+c_string_p = c_char_p
+
+
 callbacks = {}
 
 ### Exception Classes ###
@@ -147,7 +175,7 @@
 class _CXString(Structure):
 """Helper for transforming CXString results."""
 
-_fields_ = [("spelling", c_char_p), ("free", c_int)]
+_fields_ = [("spelling", c_string_p), ("free", c_int)]
 
 def __del__(self):
 conf.lib.clang_disposeString(self)
@@ -329,7 +357,7 @@
 
 @property
 def spelling(self):
-return conf.lib.clang_getDiagnosticSpelling(self)
+return str(conf.lib.clang_getDiagnosticSpelling(self))
 
 @property
 def ranges(self):
@@ -358,8 +386,8 @@
 
 def __getitem__(self, key):
 range = SourceRange()
-value = conf.lib.clang_getDiagnosticFixIt(self.diag, key,
-byref(range))
+value = str(conf.lib.clang_getDiagnosticFixIt(self.diag, key,
+byref(range)))
 if len(value) == 0:
 raise IndexError
 
@@ -392,20 +420,20 @@
 @property
 def category_name(self):
 """The string name of the category for this diagnostic."""
-return conf.lib.clang_getDiagnosticCategoryText(self)
+return str(conf.lib.clang_getDiagnosticCategoryText(self))
 
 @property
 def option(self):
 """The command-line option that enables this diagnostic."""
-return conf.lib.clang_getDiagnosticOption(self, None)
+return str(conf.lib.clang_getDiagnosticOption(self, None))
 
 @property
 def disable_option(self):
 """The command-line option that disables this diagnostic."""
 disable = _CXString()
 conf.lib.clang_getDiagnosticOption(self, byref(disable))
 
-return conf.lib.clang_getCString(disable)
+return str(conf.lib.clang_getCString(disable))
 
 def format(self, options=None):
 """
@@ -554,8 +582,8 @@
 if value >= len(self.__class__._kinds):
 self.__class__._kinds += [None] * (value - len(self.__class__._kinds) + 1)
 if self.__class__._kinds[value] is not None:
-raise ValueError,'{0} value {1} already loaded'.format(
-str(self.__class__), value)
+raise ValueError('{0} value {1} already loaded'.format(
+str(self.__class__), value))
 self.value = value
 self.__class__._kinds[value] = self
 self.__class__._name_map = None
@@ -572,12 +600,12 @@
 for key, value in self.__class__.__dict__.items():
 if isinstance(value, self.__class__):
 self._name_map[value] = key
-return self._name_map[self]
+return str(self._name_map[self])
 
 @classmethod
 def from_id(cls, id):
 if id >= len(cls._kinds) or cls._kinds[id] is None:
-raise ValueError,'Unknown template argument kind %d' % id
+  

[PATCH] D26082: Support for Python 3 in libclang python bindings

[Michał Górny via cfe-commits]

mgorny added a comment.

A few comments/questions. However, please note that those are generic Python 
comments and I haven't used or tested the clang Python API yet.




Comment at: bindings/python/clang/cindex.py:77
+# Python 3 strings are unicode, translate them to/from utf8 for C-interop
+if type(u"") == str:
+class c_string_p(c_char_p):

What is the Python3 version you're aiming to support? If I recall correctly, 
`u""` is allowed (again) since 3.4. And even then, the condition looks weird 
and makes me think a while before I figure out what it's supposed to mean.



Comment at: bindings/python/clang/cindex.py:518
 
-for i in xrange(0, count):
+for i in range(0, count):
 token = Token()

compnerd wrote:
> IIRC, `range` and `xrange` did have some performance difference.  This would 
> slow down the bindings on python 2.  The difference is obviously not 
> immediately visible unless count is very high.  I wonder if we can do 
> anything here to detect the python version and dispatch to `xrange` in python 
> 2.
The difference is that `range()` used to construct a complete list in Python 2. 
In Python 3, `xrange()` (that uses iterator) got renamed to `range()`.

If you want to avoid performance impact (not sure if it's really measurable 
here), you can do something alike C for loop:

i = 0
while i < count:
#...
i += 1

It's not really idiomatic Python though. OTOH, it won't take more lines than 
the conditional.



Comment at: bindings/python/clang/cindex.py:623
 """Return all CursorKind enumeration instances."""
-return filter(None, CursorKind._kinds)
+return [x for x in CursorKind._kinds if x]
 

Why are you changing this? The old version seems to be correct for Python3.



Comment at: bindings/python/clang/cindex.py:2371
 
+
 def __repr__(self):

Seems to be mistakenly added.



Comment at: bindings/python/clang/cindex.py:2573
 if len(args) > 0:
-args_array = (c_char_p * len(args))(* args)
+args_array = (c_string_p * len(args))()
+for i,a in enumerate(args):

I may be wrong but I think you could use a list comprehension here.

args_array = (c_string_p * len(args))([c_string_p(x) for x in args])

You can also try without `[]` to avoid the overhead of constructing list, if 
the function can take an iterator.



Comment at: bindings/python/tests/cindex/test_translation_unit.py:62
 def test_unsaved_files_2():
-import StringIO
+try:
+from StringIO import StringIO

Could you try inverting this? Python 2.7 already has `io.StringIO`, so that 
branch is much more likely to work.


https://reviews.llvm.org/D26082



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26082: Support for Python 3 in libclang python bindings

[Saleem Abdulrasool via cfe-commits]

compnerd accepted this revision.
compnerd added a comment.
This revision is now accepted and ready to land.

It would be nice if there is a simple way to handle the possible performance 
impact for python 2.  Worst case, we can deal with it when it becomes an issue.




Comment at: bindings/python/clang/cindex.py:518
 
-for i in xrange(0, count):
+for i in range(0, count):
 token = Token()

IIRC, `range` and `xrange` did have some performance difference.  This would 
slow down the bindings on python 2.  The difference is obviously not 
immediately visible unless count is very high.  I wonder if we can do anything 
here to detect the python version and dispatch to `xrange` in python 2.


https://reviews.llvm.org/D26082



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26082: Support for Python 3 in libclang python bindings

[Jonathan B Coe via cfe-commits]

jbcoe removed rL LLVM as the repository for this revision.
jbcoe updated this revision to Diff 76344.
jbcoe added a comment.

Remove mistakenly committed debugging output.


https://reviews.llvm.org/D26082

Files:
  bindings/python/clang/cindex.py
  bindings/python/tests/cindex/test_translation_unit.py

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
@@ -59,9 +59,13 @@
 assert spellings[-1] == 'y'
 
 def test_unsaved_files_2():
-import StringIO
+try:
+from StringIO import StringIO
+except:
+from io import StringIO
+
 tu = TranslationUnit.from_source('fake.c', unsaved_files = [
-('fake.c', StringIO.StringIO('int x;'))])
+('fake.c', StringIO('int x;'))])
 spellings = [c.spelling for c in tu.cursor.get_children()]
 assert spellings[-1] == 'x'
 
Index: bindings/python/clang/cindex.py
===
--- bindings/python/clang/cindex.py
+++ bindings/python/clang/cindex.py
@@ -73,6 +73,30 @@
 # this by marshalling object arguments as void**.
 c_object_p = POINTER(c_void_p)
 
+# Python 3 strings are unicode, translate them to/from utf8 for C-interop
+if type(u"") == str:
+class c_string_p(c_char_p):
+def __init__(self, p=None):
+if type(p) == str:
+p = p.encode("utf8")
+super(c_char_p, self).__init__(p)
+
+def __str__(self):
+return str(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):
+return cls(param)
+else:
+c_string_p = c_char_p
+
+
 callbacks = {}
 
 ### Exception Classes ###
@@ -147,7 +171,7 @@
 class _CXString(Structure):
 """Helper for transforming CXString results."""
 
-_fields_ = [("spelling", c_char_p), ("free", c_int)]
+_fields_ = [("spelling", c_string_p), ("free", c_int)]
 
 def __del__(self):
 conf.lib.clang_disposeString(self)
@@ -329,7 +353,7 @@
 
 @property
 def spelling(self):
-return conf.lib.clang_getDiagnosticSpelling(self)
+return str(conf.lib.clang_getDiagnosticSpelling(self))
 
 @property
 def ranges(self):
@@ -358,8 +382,8 @@
 
 def __getitem__(self, key):
 range = SourceRange()
-value = conf.lib.clang_getDiagnosticFixIt(self.diag, key,
-byref(range))
+value = str(conf.lib.clang_getDiagnosticFixIt(self.diag, key,
+byref(range)))
 if len(value) == 0:
 raise IndexError
 
@@ -392,20 +416,20 @@
 @property
 def category_name(self):
 """The string name of the category for this diagnostic."""
-return conf.lib.clang_getDiagnosticCategoryText(self)
+return str(conf.lib.clang_getDiagnosticCategoryText(self))
 
 @property
 def option(self):
 """The command-line option that enables this diagnostic."""
-return conf.lib.clang_getDiagnosticOption(self, None)
+return str(conf.lib.clang_getDiagnosticOption(self, None))
 
 @property
 def disable_option(self):
 """The command-line option that disables this diagnostic."""
 disable = _CXString()
 conf.lib.clang_getDiagnosticOption(self, byref(disable))
 
-return conf.lib.clang_getCString(disable)
+return str(conf.lib.clang_getCString(disable))
 
 def format(self, options=None):
 """
@@ -491,7 +515,7 @@
 
 token_group = TokenGroup(tu, tokens_memory, tokens_count)
 
-for i in xrange(0, count):
+for i in range(0, count):
 token = Token()
 token.int_data = tokens_array[i].int_data
 token.ptr_data = tokens_array[i].ptr_data
@@ -554,8 +578,8 @@
 if value >= len(self.__class__._kinds):
 self.__class__._kinds += [None] * (value - len(self.__class__._kinds) + 1)
 if self.__class__._kinds[value] is not None:
-raise ValueError,'{0} value {1} already loaded'.format(
-str(self.__class__), value)
+raise ValueError('{0} value {1} already loaded'.format(
+str(self.__class__), value))
 self.value = value
 self.__class__._kinds[value] = self
 self.__class__._name_map = None
@@ -572,12 +596,12 @@
 for key, value in self.__class__.__dict__.items():
 if isinstance(value, self.__class__):
 self._name_map[value] = key
-return self._name_map[self]
+return str(self._name_map[self])
 
 @classmethod
 def from_id(cls, 

[PATCH] D26082: Support for Python 3 in libclang python bindings

[Jonathan B Coe via cfe-commits]

jbcoe retitled this revision from "Incomplete support for Python 3 in libclang 
python bindings" to "Support for Python 3 in libclang python bindings".
jbcoe updated the summary for this revision.
jbcoe updated this revision to Diff 76338.
jbcoe added a comment.

Python bindings tests now pass in Python 3.


Repository:
  rL LLVM

https://reviews.llvm.org/D26082

Files:
  bindings/python/clang/cindex.py
  bindings/python/tests/cindex/test_comment.py
  bindings/python/tests/cindex/test_translation_unit.py

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
@@ -59,9 +59,13 @@
 assert spellings[-1] == 'y'
 
 def test_unsaved_files_2():
-import StringIO
+try:
+from StringIO import StringIO
+except:
+from io import StringIO
+
 tu = TranslationUnit.from_source('fake.c', unsaved_files = [
-('fake.c', StringIO.StringIO('int x;'))])
+('fake.c', StringIO('int x;'))])
 spellings = [c.spelling for c in tu.cursor.get_children()]
 assert spellings[-1] == 'x'
 
Index: bindings/python/tests/cindex/test_comment.py
===
--- bindings/python/tests/cindex/test_comment.py
+++ bindings/python/tests/cindex/test_comment.py
@@ -34,6 +34,12 @@
 f = get_cursor(tu, 'f')
 raw = f.raw_comment
 brief = f.brief_comment
+
+print(raw)
+print(type(raw))
+print(brief)
+print(type(brief))
+
 assert raw is None
 assert brief is None
 
Index: bindings/python/clang/cindex.py
===
--- bindings/python/clang/cindex.py
+++ bindings/python/clang/cindex.py
@@ -73,6 +73,30 @@
 # this by marshalling object arguments as void**.
 c_object_p = POINTER(c_void_p)
 
+# Python 3 strings are unicode, translate them to/from utf8 for C-interop
+if type(u"") == str:
+class c_string_p(c_char_p):
+def __init__(self, p=None):
+if type(p) == str:
+p = p.encode("utf8")
+super(c_char_p, self).__init__(p)
+
+def __str__(self):
+return str(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):
+return cls(param)
+else:
+c_string_p = c_char_p
+
+
 callbacks = {}
 
 ### Exception Classes ###
@@ -147,7 +171,7 @@
 class _CXString(Structure):
 """Helper for transforming CXString results."""
 
-_fields_ = [("spelling", c_char_p), ("free", c_int)]
+_fields_ = [("spelling", c_string_p), ("free", c_int)]
 
 def __del__(self):
 conf.lib.clang_disposeString(self)
@@ -329,7 +353,7 @@
 
 @property
 def spelling(self):
-return conf.lib.clang_getDiagnosticSpelling(self)
+return str(conf.lib.clang_getDiagnosticSpelling(self))
 
 @property
 def ranges(self):
@@ -358,8 +382,8 @@
 
 def __getitem__(self, key):
 range = SourceRange()
-value = conf.lib.clang_getDiagnosticFixIt(self.diag, key,
-byref(range))
+value = str(conf.lib.clang_getDiagnosticFixIt(self.diag, key,
+byref(range)))
 if len(value) == 0:
 raise IndexError
 
@@ -392,20 +416,20 @@
 @property
 def category_name(self):
 """The string name of the category for this diagnostic."""
-return conf.lib.clang_getDiagnosticCategoryText(self)
+return str(conf.lib.clang_getDiagnosticCategoryText(self))
 
 @property
 def option(self):
 """The command-line option that enables this diagnostic."""
-return conf.lib.clang_getDiagnosticOption(self, None)
+return str(conf.lib.clang_getDiagnosticOption(self, None))
 
 @property
 def disable_option(self):
 """The command-line option that disables this diagnostic."""
 disable = _CXString()
 conf.lib.clang_getDiagnosticOption(self, byref(disable))
 
-return conf.lib.clang_getCString(disable)
+return str(conf.lib.clang_getCString(disable))
 
 def format(self, options=None):
 """
@@ -491,7 +515,7 @@
 
 token_group = TokenGroup(tu, tokens_memory, tokens_count)
 
-for i in xrange(0, count):
+for i in range(0, count):
 token = Token()
 token.int_data = tokens_array[i].int_data
 token.ptr_data = tokens_array[i].ptr_data
@@ -554,8 +578,8 @@
 if value >= len(self.__class__._kinds):
 self.__class__._kinds += [None] * (value - len(self.__class__._kinds) + 1)
 if self.__class__._kinds[value] is