wlav / cppyy

Other
400 stars 41 forks source link

method to reload modified c++ code #54

Closed xayjin closed 2 years ago

xayjin commented 2 years ago

hi, cppyy is a wonderful library! I always modify c++ code in repl using cppyy, but cppyy can't reload modified c++ code. I find cppyy get a exist class in the map "gPyClasses" as a cache. So I think cppyy need three steps to reload the modified c++ code:

  1. erase the class in the map "gPyClasses"
  2. delete the class in parent ,such as cppyy.gbl
  3. cppexec the code,and use cppyy.gbl to CreateScopeProxy

I fork and add a simple function to act. [CPyCppyy] src/ProxyWrappers.cxx

PyObject* CPyCppyy::DelScopeProxy(PyObject*, PyObject* args)
{
    if (PyTuple_Size(args) < 2){
        std::cout<<"Need two args:nameString parentObject"<<std::endl;
        Py_RETURN_FALSE;
    }
    std::string name = CPyCppyy_PyText_AsString(PyTuple_GetItem(args, 0));

    if (PyErr_Occurred())
            Py_RETURN_FALSE;
    Cppyy::TCppScope_t klass = Cppyy::GetScope(name);

    if(!(bool)klass){
        Py_RETURN_FALSE;
    }
    PyClassMap_t::iterator pci = gPyClasses.find(klass);
    if (pci != gPyClasses.end()) {
        gPyClasses.erase(klass);
            std::string::size_type last = 0;
        PyObject* parent=PyTuple_GetItem(args, 1);

        std::string unscoped = name.substr(last, std::string::npos);
        PyObject_DelAttrString(parent, name.c_str());
    }
    Py_RETURN_TRUE;
}

hopely to help.

wlav commented 2 years ago

Yes, that is one issue, but the true problem is deeper: Cling can not reload C++ functions (nor classes; it can redefine variables, though). Will be fixed only when upstream moves to OrcJITv2 (although that is not seeing and Windows development currently, so there may be some more hold-ups before cppyy can adapt to it). I'm talking later today with upstream and will ask again on the current status as this is a recurring request.

That said, Python is more flexible. If you can version or namespace classes and/or methods, you can use those to rename/modify the Python ones.

xayjin commented 2 years ago

Hi, but I find that redefinition is ok in Cling Interpreter. cppyy.cppexec use the Interpreter to ProcessLine. Here is the test in cppyy:

In [12]: cppyy.cppexec("""void print_test(){std::cout<<"print1"<<std::endl;}""")
Out[12]: True

In [13]: cppyy.cppexec("""print_test();""")
print1
Out[13]: True

In [14]: cppyy.cppexec("""void print_test(){std::cout<<"print2"<<std::endl;}""")
Out[14]: True

In [15]: cppyy.cppexec("""print_test();""")
print2
Out[15]: True

Test in Root 6.20

   ------------------------------------------------------------------
  | Welcome to ROOT 6.20/04                        https://root.cern |
  | (c) 1995-2020, The ROOT Team; conception: R. Brun, F. Rademakers |
  | Built for linuxx8664gcc on May 13 2020, 05:33:44                 |
  | From tags/v6-20-04@v6-20-04                                      |
  | Try '.help', '.demo', '.license', '.credits', '.quit'/'.q'       |
   ------------------------------------------------------------------

root [0] void print(){printf("a\n");}
root [1] print()
a
root [2] void print(){printf("b\n");}
root [3] print()
b

That means print function is overrided.

wlav commented 2 years ago

You're right, that does work. But no, cppexec doesn't use ProcessLine(), which seems to make the difference: cppdef() uses ProcessLine(), which does not work, and I based my conclusions off that (as well as earlier discussions). I'll still bring it up later with upstream, to see what limitations there are.

Anyway, cool! Let me see how far I can push this. Python's reload() may be too much to ask (depending on what happens with shared libraries), but making __del__ work should be doable.

wlav commented 2 years ago

It also doesn't work in namespaces (with cppexec).

wlav commented 2 years ago

And yes, after discussing with upstream, this is a ROOT feature, not a Cling feature (cppyy still carries ROOT legacy code, enabling this).

The way it works is basically that both methods are kept around in inline namespaces (this is why it doesn't work if the function lives in an actual namespace), with the last definition replacing the previous one in the global symbol lookup table so that subsequent compilation picks the new one. This is indeed quite a flaky approach (it effectively makes Clang's internal state inconsistent) except for specific cases as in the example above, i.e. if use is isolated.

Nevertheless, I'm interested in following the white rabbit and see how far it can be pushed. There are already a few other features that are flaky (e.g. from cppyy.interactive import * or cppyy.ll.signals_as_exception()) and still useful in confined settings.

xayjin commented 2 years ago

Appreciate your reply.Yes, redefinition is not allowed in namespaces except in the global scope.

xayjin commented 2 years ago

I find modified function was treated as overloads, not redefinition. That means Cppyy::GetMethodIndicesFromName from the global scope return all the modified functions. So I have a sugguestion: replace CPPScope.cxx row 371 overloads.push_back( by overloads.insert(overloads.begin(),. After that, the new modified function will have priority. By the way, delattr(cppyy.gbl,"xxx") is needed to relaod from cling backend.

xayjin commented 2 years ago

My tricks are not stable. Thanks for your wonderful work.

wlav commented 2 years ago

Yes and no to the point about overloads. What is going on, is that the compiled stub function is cached in cppyy-backend (see https://github.com/wlav/cppyy-backend/blob/master/clingwrapper/src/clingwrapper.cxx#L833). Wipe that, and the same pre-existing overload will recreate a stub, causing a recompilation, new lookup, finding the new call and it will work from there.

So basically __del__ needs to propagate all the way down to the backend. For cleanliness, it may make sense to delete the intermediate stuff (although finding two overloads isn't a good thing), but it doesn't need to.