Python bindings: Fix handling of file bodies with multi-byte characters

[mheinzler]

Summary

With python3 there is a difference between the length of the string and the length of the utf-8 encoded bytes array. To not cut off characters at the end when the string contains multi-byte characters, the length of file contents that gets passed to clang needs to be calculated from their bytes representation.

I also added a test case that catches this. I needed to add the coding line at the top of the test unit to make python2 work with the embedded Unicode character. Alternatively we could replace the character with /uXXXX, but then there would be other problems with python2.

[kristina]

Would you mind re-uploading these patches with full context (with diff -U99999). 3 lines of context around changes makes this very difficult to review. Also I would suggest testing for Python version and using appropriate semantics (using encode in case of Python3). Not entirely sure if the bindings are supposed to be compatible with Python3, but I don't see any harm. However explicit tests that avoid changing functionality of existing Python2.7 code would be better in my opinion.

[mheinzler]

Sorry, here's the diff for the whole files.

The b function defined at the top of the file already does what you suggest. For python2 it returns the string unchanged, for python3 it calls encode. So there shouldn't be any change at all for python2.

[mheinzler]

I'm closing this because it has been fixed in master by:
https://reviews.llvm.org/D56429

bindings/python/clang/cindex.py

def from_source(cls, filename, args=None, unsaved_files=None, options=0,
         if len(unsaved_files) > 0:
             unsaved_array = (_CXUnsavedFile * len(unsaved_files))()
             for i, (name, contents) in enumerate(unsaved_files):
                 if hasattr(contents, "read"):
                     contents = contents.read()
 
                 unsaved_array[i].name = b(name)
                 unsaved_array[i].contents = b(contents)
-                unsaved_array[i].length = len(contents)
+                unsaved_array[i].length = len(unsaved_array[i].contents)
 
         ptr = conf.lib.clang_parseTranslationUnit(index, filename, args_array,
                                     len(args), unsaved_array,
                                     len(unsaved_files), options)
 
         if not ptr:
             raise TranslationUnitLoadError("Error parsing translation unit.")
 
def reparse(self, unsaved_files=None, options=0):
             for i,(name,value) in enumerate(unsaved_files):
                 if not isinstance(value, str):
                     # FIXME: It would be great to support an efficient version
                     # of this, one day.
                     value = value.read()
                     print(value)
                 if not isinstance(value, str):
                     raise TypeError('Unexpected unsaved file contents.')
-                unsaved_files_array[i].name = name
-                unsaved_files_array[i].contents = value
-                unsaved_files_array[i].length = len(value)
+                unsaved_files_array[i].name = b(name)
+                unsaved_files_array[i].contents = b(value)
+                unsaved_files_array[i].length = \
+                    len(unsaved_files_array[i].contents)
         ptr = conf.lib.clang_reparseTranslationUnit(self, len(unsaved_files),
                 unsaved_files_array, options)
 
     def save(self, filename):
         """Saves the TranslationUnit to a file.
 
         This is equivalent to passing -emit-ast to the clang frontend. The
         saved file can be loaded back into a TranslationUnit. Or, if it
def codeComplete(self, path, line, column, unsaved_files=None,
                     # FIXME: It would be great to support an efficient version
                     # of this, one day.
                     value = value.read()
                     print(value)
                 if not isinstance(value, str):
                     raise TypeError('Unexpected unsaved file contents.')
                 unsaved_files_array[i].name = b(name)
                 unsaved_files_array[i].contents = b(value)
-                unsaved_files_array[i].length = len(value)
+                unsaved_files_array[i].length = \
+                    len(unsaved_files_array[i].contents)
         ptr = conf.lib.clang_codeCompleteAt(self, path, line, column,
                 unsaved_files_array, len(unsaved_files), options)
         if ptr:
             return CodeCompletionResults(ptr)
         return None
 
     def get_tokens(self, locations=None, extent=None):
         """Obtain tokens in this translation unit.

bindings/python/tests/cindex/test_translation_unit.py

+# -*- coding: utf-8 -*-
+
 from contextlib import contextmanager
 import gc
 import os
 import tempfile
 import unittest
 
 from clang.cindex import CursorKind
 from clang.cindex import Cursor
def test_unsaved_files_2(self):
             from StringIO import StringIO
         except:
             from io import StringIO
         tu = TranslationUnit.from_source('fake.c', unsaved_files = [
                 ('fake.c', StringIO('int x;'))])
         spellings = [c.spelling for c in tu.cursor.get_children()]
         self.assertEqual(spellings[-1], 'x')
 
+    def test_unsaved_files_encoding(self):
+        tu = TranslationUnit.from_source('fake.c', ['-I./'], unsaved_files = [
+                ('fake.c', """
+// 😀
+int x;
+""")
+                ])
+        spellings = [c.spelling for c in tu.cursor.get_children()]
+        self.assertEqual(spellings[-1], 'x')
+
     def assert_normpaths_equal(self, path1, path2):
         """ Compares two paths for equality after normalizing them with
             os.path.normpath
         """
         self.assertEqual(os.path.normpath(path1),
                          os.path.normpath(path2))
 
     def test_includes(self):