Ensure that held class temporaries get resolved over multiline expressions correctly.

Basically, we want to ensure that we can write expressions like

	listOfHeldClass[0].x = 10

and have the assignment modify the copy of the held class that lives within the list.
To do this, we need for listOfHeldClass[0] to return a RefTo(H) (where H is whatever
the list is holding). Unfortunately, that would mean that we have a reference to a held
class living in the python heap, which is very likely to cause a segfault, and which
doesn't mirror the semantics of the compiler.

The solution we adopted originally was to keep track of all held class temporaries
and to convert them to non-temporaries (e.g. make copies of the object) when the
current statement has finished executing.  This mirrors the way the compiler works:
temporaries exist for the duration of the expression we're compiling, which allows
chained sequences like this to work.

HOWEVER, the mechanism we use is to put a trace on the parent stackframe, and to
use that trace to discover when we've hit the next frame. Unfortunately, an expression like

	listOfHeldClass[0].f(
		10
	)

causes us to evaluate the '10' on a new line, which triggers the temporary to resolve. This
breaks the entire model.

This change causes us to cache the subset of line numbers that are the starts of new statements.
These line numbers are the ones where the references should be converted to concrete
instances of the held class.

Author: braxtonmckee

typed_python/PyTemporaryReferenceTracer.cpp

@@ -16,25 +16,68 @@
 
 #include "PyTemporaryReferenceTracer.hpp"
 
+bool PyTemporaryReferenceTracer::isLineNewStatement(PyObject* code, int line) {
+    auto it = codeObjectToExpressionLines.find(code);
+
+    if (it != codeObjectToExpressionLines.end()) {
+        return it->second.find(line) != it->second.end();
+    }
+
+    // this permanently memoizes this code object in this global object
+    // this should be OK because there are (usually) a small and finite number of
+    // code objects in a given program.
+    incref(code);
+    auto& lineNumbers = codeObjectToExpressionLines[code];
+
+    static PyObject* internals = internalsModule();
+
+    PyObjectStealer res(
+        PyObject_CallMethod(internals, "extractCodeObjectNewStatementLineNumbers", "O", code, NULL)
+    );
+
+    if (!res) {
+        PyErr_Clear();
+    } else {
+        iterate((PyObject*)res, [&](PyObject* lineNo) {
+            if (PyLong_Check(lineNo)) {
+                lineNumbers.insert(PyLong_AsLong(lineNo));
+            }
+        });
+    }
+
+    return lineNumbers.find(line) != lineNumbers.end();
+}
+
 
 int PyTemporaryReferenceTracer::globalTraceFun(PyObject* dummyObj, PyFrameObject* frame, int what, PyObject* arg) {
     if (frame != globalTracer.mostRecentEmptyFrame) {
-        auto it = globalTracer.frameToHandles.find(frame);
+        bool shouldProcess = true;
+
+        if (what == PyTrace_LINE) {
+            shouldProcess = globalTracer.isLineNewStatement(
+                (PyObject*)frame->f_code,
+                frame->f_lineno
+            );
+        }
+
+        if (shouldProcess) {
+            auto it = globalTracer.frameToHandles.find(frame);
 
-        if (it == globalTracer.frameToHandles.end()) {
-            globalTracer.mostRecentEmptyFrame = frame;
-        } else {
-            globalTracer.mostRecentEmptyFrame = nullptr;
+            if (it == globalTracer.frameToHandles.end()) {
+                globalTracer.mostRecentEmptyFrame = frame;
+            } else {
+                globalTracer.mostRecentEmptyFrame = nullptr;
 
-            for (auto objAndAction: it->second) {
-                if (objAndAction.second == TraceAction::ConvertTemporaryReference) {
-                    ((PyInstance*)objAndAction.first)->resolveTemporaryReference();
+                for (auto objAndAction: it->second) {
+                    if (objAndAction.second == TraceAction::ConvertTemporaryReference) {
+                        ((PyInstance*)objAndAction.first)->resolveTemporaryReference();
+                    }
+
+                    decref(objAndAction.first);
                 }
 
-                decref(objAndAction.first);
+                globalTracer.frameToHandles.erase(it);
             }
-
-            globalTracer.frameToHandles.erase(it);
         }
     }
 

typed_python/PyTemporaryReferenceTracer.hpp

@@ -34,13 +34,17 @@ class PyTemporaryReferenceTracer {
 
     std::unordered_map<PyFrameObject*, std::vector<std::pair<PyObject*, TraceAction> > > frameToHandles;
 
+    std::unordered_map<PyObject*, std::set<int> > codeObjectToExpressionLines;
+
     // the most recent frame we touched that has nothing in it
     PyFrameObject* mostRecentEmptyFrame;
 
     Py_tracefunc priorTraceFunc;
 
     PyObject* priorTraceFuncArg;
 
+    bool isLineNewStatement(PyObject* code, int line);
+
     static PyTemporaryReferenceTracer globalTracer;
 
     static int globalTraceFun(PyObject* obj, PyFrameObject* frame, int what, PyObject* arg);

typed_python/compiler/tests/held_class_interpreter_semantics_test.py

@@ -16,6 +16,9 @@ class H(Class, Final):
     def f(self):
         return self.x + self.y
 
+    def addToX(self, y):
+        return self.x + y
+
     def increment(self):
         self.x += 1
         self.y += 1
@@ -45,6 +48,26 @@ def runTest():
 
         assert runTest() == 30
 
+    def test_construct_and_call_method_multiline(self):
+        def runTest():
+            return H(x=10, y=20).addToX(
+                # this is deliberately on another line
+                10
+            )
+
+        assert runTest() == 20
+
+    def test_can_assign_to_held_class_in_list(self):
+        def runTest():
+            aList = ListOf(H)()
+            aList.resize(1)
+
+            aList[0].increment()
+
+            assert aList[0].x == 1
+
+        runTest()
+
     def test_list_of_held_class_item_type(self):
         assert sys.gettrace() is None
 

typed_python/internals.py

@@ -1,4 +1,4 @@
-#   Copyright 2017-2019 typed_python Authors
+#   Copyright 2017-2022 typed_python Authors
 #
 #   Licensed under the Apache License, Version 2.0 (the "License");
 #   you may not use this file except in compliance with the License.
@@ -622,3 +622,25 @@ def Held(T):
         raise Exception(f"{T} is not a Class")
 
     return T.HeldClass
+
+
+def extractCodeObjectNewStatementLineNumbers(codeObject):
+    """Return the subset of the line numbers on which codeObject has new statements."""
+    from typed_python.python_ast import convertFunctionToAlgebraicPyAst
+
+    try:
+        ast = convertFunctionToAlgebraicPyAst(codeObject)
+
+        res = set()
+
+        if ast.matches.FunctionDef:
+            for statement in ast.body:
+                res.add(statement.line_number)
+
+        return res
+    except Exception:
+        # nasty to swallow the exception like this. At least we report it...
+        import traceback
+        traceback.print_exc()
+
+        return []