[GiNaC-devel] remove_dirac_ONE() and documentation patches

[Alexey Sheplyakov]

Hi,

23.05.2019, 10:44, "Vladimir V. Kisil" <kisilv at maths.leeds.ac.uk>:
>         Dear Alexey,
>
>         I am attaching another iteration of the patch as suggested. The
>   exception-free method is used internally for a better protection.

Firstly making two almost identical copies of remove_dirac_ONE is definitely a bad idea.
Two possible approaches are:

a) wrap exception-throwing remove_dirac_ONE like this

ex remove_dirac_ONE(const ex& e, bool& success, unsigned char rl, unsigned options) {
         ex ret;
         try {
                ret = remove_dirac_ONE(e, rl, options);
         } catch (...) {
                success = false;
                ret = _ex0;
         }
         return ret;
}

This way exceptions will propagate within ginac.dll itself only, and there's no code duplication.

b) alternatively one could move the actual computation to non-throwing variant of remove_dirac_ONE,
and make a wrapper which does throw exceptions:

ex remove_dirac_ONE(const ex& e, unsigned char rl, unsigned options) {
    bool success = true;
    ex ret = remove_dirac_ONE(e, success, rl, options);
    if (!success) {
        // perhaps non-throwing variant should set an integer status instead of a bool flag, so a more specific exception can be thrown
        throw std::runtime_error("some meaningful error message here");
    }
     return ret;
}

Secondly I don't quite get what's the point of replacing the throwing variant of remove_dirac_ONE,
and throwing the very same exception manually:

> +	bool success;
> +
>  	if (! ex_to<idx>(mu).is_dim_numeric())
>  		throw(std::invalid_argument("clifford_to_lst(): index should have a numeric dimension"));
>  	unsigned int D = ex_to<numeric>(ex_to<idx>(mu).get_dim()).to_int();
> @@ -1341,7 +1399,9 @@ lst clifford_to_lst(const ex & e, const ex & c, bool algebraic)
>  				|| (! is_a<numeric>(pow(c.subs(mu == i, subs_options::no_pattern), 2))))
>  				algebraic = false;
>  	lst V; 
> -	ex v0 = remove_dirac_ONE(canonicalize_clifford(e+clifford_prime(e)))/2;
> +	ex v0 = remove_dirac_ONE(canonicalize_clifford(e+clifford_prime(e)), success)/2;
> +	if (!success)
> +		throw(std::invalid_argument("clifford_to_lst(): the argument is not a Clifford vector"));

I.e. why can't we keep calling

ex v0 = remove_dirac_ONE(canonicalize_clifford(e+clifford_prime(e)))/2;

and have remove_dirac_ONE throw an appropriate exception?

> Previously remote_dirac_ONE() reported that an expression is not a Clifford scalar by throwing an exception.
> This produced crashes in Qt applications on Windows.

Not really. Throwing exceptions across shared libraries (DLLs) on Windows is a little bit tricky, but it's certainly possible.
The crucial point is to make all DLLs use the same C++ runtime (which implies the C++ runtime should be linked
dynamically into all DLLs linked into the application).

Also patching just remove_dirac_ONE is sort of pointless, since most GiNaC methods and functions throw exceptions too.

Best regards,
       Alexey