Skip to content

Commit

Permalink
Remove runtime strict rules
Browse files Browse the repository at this point in the history 
They don't improve safety, but hurt performance
  • Loading branch information
@VasiliyRyabtsev
VasiliyRyabtsev committed Oct 30, 2024
1 parent 11f7de6 commit ce89651
Show file tree
Hide file tree
Showing 5 changed files with 7 additions and 125 deletions.
68 changes: 0 additions & 68 deletions doc/source/reference/language/compiler_directives.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,42 +31,6 @@ Prefix directive with 'default:' to apply it to entire VM, otherwise directive a
List of Complier directives
=============================

----------------
Strict booleans
----------------

::

#strict-bool

Force booleans in conditional expressions, raise an error on non-boolean types.

Non-boolean in conditional expressions can lead to errors.
Because all programming languages with such feature behave differently, for example Python treats empty arrays and strings as false,
but JS doesn't; PHP convert logical expression to Boolean, etc.
Another reason is that due to dynamic nature of Quirrel those who read code can't know what exactly author meant with it.
::

if (some_var) //somevar can be null, 0, 0.0 or object

or

::

local foo = a() || b() && c //what author means? What will be foo?



----------------------------
Implicit bool expressions
----------------------------

::

#relaxed-bool

Original Squirrel 3.1 behavior (treat boolean false, null, 0 and 0.0 as false, all other values as true)


-----------------------------------------------
Disable access to root table via ``::``
Expand All @@ -89,38 +53,6 @@ Enable access to root table via ``::``

Allows use of ``::`` operator for the current unit

----------------------------------------------
Disable implicit string concatenation
----------------------------------------------

::

#no-plus-concat

Throws error on string concatenation with +.
It is slower for multple strings, use concat or join instead.
It is also not safe and can cause errors, as + is supposed to be at least Associative operation, and usually Commutative too.
But + for string concatenation is not associative, e.g.

Example:
::

let a = 1
let b = 2
let c = "3"
(a + b) + c != a + (b + c) // "33" != "123"

This actually happens especially on reduce of arrays and alike.

----------------------------------------------
Enable plus string concatenation
----------------------------------------------

::

#allow-plus-concat

Allow using plus operator '+' to concatenate strings.

----------------------------------------------
Clone operator
Expand Down
4 changes: 0 additions & 4 deletions squirrel/sqastparser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,6 @@ struct SQPragmaDescriptor {
static const SQPragmaDescriptor pragmaDescriptors[] = {
{ "strict", LF_STRICT, 0 },
{ "relaxed", 0, LF_STRICT },
{ "strict-bool", LF_STRICT_BOOL, 0 },
{ "relaxed-bool", 0, LF_STRICT_BOOL },
{ "no-plus-concat", LF_NO_PLUS_CONCAT, 0 },
{ "allow-plus-concat", 0, LF_NO_PLUS_CONCAT },
{ "forbid-root-table", LF_FORBID_ROOT_TABLE, 0 },
{ "allow-root-table", 0, LF_FORBID_ROOT_TABLE },
{ "disable-optimizer", LF_DISABLE_OPTIMIZER, 0 },
Expand Down
8 changes: 1 addition & 7 deletions squirrel/sqfuncproto.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,7 @@ enum SQLangFeature {
LF_FORBID_GLOBAL_CONST_REWRITE = 0x000400,
LF_FORBID_IMPLICIT_DEF_DELEGATE = 0x000800,

// runtime stage
LF_STRICT_BOOL = 0x010000,
LF_NO_PLUS_CONCAT = 0x020000,

LF_STRICT = LF_STRICT_BOOL |
LF_NO_PLUS_CONCAT |
LF_FORBID_ROOT_TABLE
LF_STRICT = LF_FORBID_ROOT_TABLE
};

struct SQOuterVar
Expand Down
48 changes: 6 additions & 42 deletions squirrel/sqvm.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -748,27 +748,6 @@ bool SQVM::IsFalse(const SQObjectPtr &o)
return false;
}

SQVM::BooleanResult SQVM::ResolveBooleanResult(const SQObjectPtr &o)
{
if (sq_type(o) != OT_BOOL)
return IsFalse(o) ? LEGACY_FALSE : LEGACY_TRUE;
return (_integer(o) == 0) ? BOOL_FALSE : BOOL_TRUE;
}

#define VALIDATE_BOOLEAN_RESULT(r, o) {\
if ((_closure(ci->_closure)->_function->lang_features & LF_STRICT_BOOL) && IsBooleanResultLegacy(r)) { \
Raise_Error(_SC("Expression type is %s, not bool as expected"), GetTypeName((o))); \
SQ_THROW(); \
} \
}

#define VALIDATE_NOT_STRING(a, b) { \
if ((_closure(ci->_closure)->_function->lang_features & LF_NO_PLUS_CONCAT) && ((sq_type(a) | sq_type(b)) & _RT_STRING)) \
{ \
Raise_Error(_SC("'+' cannot be applied to string")); \
SQ_THROW(); \
} \
}

extern SQInstructionDesc g_InstrDesc[];
bool SQVM::Execute(SQObjectPtr &closure, SQInteger nargs, SQInteger stackbase,SQObjectPtr &outres, SQBool invoke_err_handler,ExecutionType et)
Expand Down Expand Up @@ -1173,7 +1152,6 @@ bool SQVM::Execute(SQObjectPtr &closure, SQInteger nargs, SQInteger stackbase,SQ
TARGET = (!res)?true:false;
} continue;
case _OP_ADD:
VALIDATE_NOT_STRING(STK(arg2), STK(arg1));
_ARITH_(+,TARGET,STK(arg2),STK(arg1));
continue;
case _OP_SUB: _ARITH_(-,TARGET,STK(arg2),STK(arg1)); continue;
Expand All @@ -1200,18 +1178,14 @@ bool SQVM::Execute(SQObjectPtr &closure, SQInteger nargs, SQInteger stackbase,SQ
case _OP_LOADCALLEE: TARGET = ci->_closure; continue;
case _OP_DMOVE: STK(arg0) = STK(arg1); STK(arg2) = STK(arg3); continue;
case _OP_JMP: ci->_ip += (sarg1); continue;
//case _OP_JNZ: if(!ResolveBooleanResult(STK(arg0))) ci->_ip+=(sarg1); continue;
//case _OP_JNZ: if(!IsFalse(STK(arg0))) ci->_ip+=(sarg1); continue;
case _OP_JCMP: {
_GUARD(CMP_OP((CmpOP)arg3,STK(arg2),STK(arg0),temp_reg));
BooleanResult result = ResolveBooleanResult(temp_reg);
VALIDATE_BOOLEAN_RESULT(result, temp_reg);
if(IsBooleanResultFalse(result)) ci->_ip+=(sarg1);
if(IsFalse(temp_reg)) ci->_ip+=(sarg1);
}
continue;
case _OP_JZ: {
BooleanResult result = ResolveBooleanResult(STK(arg0));
VALIDATE_BOOLEAN_RESULT(result, STK(arg0));
if(IsBooleanResultFalse(result)) ci->_ip+=(sarg1);
if(IsFalse(STK(arg0))) ci->_ip+=(sarg1);
} continue;
case _OP_GETOUTER: {
SQClosure *cur_cls = _closure(ci->_closure);
Expand Down Expand Up @@ -1274,7 +1248,6 @@ bool SQVM::Execute(SQObjectPtr &closure, SQInteger nargs, SQInteger stackbase,SQ
continue;
case _OP_INC: {
SQObjectPtr o(sarg3);
VALIDATE_NOT_STRING(STK(arg2), o);
_GUARD(DerefInc('+',TARGET, STK(arg1), STK(arg2), o, false, arg1));
} continue;
case _OP_INCL: {
Expand All @@ -1284,13 +1257,11 @@ bool SQVM::Execute(SQObjectPtr &closure, SQInteger nargs, SQInteger stackbase,SQ
}
else {
SQObjectPtr o(sarg3); //_GUARD(LOCAL_INC('+',TARGET, STK(arg1), o));
VALIDATE_NOT_STRING(a, o);
_ARITH_(+,a,a,o);
}
} continue;
case _OP_PINC: {
SQObjectPtr o(sarg3);
VALIDATE_NOT_STRING(STK(arg2), o);
_GUARD(DerefInc('+',TARGET, STK(arg1), STK(arg2), o, true, arg1));
} continue;
case _OP_PINCL: {
Expand All @@ -1301,7 +1272,6 @@ bool SQVM::Execute(SQObjectPtr &closure, SQInteger nargs, SQInteger stackbase,SQ
}
else {
SQObjectPtr o(sarg3);
VALIDATE_NOT_STRING(STK(arg1), o);
_GUARD(PLOCAL_INC('+',TARGET, STK(arg1), o));
}

Expand All @@ -1314,19 +1284,15 @@ bool SQVM::Execute(SQObjectPtr &closure, SQInteger nargs, SQInteger stackbase,SQ
TARGET = (sq_type(STK(arg2)) == OT_INSTANCE) ? (_instance(STK(arg2))->InstanceOf(_class(STK(arg1)))?true:false) : false;
continue;
case _OP_AND:{
BooleanResult result = ResolveBooleanResult(STK(arg2));
VALIDATE_BOOLEAN_RESULT(result, STK(arg2));
if(IsBooleanResultFalse(result)) {
if(IsFalse(STK(arg2))) {
TARGET = STK(arg2);
ci->_ip += (sarg1);
}

}
continue;
case _OP_OR:{
BooleanResult result = ResolveBooleanResult(STK(arg2));
VALIDATE_BOOLEAN_RESULT(result, STK(arg2));
if(!IsBooleanResultFalse(result)) {
if(!IsFalse(STK(arg2))) {
TARGET = STK(arg2);
ci->_ip += (sarg1);
}
Expand All @@ -1341,9 +1307,7 @@ bool SQVM::Execute(SQObjectPtr &closure, SQInteger nargs, SQInteger stackbase,SQ
continue;
case _OP_NEG: _GUARD(NEG_OP(TARGET,STK(arg1))); continue;
case _OP_NOT:{
BooleanResult result = ResolveBooleanResult(STK(arg1));
VALIDATE_BOOLEAN_RESULT(result, STK(arg1));
TARGET = IsBooleanResultFalse(result);
TARGET = IsFalse(STK(arg1));
} continue;
case _OP_BWNOT:
if(sq_type(STK(arg1)) == OT_INTEGER) {
Expand Down
4 changes: 0 additions & 4 deletions squirrel/sqvm.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,6 @@ typedef sqvector<CallInfo> CallInfoVec;
void Remove(SQInteger n);

static bool IsFalse(const SQObjectPtr &o);
enum BooleanResult {BOOL_FALSE = 0, LEGACY_FALSE = 1, BOOL_TRUE = 2, LEGACY_TRUE = 3};
static BooleanResult ResolveBooleanResult(const SQObjectPtr &o);
static bool IsBooleanResultFalse(BooleanResult r){return r <= LEGACY_FALSE;}
static bool IsBooleanResultLegacy(BooleanResult r){return ((unsigned)r) & 1;}

void Pop();
void Pop(SQInteger n);
Expand Down

0 comments on commit ce89651

Please sign in to comment.