root-project / root

The official repository for ROOT: analyzing, storing and visualizing big data, scientifically
https://root.cern
Other
2.64k stars 1.26k forks source link

cppyy (pyroot) incorrectly claiming TypeError from overloaded methods that throw exceptions #16405

Open will-cern opened 1 week ago

will-cern commented 1 week ago

Check duplicate issues.

Description

I have a class with some overloaded methods that are designed to throw exceptions if the user messes up. In non-overloaded methods this exception is passed back to the user, exactly as expected. But when the method is overloaded, cppyy seems to assume that the failure was a TypeError issue rather than returning the underlying exception. Below is a reproducer.

Reproducer

import ROOT

ROOT.gInterpreter.ProcessLine("""
class MyClass {
public:
    class MyObj {
    public:
        MyObj(const char*) { }
    };
    void MyMethod(const MyObj& x = "") {
      throw std::runtime_error("My exception");
    }
    void MyMethod(const MyObj& x, bool another) {
      throw std::runtime_error("My second exception");
    }

};
""")
MyClass().MyMethod("hi")

Produces:

TypeError: none of the 2 overloaded methods succeeded. Full details:
  void MyClass::MyMethod(const MyClass::MyObj& x, bool another) =>
    TypeError: takes at least 2 arguments (1 given)
  void MyClass::MyMethod(const MyClass::MyObj& x = "") =>
    TypeError: could not convert argument 1

The issue is the second exception, since there isn't a conversion error here. Pyroot should report the exception (runtime_error("My Exception")) here instead.

ROOT version

6.32

Installation method

source

Operating system

all

Additional context

No response

guitargeek commented 1 week ago

Hi @will-cern! Just to double check with you before closing this issue as duplicate: this is the same as #9909, right? Coincidentally, reported by Carsten :slightly_smiling_face:

In case it is really a duplicate, please close this issue here and make a comment in Carstens issue, so that it's documented that more than one person cares about it and we can bump the priority and have a discussion at a central place.

will-cern commented 1 week ago

Actually I think it's different to that issue. In the other issue he wanted pyROOT to throw one of the exceptions, but it at least is telling the user what the exception types were from each of the c++ methods it tried. In my case, I'm unhappy that pyROOT seems to be hiding the exceptions that c++ threw behind a TypeError, rather than saying the correct exception type (which was a runtime error, and ideally to print the message associated to that).

will-cern commented 1 week ago

Maybe to be clearer, I would expect the output to be:

TypeError: none of the 2 overloaded methods succeeded. Full details:
  void MyClass::MyMethod(const MyClass::MyObj& x, bool another) =>
    TypeError: takes at least 2 arguments (1 given)
  void MyClass::MyMethod(const MyClass::MyObj& x = "") =>
    RuntimeError: My exception

I.e. the second method didn't fail because of a conversion type error, it failed because the method threw a runtime exception