Skip to content

Commit

Permalink
Make the assertion level thread local
Browse files Browse the repository at this point in the history
... by moving CurrentAssertionLevel to the kernel, into the GAPState.

While at it, also validate the level argument given to Assert(), and error out
if it is not a non-negative small integer.

While no code should have ever directly accessed CurrentAssertionLevel, there
is a minimal chance that some legacy code somewhere does this. To make sure it
suddenly runs with an incorrect assertion level without anybody noticing, we
keep defining CurrentAssertionLevel, but as a constant bound to `false`. This
way, any code trying to modify it, or to compare it against an integer, will
run into an error.
  • Loading branch information
fingolfin committed Jan 2, 2020
1 parent 4365d61 commit c17e62f
Show file tree
Hide file tree
Showing 14 changed files with 79 additions and 159 deletions.
44 changes: 0 additions & 44 deletions lib/assert.gd

This file was deleted.

46 changes: 0 additions & 46 deletions lib/assert.gi

This file was deleted.

2 changes: 0 additions & 2 deletions lib/read1.g
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ ReadLib( "record.gd" );
ReadLib( "random.gd" );

ReadLib( "cache.gi" );
ReadLib( "assert.gd" );
ReadLib( "coll.gi" );

ReadLib( "flag.g" );
Expand Down Expand Up @@ -80,7 +79,6 @@ ReadLib( "mat8bit.gd" );
ReadLib( "global.gd" );

ReadLib( "info.gi" );
ReadLib( "assert.gi" );
ReadLib( "global.gi" );

ReadLib( "options.gd" );
Expand Down
4 changes: 2 additions & 2 deletions src/compiler.c
Original file line number Diff line number Diff line change
Expand Up @@ -4971,7 +4971,7 @@ static void CompAssert2(Stat stat)

Emit( "\n/* Assert( ... ); */\n" );
lev = CompExpr(READ_STAT(stat, 0));
Emit( "if ( ! LT(CurrentAssertionLevel, %c) ) {\n", lev );
Emit( "if ( STATE(CurrentAssertionLevel) >= %i ) {\n", lev );
cnd = CompBoolExpr(READ_STAT(stat, 1));
Emit( "if ( ! %c ) {\n", cnd );
Emit( "AssertionFailure();\n" );
Expand All @@ -4996,7 +4996,7 @@ static void CompAssert3(Stat stat)

Emit( "\n/* Assert( ... ); */\n" );
lev = CompExpr(READ_STAT(stat, 0));
Emit( "if ( ! LT(CurrentAssertionLevel, %c) ) {\n", lev );
Emit( "if ( STATE(CurrentAssertionLevel) >= %i ) {\n", lev );
cnd = CompBoolExpr(READ_STAT(stat, 1));
Emit( "if ( ! %c ) {\n", cnd );
msg = CompExpr(READ_STAT(stat, 2));
Expand Down
24 changes: 23 additions & 1 deletion src/gap.c
Original file line number Diff line number Diff line change
Expand Up @@ -1377,6 +1377,20 @@ static Obj FuncUPDATE_STAT(Obj self, Obj name, Obj newStat)
}


static Obj FuncSetAssertionLevel(Obj self, Obj level)
{
RequireNonnegativeSmallInt("SetAssertionLevel", level);
STATE(CurrentAssertionLevel) = INT_INTOBJ(level);
return 0;
}


static Obj FuncAssertionLevel(Obj self)
{
return INTOBJ_INT(STATE(CurrentAssertionLevel));
}


/****************************************************************************
**
*V GVarFuncs . . . . . . . . . . . . . . . . . . list of functions to export
Expand Down Expand Up @@ -1423,6 +1437,10 @@ static StructGVarFunc GVarFuncs[] = {
GVAR_FUNC_1ARGS(MASTER_POINTER_NUMBER, ob),
GVAR_FUNC_1ARGS(BREAKPOINT, integer),
GVAR_FUNC_2ARGS(UPDATE_STAT, string, object),

GVAR_FUNC_1ARGS(SetAssertionLevel, level),
GVAR_FUNC_0ARGS(AssertionLevel),

{ 0, 0, 0, 0, 0 }

};
Expand Down Expand Up @@ -1474,7 +1492,7 @@ static Int PostRestore (
MemoryAllocated = GVarName( "memory_allocated" );

QUITTINGGVar = GVarName( "QUITTING" );

return 0;
}

Expand Down Expand Up @@ -1505,6 +1523,10 @@ static Int InitLibrary (
AssReadOnlyGVar(GVarName("time"), INTOBJ_INT(0));
AssReadOnlyGVar(GVarName("memory_allocated"), INTOBJ_INT(0));

// ensure any legacy code which directly tries to set the former GAP
// global 'CurrentAssertionLevel' or tries to compare to an integer,
// runs into an error.
AssConstantGVar(GVarName("CurrentAssertionLevel"), False);

return PostRestore( module );
}
Expand Down
3 changes: 3 additions & 0 deletions src/gapstate.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ typedef struct GAPState {
Obj StackObj;
Obj Tilde;

// The current assertion level for use in Assert
Int CurrentAssertionLevel;

/* From gvar.c */
Obj CurrNamespace;

Expand Down
30 changes: 3 additions & 27 deletions src/intrprtr.c
Original file line number Diff line number Diff line change
Expand Up @@ -3808,16 +3808,12 @@ void IntrInfoEnd( UInt narg )
*F IntrAssertEnd2Args() . . . . called after reading the closing parenthesis
*F IntrAssertEnd3Args() . . . . called after reading the closing parenthesis
**
*V CurrentAssertionLevel . . . . . . . . . . . . . . copy of GAP variable
**
**
** STATE(IntrIgnoring) is increased by (a total of) 2 if an assertion either
** is not tested (because we were Ignoring when we got to it, or due to
** level) or is tested and passes
*/

Obj CurrentAssertionLevel;

void IntrAssertBegin ( void )
{
/* ignore or code */
Expand All @@ -3830,18 +3826,16 @@ void IntrAssertBegin ( void )

void IntrAssertAfterLevel ( void )
{
Obj level;

/* ignore or code */
SKIP_IF_RETURNING();
if ( STATE(IntrIgnoring) > 0 ) { STATE(IntrIgnoring)++; return; }
if ( STATE(IntrCoding) > 0 ) { CodeAssertAfterLevel(); return; }


level = PopObj();
Int level = GetSmallIntEx("Assert", PopObj(), "<lev>");

if (LT( CurrentAssertionLevel, level))
STATE(IntrIgnoring) = 1;
if (STATE(CurrentAssertionLevel) < level)
STATE(IntrIgnoring) = 1;
}

void IntrAssertAfterCondition ( void )
Expand Down Expand Up @@ -3934,7 +3928,6 @@ static Int InitKernel (
InitGlobalBag( &STATE(Tilde), "STATE(Tilde)" );
#endif

InitCopyGVar( "CurrentAssertionLevel", &CurrentAssertionLevel );
InitFopyGVar( "CONVERT_FLOAT_LITERAL_EAGER", &CONVERT_FLOAT_LITERAL_EAGER);

/* The work of handling Options is also delegated*/
Expand All @@ -3945,22 +3938,6 @@ static Int InitKernel (
}


/****************************************************************************
**
*F InitLibrary( <module> ) . . . . . . . initialise library data structures
*/
static Int InitLibrary (
StructInitInfo * module )
{
UInt lev;

/* The Assertion level is also controlled at GAP level */
lev = GVarName("CurrentAssertionLevel");
AssGVar( lev, INTOBJ_INT(0) );

return 0;
}

static Int InitModuleState(void)
{
STATE(IntrCoding) = 0;
Expand All @@ -3981,7 +3958,6 @@ static StructInitInfo module = {
.type = MODULE_BUILTIN,
.name = "intrprtr",
.initKernel = InitKernel,
.initLibrary = InitLibrary,

.initModuleState = InitModuleState,
};
Expand Down
4 changes: 0 additions & 4 deletions src/intrprtr.h
Original file line number Diff line number Diff line change
Expand Up @@ -859,8 +859,6 @@ void IntrInfoEnd(UInt narg);
**
*F IntrAssertEnd2Args() . . . . called after reading the closing parenthesis
*F IntrAssertEnd3Args() . . . . called after reading the closing parenthesis
**
*V CurrentAssertionLevel . . . . . . . . . . . . . . . copy of GAP variable
*/

void IntrAssertBegin(void);
Expand All @@ -869,8 +867,6 @@ void IntrAssertAfterCondition(void);
void IntrAssertEnd2Args(void);
void IntrAssertEnd3Args(void);

extern Obj CurrentAssertionLevel;

/****************************************************************************
**
*F IntrSaveWSBegin() . . . . . . . . . . . . . Start interpreting a save WS
Expand Down
10 changes: 8 additions & 2 deletions src/stats.c
Original file line number Diff line number Diff line change
Expand Up @@ -892,10 +892,13 @@ static UInt ExecInfo(Stat stat)
static UInt ExecAssert2Args(Stat stat)
{
Obj level;
Int lev;
Obj cond;

level = EVAL_EXPR(READ_STAT(stat, 0));
if ( ! LT(CurrentAssertionLevel, level) ) {
lev = GetSmallIntEx("Assert", level, "<lev>");

if (STATE(CurrentAssertionLevel) >= lev) {
cond = EVAL_EXPR(READ_STAT(stat, 1));
RequireTrueOrFalse("Assert", cond);
if (cond == False) {
Expand All @@ -917,11 +920,14 @@ static UInt ExecAssert2Args(Stat stat)
static UInt ExecAssert3Args(Stat stat)
{
Obj level;
Int lev;
Obj cond;
Obj message;

level = EVAL_EXPR(READ_STAT(stat, 0));
if ( ! LT(CurrentAssertionLevel, level) ) {
lev = GetSmallIntEx("Assert", level, "<lev>");

if (STATE(CurrentAssertionLevel) >= lev) {
cond = EVAL_EXPR(READ_STAT(stat, 1));
RequireTrueOrFalse("Assert", cond);
if (cond == False) {
Expand Down
Loading

0 comments on commit c17e62f

Please sign in to comment.