Add a callback to FuncJUMP_TO_CATCH #1690
Conversation
markuspf
added
the
kind: enhancement
src/gapstate.h
Outdated
| @@ -148,6 +148,7 @@ typedef struct GAPState { | |||
| Obj ErrorLVars0; // the initial ErrorLVars value, i.e. for the lvars were the break occurred | |||
| Obj ErrorLVars; // ErrorLVars as modified by DownEnv / UpEnv | |||
| Int ErrorLLevel; // record where on the stack ErrorLVars is relative to the top, i.e. ErrorLVars0 | |||
| void (*JumpToCatchFunc)(); // Error catching function for GAP shared library | |||
There was a problem hiding this comment.
This JumpToCatchFunc is a bit mysterious, and of course currently never set. And this comment doesn't really help ("shared library? what shared library?")
Perhaps "Callback invoked by FuncJUMP_TO_CATCH" would be more helpful here, and then a somewhat longer explanation in "FuncJUMP_TO_CATCH", like: "Optionally invoke a callback just before we jump. This is not used by GAP itself, but exists for use in code linking against GAP."
Codecov Report
@@ Coverage Diff @@
## master #1690 +/- ##
==========================================
- Coverage 69.72% 66.39% -3.33%
==========================================
Files 482 569 +87
Lines 254511 310151 +55640
Branches 0 12029 +12029
==========================================
+ Hits 177453 205928 +28475
- Misses 77058 101427 +24369
- Partials 0 2796 +2796
|
olexandr-konovalov
added
the
gapdays2017-fall
|
Not ready. |
markuspf
force-pushed
the
jump-to-catch
branch
2 times, most recently
from
2186e56 to
6d84bca
Compare
|
As part of an effort with @sebasguts to get Added a longer comment about what the added callback function is supposed to do. I am not sure this is the correct way to handle errors for a library in the long run, but at least for now this works (and is compatible with whatever Sage wants to do). If anyone has a better idea (that ideally does not require a complete rewrite of error handling right now), I am all ears. |
markuspf
requested review from
fingolfin and
ChrisJefferson
and removed request for
ChrisJefferson
There was a problem hiding this comment.
I don't mind this much, now that there is a comment explaining what it's for; and with the implicit understanding that we just keep this around for the time being, because it is pragmatic, but that if we ever refactor error handling, we can replace it with something better (if we coordinate this with SageMath, it shouldn't hurt them either, after all).
src/gap.c
Outdated
| } | ||
|
|
||
| /* TL: UInt UserHasQuit; */ | ||
| /* TL: UInt UserHasQUIT; */ | ||
| UInt SystemErrorCode; |
There was a problem hiding this comment.
These three lines look like a merge artifact -- remove?
src/gapstate.h
Outdated
| @@ -123,6 +123,9 @@ typedef struct GAPState { | |||
| Obj BaseShellContext; | |||
| Obj ErrorLVars; // ErrorLVars as modified by DownEnv / UpEnv | |||
| Int ErrorLLevel; // record where on the stack ErrorLVars is relative to the top, i.e. BaseShellContext | |||
| void (*JumpToCatchFunc)(); // Callback that is called in FuncCALL_TO_CATCH, | |||
There was a problem hiding this comment.
It is called in FuncJUMP_TO_CATCH, not CALL_...
Would you mind renaming the callback to JumpToCatchCallback? I find that slightly more helpful when reading the code. But it's probably subjective, so shrug
markuspf
force-pushed
the
jump-to-catch
branch
2 times, most recently
from
fe65fd3 to
ed3047d
Compare
This is another bit of the GAP dynamic library code. Again without tests and without a guarantee that this will work in HPC-GAP
markuspf
force-pushed
the
jump-to-catch
branch
from
ed3047d to
80cc26f
Compare
fingolfin
added
the
release notes: added
This is another bit of the GAP dynamic library code. Again without tests
and without a guarantee that this will work in HPC-GAP