wlav / cppyy

Other
384 stars 38 forks source link

`TypeError` instead of common exception type for overloaded functions #230

Open m-fila opened 2 months ago

m-fila commented 2 months ago

According to documentation in cases when all overloads of a functions throw the same exception that exception type should be reported by cppyy and only in cases when the overloads throw different exceptions then TypeError will be used. In the following snippet both overloads throw std.logic_error but TypeError is used instead:

>>> import cppyy
>>> cppyy.cppdef("""
...   class Foo {
...     public:
...     void bar() { throw std::logic_error("This is fine"); }
...     void bar() const { throw std::logic_error("This is fine"); } 
...   };
...   """)
True
>>> foo = cppyy.gbl.Foo()
>>> try:
...   foo.bar()
... except cppyy.gbl.std.logic_error:
...   pass
... 
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
TypeError: none of the 2 overloaded methods succeeded. Full details:
  void Foo::bar() =>
    logic_error: This is fine
  void Foo::bar() =>
    logic_error: This is fine
>>> 

The same problem occurs when the overloads differ by argument type (with implicit conversion) and are free functions. Is this expected?

In older cppyy (for instance 1.6.2) different behaviors is observed as std.logic_error is used:

>>> foo.bar()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
cppyy.gbl.std.logic_error: none of the 2 overloaded methods succeeded. Full details:
  void Foo::bar() =>
    logic_error: This is fine
  void Foo::bar() =>
    logic_error: This is fine
>>> 
wlav commented 2 months ago

I forget whether this is intentional ...

There have, over time, been different opinions on how to deal with C++ exceptions from methods and how they fit into overloads. The original motivation for including C++ exceptions into the overload mechanism is that they could originate from failed argument conversion attempts (e.g. disallowing integer to double promotion) and should thus be treated the same way as any Python -> C++ argument conversion attempts to select an appropriate overload. An important feature there is that these exceptions are based on the argument type and thus always behave in that way.

However, exceptions can also come from logic and control flow errors. In that case, the types of the exceptions could be based on the argument value and thus sometimes you would get one exception type, another time a mix. Hence using a TypeError in all cases would be more predictable.

The code as current is an attempt to balance these two: if a single C++ exception lurks between multiple overloads that failed with Python exceptions, it assumes that that C++ one originated from a logic error in a call that could have succeeded with a correct value and hence is the main, relevant, one that should be reported. (Note that all overloads are still tried; the difference is only in the reporting.) Otherwise, if there are multiple C++ exceptions, it is assumed that these are argument conversion errors, hence the TypeError.

But yes, I do think that although the above would then be considered an overload failure due to argument conversion error, it should still pick the single most narrow exception type if all overloads raise the same C++ exception type, simply for consistency as to what would happen if these were Python exceptions. Code is in repo.

Note that there remains a subtle difference in the behavior of constructors v.s. methods this way: the default copy or move constructor introduces an argument conversion exception in many cases where, causing the reported exception back to TypeError b/c of types being mixed if there is more than one C++ exception.