tschoonj / xraylib

A library for X-ray matter interaction cross sections for X-ray fluorescence applications
https://github.com/tschoonj/xraylib/wiki
Other
120 stars 54 forks source link

C++: write proper bindings #139

Closed tschoonj closed 3 years ago

lgtm-com[bot] commented 3 years ago

This pull request introduces 1 alert when merging 9ac4f202f6527a11a12186ddad7a0f277f2e493f into 327f2e0d041c6f5f2ace294ed01587880e41c6b8 - view on LGTM.com

new alerts:

tschoonj commented 3 years ago

@mschollmeier what do you think about these C++ bindings?

mschollmeier commented 3 years ago

@tschoonj That's very close to what I had started, apologies for not finding the time to work on that. Thank you for doing the work! It looks like you've decided not to use a Python script to generate the c++ code. How come?

I have a few more error codes in process_error, which you could consider adding:

/* process_error translates error codes from xraylib to c++
* and throws the corresponding std exception.
* c++ reference: https://en.cppreference.com/w/cpp/error/exception
* and https://stackoverflow.com/questions/11938979/what-exception-classes-are-in-the-standard-c-library
*/
void process_error(xrl_error *error, std::string funcName) {

    if (!error)
        return;

    switch (error->code) {

    case XRL_ERROR_MEMORY: /* set in case of a memory allocation problem */
        throw std::bad_alloc();
    case XRL_ERROR_INVALID_ARGUMENT: /* set in case an invalid argument gets passed to a routine */
        throw std::invalid_argument(funcName + error->message);
    case XRL_ERROR_IO: /* set in case an error involving input/output occurred */
        throw std::ios_base::failure(error->message);
    case XRL_ERROR_TYPE: /* set in case an error involving type conversion occurred */
        throw std::bad_cast();
    case XRL_ERROR_UNSUPPORTED: /* set in case an unsupported feature has been requested */
        throw std::invalid_argument(funcName + error->message);
    case XRL_ERROR_RUNTIME:
        throw std::runtime_error(error->message); /* set in case an unexpected runtime error occurred */
    }
}
tschoonj commented 3 years ago

Glad to read you like it!

What about the classes I introduced? Do you think that this is sensible?

I decided to skip the Python script and use macros instead, mostly because I thought it was simpler to have just one header file compared to having three files (one handwritten header, one generated header, one python script)...

Unless you have any objections I will merge this PR in. Feel free to then open a PR to modify the error code handling function.

mschollmeier commented 3 years ago

Hi Tom, I've started to take a closer look at the bindings. I've noticed that you can replace the various _XRL_FUNCTION_nxx macros with just one macro, using a variadic template function:

#define _XRL_FUNCTION(_name) \
    template<typename... T> \
    double _name(const T... args) { \
    xrl_error *error = nullptr; \
    double rv = ::_name(args..., &error); \
    _process_error(error); \
    return rv; \
    }

This compiles without complaints. Spot checks for a few functions yield the correct results, I'll do more testing and if successful submit a pull request.

tschoonj commented 3 years ago

Hi Marius,

I have merged this PR in: please base your changes on the updated master branch. That way the C++ tests can be reused and will be run as part of the CI.