Use native traceback mutation on python 3.7

[njsmith]

Since python/cpython#4793 was merged, it looks like traceback objects will support Python-level instantiation and mutation in 3.7. We should use that instead of the current ctypes horrors.

We should also hassle the PyPy devs about implementing this. (In some ways this is even more interesting, because we're more aggressive about dropping support for old PyPy versions.)

[davidism]

We should also hassle the PyPy devs about implementing this.

Is there an assumption that they won't make this assignable when they support 3.7 even though it's documented now?

I'm currently fixing Jinja's traceback rewriting due to the changes in 3.7 and an issue with ctypes on debug builds (pallets/jinja#1050, pallets/jinja#1051, pallets/jinja#1104), and it would be nice if I can assume any 3.7 means tb_next is assignable. PyPy's docs for tproxy now list it as deprecated.

[pquentin]

Another argument in favor of this is that we're currently using PyPy transparent proxies for concat_tb:

trio/trio/_core/_multierror.py

Lines 249 to 356 in 4f65ff2

	################################################################
	# concat_tb
	################################################################
	# We need to compute a new traceback that is the concatenation of two existing
	# tracebacks. This requires copying the entries in 'head' and then pointing
	# the final tb_next to 'tail'.
	#
	# NB: 'tail' might be None, which requires some special handling in the ctypes
	# version.
	#
	# The complication here is that Python doesn't actually support copying or
	# modifying traceback objects, so we have to get creative...
	#
	# On CPython, we use ctypes. On PyPy, we use "transparent proxies".
	#
	# Jinja2 is a useful source of inspiration:
	# https://github.com/pallets/jinja/blob/master/jinja2/debug.py
	try:
	import tputil
	except ImportError:
	have_tproxy = False
	else:
	have_tproxy = True
	if have_tproxy:
	# http://doc.pypy.org/en/latest/objspace-proxies.html
	def copy_tb(base_tb, tb_next):
	def controller(operation):
	# Rationale for pragma: I looked fairly carefully and tried a few
	# things, and AFAICT it's not actually possible to get any
	# 'opname' that isn't __getattr__ or __getattribute__. So there's
	# no missing test we could add, and no value in coverage nagging
	# us about adding one.
	if operation.opname in [
	"__getattribute__", "__getattr__"
	]: # pragma: no cover
	if operation.args[0] == "tb_next":
	return tb_next
	return operation.delegate()
	return tputil.make_proxy(controller, type(base_tb), base_tb)
	else:
	# ctypes it is
	import ctypes
	# How to handle refcounting? I don't want to use ctypes.py_object because
	# I don't understand or trust it, and I don't want to use
	# ctypes.pythonapi.Py_{Inc,Dec}Ref because we might clash with user code
	# that also tries to use them but with different types. So private _ctypes
	# APIs it is!
	import _ctypes
	class CTraceback(ctypes.Structure):
	_fields_ = [
	("PyObject_HEAD", ctypes.c_byte * object().__sizeof__()),
	("tb_next", ctypes.c_void_p),
	("tb_frame", ctypes.c_void_p),
	("tb_lasti", ctypes.c_int),
	("tb_lineno", ctypes.c_int),
	]
	def copy_tb(base_tb, tb_next):
	# TracebackType has no public constructor, so allocate one the hard way
	try:
	raise ValueError
	except ValueError as exc:
	new_tb = exc.__traceback__
	c_new_tb = CTraceback.from_address(id(new_tb))
	# At the C level, tb_next either pointer to the next traceback or is
	# NULL. c_void_p and the .tb_next accessor both convert NULL to None,
	# but we shouldn't DECREF None just because we assigned to a NULL
	# pointer! Here we know that our new traceback has only 1 frame in it,
	# so we can assume the tb_next field is NULL.
	assert c_new_tb.tb_next is None
	# If tb_next is None, then we want to set c_new_tb.tb_next to NULL,
	# which it already is, so we're done. Otherwise, we have to actually
	# do some work:
	if tb_next is not None:
	_ctypes.Py_INCREF(tb_next)
	c_new_tb.tb_next = id(tb_next)
	assert c_new_tb.tb_frame is not None
	_ctypes.Py_INCREF(base_tb.tb_frame)
	old_tb_frame = new_tb.tb_frame
	c_new_tb.tb_frame = id(base_tb.tb_frame)
	_ctypes.Py_DECREF(old_tb_frame)
	c_new_tb.tb_lasti = base_tb.tb_lasti
	c_new_tb.tb_lineno = base_tb.tb_lineno
	return new_tb
	def concat_tb(head, tail):
	# We have to use an iterative algorithm here, because in the worst case
	# this might be a RecursionError stack that is by definition too deep to
	# process by recursion!
	head_tbs = []
	pointer = head
	while pointer is not None:
	head_tbs.append(pointer)
	pointer = pointer.tb_next
	current_head = tail
	for head_tb in reversed(head_tbs):
	current_head = copy_tb(head_tb, tb_next=current_head)
	return current_head

They're documented and Trio uses them since 2017-02. However, in 2018-05, @mattip marked them as deprecated. It does not look like they're planning to remove them right now, but migrating away from tputil is probably still desirable.

[njsmith]

@davidism Hey David, sorry I missed your message.

Is there an assumption that they won't make this assignable when they support 3.7 even though it's documented now?

They probably will eventually, since the CPython 3.7 test suite has tests for it, and PyPy will eventually import those tests when adding 3.7 support. But they might do it quicker if we hassle them :-)

When I talked to the PyPy devs about this originally, they told me they knew that jinja2 depends on the deprecated "transparent proxies" code, and they weren't planning to break jinja2, so even though it was deprecated I could expect at least this specific case to keep working. That's why I felt OK using it in Trio :-). But I'm sure they wouldn't mind if we all switched to something more sensible.

I'm currently fixing Jinja's traceback rewriting due to the changes in 3.7 and an issue with ctypes on debug builds (pallets/jinja#1050, pallets/jinja#1051, pallets/jinja#1104), and it would be nice if I can assume any 3.7 means tb_next is assignable. PyPy's docs for tproxy now list it as deprecated.

Oh yeah, uh... I actually fixed this already when I was cribbing from jinja2. I probably should have sent you a note at some point, sorry :-). It's actually very simple to fix, though it needs some explanation:

trio/trio/_core/_multierror.py

Lines 303 to 304 in ae557cc

	_fields_ = [
	("PyObject_HEAD", ctypes.c_byte * object().__sizeof__()),

The idea is that we know a Python object is just a bare PyObject_HEADER without any additional fields, so whatever size the object struct is, that's the size of a PyObject_HEADER. And if you don't care about accessing the fields inside it (refcount etc.), then you don't need to actually spell out those fields for ctypes; you just need a blob of "something" the right size so ctypes can figure out where the subsequent fields you actually care about are located. So if we tell ctypes that the first field is a byte array with object().__sizeof__() elements in it, then everything Just Works, on every Python version. And you even get to delete the gross hacks that try to guess how CPython is compiled by looking for sys.getobjects or whatever.

[mattip]

Ping @arigo

[davidism]

Thanks for the explanation. I was scared off by the use of _ctypes, so I ended up using py_object and pythonapi, since it was just as opaque to me as everything else, and at least seemed like the "intended" way. Here's the implementation I ended up with: https://github.com/pallets/jinja/blob/e08dadd220563609a67bc32d4d1c0abe91699231/src/jinja2/debug.py#L238. Support for 3.7+ and pypy is above that. I was able to greatly simplify the entire traceback rewriting implementation.