oscar-system / GAP.jl

GAP packages for Julia integration
https://oscar-system.github.io/GAP.jl/
GNU Lesser General Public License v3.0
58 stars 20 forks source link

`GAP.evalstr` hides indirect error messages #896

Open fingolfin opened 1 year ago

fingolfin commented 1 year ago

Related to issue #895, GAP.evalstr also seems to hide error messages in some cases. It works if the error is reported by one of the evaluated expressions:

julia> GAP.evalstr("1/0")
ERROR: Error thrown by GAP: Error, Rational operations: <divisor> must not be zero
not in any function at stream:1

But now put 1/0; into a file, say foo.g, and let GAP's Read read that:

julia> GAP.evalstr("Read(\"foo.g\");")

julia>

Then no error is reported. Because our error handler eats up the error message; and the hack in evalstr to print the error anyway is not triggered in this case, because Read itself here look as if it "succeeded" based on the information available to evalstr.

One way to address this might be to remove the if any(x->x[1] == false, res) check from evalstr. But I am not sure if this might not have other, undesired consequences? In any case, it seems we should add more tests for evalstr to lock down the desired behavior...

BTW the above issue can be completely avoided by using GAP.Globals.Read(g"foo.g").

fingolfin commented 1 year ago

Oh of course my hack doesn't work in general because it always calls error, which I guess is not what we want :joy:. But my point more was that the error is there, we just need to detect it properly and print it.

ThomasBreuer commented 1 year ago

When I put the command 1/0; into the file foo.g and call GAP.evalstr_ex("Read(\"foo.g\");") then I get GAP: [ [ true,, false,, "" ] ]. The true means that the statement was successful. If I understand this right then the problem is that libgap's GAP_EvalString does not notice the error.

fingolfin commented 1 year ago

Here's what I would suggest we try (even if it is not perfect): modify the check in evalstr which currently does if any(x->x[1] == false, res) and instead just check if the error output buffer is non-empty.