wlav / cppyy

Other
384 stars 38 forks source link

Exception missing for compile error triggered by call to C++ function #243

Open marktsuchida opened 1 month ago

marktsuchida commented 1 month ago

In the following test program I make a call to a non-template member function of a class template. Template instantiation of this function causes a compile error. The error is correctly printed to stdout, but cppyy fails to raise any exception.

repro.py:

import cppyy

cppyy.include("string")
cppyy.include("type_traits")

cppyy.cppdef("""
template <typename Allowed> struct handler {
    template <typename Event,
              typename = std::enable_if_t<std::is_same_v<Event, Allowed>>>
    void handle(Event const &event) {}
};

template <typename Handler> struct wrapper {
    Handler handler;

    explicit wrapper(Handler handler) : handler(handler) {}

    void handle(std::string const &event) {
        handler.handle(event);  // Error with Handler = handler<int>
    }
};

auto make_wrapped() {
    return wrapper(handler<int>());
}
""")

w = cppyy.gbl.make_wrapped()  # OK, handle() template not instantiated yet

print("Calling handle(std::string const &) on wrapper")

# Expected: SyntaxError (or some exception)
# Actual: No exception; yet prints the compile error to stderr
r = w.handle("abc")

print("Did not raise exception")
$ python repro.py
Calling handle(std::string const &) on wrapper
input_line_20:14:17: error: no matching member function for call to 'handle'
        handler.handle(event);  // Error with Handler = handler<int>
        ~~~~~~~~^~~~~~
input_line_22:6:35: note: in instantiation of member function 'wrapper<handler<int>>::handle' requested here
   ((wrapper<handler<int>>*)obj)->handle((const std::string&)*(const std::string*)args[0]);
                                  ^
input_line_20:5:10: note: candidate template ignored: requirement 'std::is_same_v<std::string, int>' was not satisfied [with Event = std::string]
    void handle(Event const &event) {}
         ^
Did not raise exception

cppyy 3.1.2, macOS arm64 or x86-64.

(The fact that there is no immediate error from the cppdef() part, and that the call to wrapper<...>::handle() is what triggers the compile error, matches the behavior of Clang 13 (Compiler Explorer) as well as other compilers. The only problem here is that there is no way for the Python code to be aware of the error.)

wlav commented 1 month ago

Interesting one ... the printout can't be helped, that's a Cling problem of not providing callbacks on errors. Even cppdef simply captures stderr. :/

Here, the issue seems to be that in order to detect failure, the return result needs to be checked. However, since the method returns void no result is expected and it's thus not checked.

wlav commented 1 month ago

Yes, was b/c void returns were a separate implementation from non-void returns. Now fixed in repo: https://github.com/wlav/cppyy-backend/commit/9611a92593ab4b1e95faad4d44908c2caa739862 .

marktsuchida commented 1 month ago

Thanks so much for fixing! I ran into this while creating my workaround for #242 (essentially wrapping my function templates using a non-template overload set), so it's a relief that this will work in the next release.

marktsuchida commented 4 weeks ago

On thinking about this further, it occurs to me that throwing std::runtime_error in this case (as well as the analogous case with a non-void return type) is slightly problematic in that there is no way to know if the exception indicates a failure to call the C++ function at all (usually suggesting a programming error) as opposed to a successful call in which the C++ function itself happened to throw std::runtime_error (usually a genuine runtime error). Not sure how easy this would be but it would be nice if the former case would result in a Python exception such as SyntaxError, or at least a specific (custom) derived class of std::runtime_error. Not as critical as the fix you already made, but thought I'd mention this.

wlav commented 4 weeks ago

The code in clingwrapper.cxx shouldn't depend on Python, so it can't set a SyntaxError. Moreover, as said above, there is no information programmatically available from Cling as to what the error was, but the most common case is missing linker symbols, hence the choice for runtime_error rather than claiming a syntax error. Making the exception a more obvious derived class is possible, but for it to then be visible on the Python side as said derived type, it needs to be exposed to the JIT, which is slightly more convoluted than just changing clingwrapper.cxx. (I wish I could just JIT it, but that doesn't play nice on Windows; see the test suite.) Can look into that.

marktsuchida commented 3 weeks ago

Difficult situation, I see. Thanks for the explanation.