From f71aad4cdde28b4cb487481e103615623b3fb7ca Mon Sep 17 00:00:00 2001 From: Christoph Oelckers Date: Fri, 18 Nov 2016 14:19:55 +0100 Subject: [PATCH] - cleanup of the remaining FxBinary operators. - changed FxCompareEq with strings and other types that can be cast to from a string always convert the string to the other type before comparing. --- src/scripting/codegeneration/codegen.cpp | 473 +++++++++++------------ src/scripting/codegeneration/codegen.h | 22 +- src/scripting/decorate/thingdef_exp.cpp | 20 +- src/scripting/zscript/zcc_compile.cpp | 8 +- 4 files changed, 260 insertions(+), 263 deletions(-) diff --git a/src/scripting/codegeneration/codegen.cpp b/src/scripting/codegeneration/codegen.cpp index c18548bdc..e57303c0a 100644 --- a/src/scripting/codegeneration/codegen.cpp +++ b/src/scripting/codegeneration/codegen.cpp @@ -838,7 +838,7 @@ FxExpression *FxIntCast::Resolve(FCompileContext &ctx) { ExpVal constval = static_cast(basex)->GetValue(); FxExpression *x = new FxConstant(constval.GetInt(), ScriptPosition); - if (!NoWarn && constval.GetInt() != constval.GetFloat()) + if (constval.GetInt() != constval.GetFloat()) { ScriptPosition.Message(MSG_WARNING, "Truncation of floating point constant %f", constval.GetFloat()); } @@ -2363,157 +2363,7 @@ FxBinary::~FxBinary() // //========================================================================== -bool FxBinary::ResolveLR(FCompileContext& ctx, bool castnumeric) -{ - RESOLVE(left, ctx); - RESOLVE(right, ctx); - if (!left || !right) - { - delete this; - return false; - } - - if (left->ValueType == TypeString || right->ValueType == TypeString) - { - switch (Operator) - { - case '<': - case '>': - case TK_Geq: - case TK_Leq: - case TK_Eq: - case TK_Neq: - case TK_ApproxEq: - if (left->ValueType != TypeString) - { - left = new FxStringCast(left); - left = left->Resolve(ctx); - if (left == nullptr) - { - delete this; - return false; - } - } - if (right->ValueType != TypeString) - { - right = new FxStringCast(right); - right = right->Resolve(ctx); - if (right == nullptr) - { - delete this; - return false; - } - } - ValueType = TypeBool; - break; - - default: - ScriptPosition.Message(MSG_ERROR, "Incompatible operands for comparison"); - delete this; - return false; - } - } - else if (left->IsVector() || right->IsVector()) - { - switch (Operator) - { - case TK_Eq: - case TK_Neq: - if (left->ValueType != right->ValueType) - { - ScriptPosition.Message(MSG_ERROR, "Incompatible operands for comparison"); - delete this; - return false; - } - ValueType = TypeBool; - break; - - default: - ScriptPosition.Message(MSG_ERROR, "Incompatible operation for vector type"); - delete this; - return false; - - } - } - else if (left->ValueType == TypeBool && right->ValueType == TypeBool) - { - if (Operator == '&' || Operator == '|' || Operator == '^' || ctx.FromDecorate) - { - ValueType = TypeBool; - } - else - { - ValueType = TypeSInt32; // math operations on bools result in integers. - } - } - else if (left->ValueType == TypeName && right->ValueType == TypeName) - { - // pointers can only be compared for equality. - if (Operator == TK_Eq || Operator == TK_Neq) - { - ValueType = TypeBool; - return true; - } - else - { - ScriptPosition.Message(MSG_ERROR, "Invalid operation for names"); - delete this; - return false; - } - } - else if (left->IsNumeric() && right->IsNumeric()) - { - if (left->ValueType->GetRegType() == REGT_INT && right->ValueType->GetRegType() == REGT_INT) - { - ValueType = TypeSInt32; - } - else - { - ValueType = TypeFloat64; - } - } - else if (left->ValueType->GetRegType() == REGT_POINTER) - { - if (left->ValueType == right->ValueType || right->ValueType == TypeNullPtr || left->ValueType == TypeNullPtr || - AreCompatiblePointerTypes(left->ValueType, right->ValueType)) - { - // pointers can only be compared for equality. - if (Operator == TK_Eq || Operator == TK_Neq) - { - ValueType = TypeBool; - return true; - } - else - { - ScriptPosition.Message(MSG_ERROR, "Invalid operation for pointers"); - delete this; - return false; - } - } - } - else - { - // To check: It may be that this could pass in DECORATE, although setting TypeVoid here would pretty much prevent that. - ScriptPosition.Message(MSG_ERROR, "Incompatible operator"); - delete this; - return false; - } - assert(ValueType != nullptr && ValueType < (PType*)0xfffffffffffffff); - - if (castnumeric) - { - // later! - } - return true; -} - -//========================================================================== -// -// -// -//========================================================================== - -void FxBinary::Promote(FCompileContext &ctx) +bool FxBinary::Promote(FCompileContext &ctx, bool forceint) { // math operations of unsigned ints results in an unsigned int. (16 and 8 bit values never get here, they get promoted to regular ints elsewhere already.) if (left->ValueType == TypeUInt32 && right->ValueType == TypeUInt32) @@ -2524,7 +2374,7 @@ void FxBinary::Promote(FCompileContext &ctx) { ValueType = TypeSInt32; // Addition and subtraction forces all integer-derived types to signed int. } - else + else if (!forceint) { ValueType = TypeFloat64; if (left->IsFloat() && right->IsInteger()) @@ -2536,6 +2386,34 @@ void FxBinary::Promote(FCompileContext &ctx) left = (new FxFloatCast(left))->Resolve(ctx); } } + else if (ctx.FromDecorate) + { + // For DECORATE which allows floats here. ZScript does not. + if (left->IsFloat()) + { + left = new FxIntCast(left, ctx.FromDecorate); + left = left->Resolve(ctx); + } + if (right->IsFloat()) + { + right = new FxIntCast(right, ctx.FromDecorate); + right = right->Resolve(ctx); + } + if (left == nullptr || right == nullptr) + { + delete this; + return false; + } + ValueType = TypeSInt32; + + } + else + { + ScriptPosition.Message(MSG_ERROR, "Integer operand expected"); + delete this; + return false; + } + return true; } //========================================================================== @@ -3064,7 +2942,7 @@ FxExpression *FxCompareRel::Resolve(FCompileContext& ctx) return nullptr; } } - ValueType == TypeString; + ValueType = TypeString; } else if (left->IsNumeric() && right->IsNumeric()) { @@ -3238,24 +3116,66 @@ FxExpression *FxCompareEq::Resolve(FCompileContext& ctx) { CHECKRESOLVED(); - if (!ResolveLR(ctx, true)) - return nullptr; - + RESOLVE(left, ctx); + RESOLVE(right, ctx); if (!left || !right) { delete this; return nullptr; } - if (!IsNumeric() && !IsPointer() && !IsVector() && ValueType != TypeName) + if (left->ValueType != right->ValueType) // identical types are always comparable, if they can be placed in a register, so we can save most checks if this is the case. { - ScriptPosition.Message(MSG_ERROR, "Numeric type expected"); - delete this; - return nullptr; + // Special cases: Compare strings and names with names, sounds, colors, state labels and class types. + // These are all types a string can be implicitly cast into, so for convenience, so they should when doing a comparison. + if ((left->ValueType == TypeString || left->ValueType == TypeName) && + (right->ValueType == TypeName || right->ValueType == TypeSound || right->ValueType == TypeColor || right->ValueType->IsKindOf(RUNTIME_CLASS(PClassPointer)) || right->ValueType == TypeStateLabel)) + { + left = new FxTypeCast(left, right->ValueType, false, true); + left = left->Resolve(ctx); + ABORT(left); + ValueType = right->ValueType; + } + else if ((right->ValueType == TypeString || right->ValueType == TypeName) && + (left->ValueType == TypeName || left->ValueType == TypeSound || left->ValueType == TypeColor || left->ValueType->IsKindOf(RUNTIME_CLASS(PClassPointer)) || left->ValueType == TypeStateLabel)) + { + right = new FxTypeCast(right, left->ValueType, false, true); + right = right->Resolve(ctx); + ABORT(right); + ValueType = left->ValueType; + } + else if (left->IsNumeric() && right->IsNumeric()) + { + Promote(ctx); + } + else if (left->ValueType->GetRegType() == REGT_POINTER && right->ValueType->GetRegType() == REGT_POINTER) + { + if (left->ValueType != right->ValueType && right->ValueType != TypeNullPtr && left->ValueType != TypeNullPtr && + !AreCompatiblePointerTypes(left->ValueType, right->ValueType)) + { + goto error; + } + } + else + { + goto error; + } + } + else if (left->ValueType->GetRegType() == REGT_NIL) + { + goto error; + } + else + { + ValueType = left->ValueType; + } + + if (Operator == TK_ApproxEq && ValueType->GetRegType() != REGT_FLOAT && ValueType->GetRegType() != REGT_STRING) + { + // Only floats, vectors and strings have handling for '~==', for all other types this is an error. + goto error; } - if (Operator == TK_ApproxEq && left->ValueType->GetRegType() != REGT_FLOAT && left->ValueType->GetRegType() != REGT_STRING) - Operator = TK_Eq; if (left->isConstant() && right->isConstant()) { int v; @@ -3357,9 +3277,13 @@ FxExpression *FxCompareEq::Resolve(FCompileContext& ctx) } } } - Promote(ctx); ValueType = TypeBool; return this; + +error: + ScriptPosition.Message(MSG_ERROR, "Incompatible operands for %s comparison", Operator == TK_Eq ? "==" : Operator == TK_Neq ? "!=" : "~=="); + delete this; + return nullptr; } //========================================================================== @@ -3435,7 +3359,7 @@ ExpEmit FxCompareEq::Emit(VMFunctionBuilder *build) // //========================================================================== -FxBinaryInt::FxBinaryInt(int o, FxExpression *l, FxExpression *r) +FxBitOp::FxBitOp(int o, FxExpression *l, FxExpression *r) : FxBinary(o, l, r) { ValueType = TypeSInt32; @@ -3447,48 +3371,39 @@ FxBinaryInt::FxBinaryInt(int o, FxExpression *l, FxExpression *r) // //========================================================================== -FxExpression *FxBinaryInt::Resolve(FCompileContext& ctx) +FxExpression *FxBitOp::Resolve(FCompileContext& ctx) { CHECKRESOLVED(); - if (!ResolveLR(ctx, false)) - return nullptr; - if (IsFloat() && ctx.FromDecorate) + RESOLVE(left, ctx); + RESOLVE(right, ctx); + if (!left || !right) { - // For DECORATE which allows floats here. ZScript does not. - if (left->ValueType->GetRegType() != REGT_INT) - { - left = new FxIntCast(left, ctx.FromDecorate); - left = left->Resolve(ctx); - } - if (right->ValueType->GetRegType() != REGT_INT) - { - right = new FxIntCast(right, ctx.FromDecorate); - right = right->Resolve(ctx); - } - if (left == nullptr || right == nullptr) - { - delete this; - return nullptr; - } - ValueType = TypeSInt32; + delete this; + return false; } - if (ValueType->GetRegType() != REGT_INT) + if (left->ValueType == TypeBool && right->ValueType == TypeBool) { - ScriptPosition.Message(MSG_ERROR, "Integer type expected"); + ValueType = TypeBool; + } + else if (left->IsNumeric() && right->IsNumeric()) + { + if (!Promote(ctx, true)) return nullptr; + } + else + { + ScriptPosition.Message(MSG_ERROR, "Incompatible operands for bit operation"); delete this; return nullptr; } - else if (left->isConstant() && right->isConstant()) + + if (left->isConstant() && right->isConstant()) { int v1 = static_cast(left)->GetValue().GetInt(); int v2 = static_cast(right)->GetValue().GetInt(); FxExpression *e = new FxConstant( - Operator == TK_LShift? v1 << v2 : - Operator == TK_RShift? v1 >> v2 : - Operator == TK_URShift? int((unsigned int)(v1) >> v2) : Operator == '&'? v1 & v2 : Operator == '|'? v1 | v2 : Operator == '^'? v1 ^ v2 : 0, ScriptPosition); @@ -3505,7 +3420,98 @@ FxExpression *FxBinaryInt::Resolve(FCompileContext& ctx) // //========================================================================== -ExpEmit FxBinaryInt::Emit(VMFunctionBuilder *build) +ExpEmit FxBitOp::Emit(VMFunctionBuilder *build) +{ + assert(left->ValueType->GetRegType() == REGT_INT); + assert(right->ValueType->GetRegType() == REGT_INT); + int instr, rop; + ExpEmit op1, op2; + + op1 = left->Emit(build); + op2 = right->Emit(build); + if (op1.Konst) + { + swapvalues(op1, op2); + } + assert(!op1.Konst); + rop = op2.RegNum; + op2.Free(build); + op1.Free(build); + + instr = Operator == '&' ? OP_AND_RR : + Operator == '|' ? OP_OR_RR : + Operator == '^' ? OP_XOR_RR : -1; + + assert(instr > 0); + ExpEmit to(build, REGT_INT); + build->Emit(instr + op2.Konst, to.RegNum, op1.RegNum, rop); + return to; +} + +//========================================================================== +// +// +// +//========================================================================== + +FxShift::FxShift(int o, FxExpression *l, FxExpression *r) + : FxBinary(o, l, r) +{ + ValueType = TypeSInt32; +} + +//========================================================================== +// +// +// +//========================================================================== + +FxExpression *FxShift::Resolve(FCompileContext& ctx) +{ + CHECKRESOLVED(); + RESOLVE(left, ctx); + RESOLVE(right, ctx); + if (!left || !right) + { + delete this; + return false; + } + + if (left->IsNumeric() && right->IsNumeric()) + { + if (!Promote(ctx, true)) return nullptr; + if (ValueType == TypeUInt32 && Operator == TK_RShift) Operator = TK_URShift; + } + else + { + ScriptPosition.Message(MSG_ERROR, "Incompatible operands for shift operation"); + delete this; + return nullptr; + } + + if (left->isConstant() && right->isConstant()) + { + int v1 = static_cast(left)->GetValue().GetInt(); + int v2 = static_cast(right)->GetValue().GetInt(); + + FxExpression *e = new FxConstant( + Operator == TK_LShift ? v1 << v2 : + Operator == TK_RShift ? v1 >> v2 : + Operator == TK_URShift ? int((unsigned int)(v1) >> v2) : 0, ScriptPosition); + + delete this; + return e; + } + return this; +} + +//========================================================================== +// +// +// +//========================================================================== + +ExpEmit FxShift::Emit(VMFunctionBuilder *build) { assert(left->ValueType->GetRegType() == REGT_INT); assert(right->ValueType->GetRegType() == REGT_INT); @@ -3514,65 +3520,41 @@ ExpEmit FxBinaryInt::Emit(VMFunctionBuilder *build) { OP_SLL_RR, OP_SLL_KR, OP_SLL_RI }, // TK_LShift { OP_SRA_RR, OP_SRA_KR, OP_SRA_RI }, // TK_RShift { OP_SRL_RR, OP_SRL_KR, OP_SRL_RI }, // TK_URShift - { OP_AND_RR, 0, OP_AND_RK }, // '&' - { OP_OR_RR, 0, OP_OR_RK }, // '|' - { OP_XOR_RR, 0, OP_XOR_RK }, // '^' }; int index, instr, rop; ExpEmit op1, op2; index = Operator == TK_LShift ? 0 : Operator == TK_RShift ? 1 : - Operator == TK_URShift ? 2 : - Operator == '&' ? 3 : - Operator == '|' ? 4 : - Operator == '^' ? 5 : -1; + Operator == TK_URShift ? 2 : -1; assert(index >= 0); op1 = left->Emit(build); - if (index < 3) - { // Shift instructions use right-hand immediates instead of constant registers. - if (right->isConstant()) - { - rop = static_cast(right)->GetValue().GetInt(); - op2.Konst = true; - } - else - { - op2 = right->Emit(build); - assert(!op2.Konst); - op2.Free(build); - rop = op2.RegNum; - } + + // Shift instructions use right-hand immediates instead of constant registers. + if (right->isConstant()) + { + rop = static_cast(right)->GetValue().GetInt(); + op2.Konst = true; } else - { // The other operators only take a constant on the right-hand side. + { op2 = right->Emit(build); - if (op1.Konst) - { - swapvalues(op1, op2); - } - assert(!op1.Konst); - rop = op2.RegNum; + assert(!op2.Konst); op2.Free(build); + rop = op2.RegNum; } + if (!op1.Konst) { op1.Free(build); - if (!op2.Konst) - { - instr = InstrMap[index][0]; - } - else - { - instr = InstrMap[index][2]; - } + instr = InstrMap[index][op2.Konst? 0:2]; } else { assert(!op2.Konst); instr = InstrMap[index][1]; } - assert(instr != 0); + assert(instr > 0); ExpEmit to(build, REGT_INT); build->Emit(instr, to.RegNum, op1.RegNum, rop); return to; @@ -3599,30 +3581,27 @@ FxLtGtEq::FxLtGtEq(FxExpression *l, FxExpression *r) FxExpression *FxLtGtEq::Resolve(FCompileContext& ctx) { CHECKRESOLVED(); - if (!ResolveLR(ctx, true)) - return nullptr; - if (!left->IsNumeric() || !right->IsNumeric()) + RESOLVE(left, ctx); + RESOLVE(right, ctx); + if (!left || !right) + { + delete this; + return false; + } + + if (left->IsNumeric() && right->IsNumeric()) + { + Promote(ctx); + } + else { ScriptPosition.Message(MSG_ERROR, "<>= expects two numeric operands"); delete this; return nullptr; } - if (left->ValueType->GetRegType() != right->ValueType->GetRegType()) - { - if (left->ValueType->GetRegType() == REGT_INT) - { - left = new FxFloatCast(left); - SAFE_RESOLVE(left, ctx); - } - if (right->ValueType->GetRegType() == REGT_INT) - { - right = new FxFloatCast(left); - SAFE_RESOLVE(left, ctx); - } - } - else if (left->isConstant() && right->isConstant()) + if (left->isConstant() && right->isConstant()) { // let's cut this short and always compare doubles. For integers the result will be exactly the same as with an integer comparison, either signed or unsigned. auto v1 = static_cast(left)->GetValue().GetFloat(); diff --git a/src/scripting/codegeneration/codegen.h b/src/scripting/codegeneration/codegen.h index 5f7791e3e..df0634b43 100644 --- a/src/scripting/codegeneration/codegen.h +++ b/src/scripting/codegeneration/codegen.h @@ -813,8 +813,7 @@ public: FxBinary(int, FxExpression*, FxExpression*); ~FxBinary(); - bool ResolveLR(FCompileContext& ctx, bool castnumeric); - void Promote(FCompileContext &ctx); + bool Promote(FCompileContext &ctx, bool forceint = false); }; //========================================================================== @@ -899,11 +898,26 @@ public: // //========================================================================== -class FxBinaryInt : public FxBinary +class FxBitOp : public FxBinary { public: - FxBinaryInt(int, FxExpression*, FxExpression*); + FxBitOp(int, FxExpression*, FxExpression*); + FxExpression *Resolve(FCompileContext&); + ExpEmit Emit(VMFunctionBuilder *build); +}; + +//========================================================================== +// +// FxBinary +// +//========================================================================== + +class FxShift : public FxBinary +{ +public: + + FxShift(int, FxExpression*, FxExpression*); FxExpression *Resolve(FCompileContext&); ExpEmit Emit(VMFunctionBuilder *build); }; diff --git a/src/scripting/decorate/thingdef_exp.cpp b/src/scripting/decorate/thingdef_exp.cpp index 2d69be340..e7d217609 100644 --- a/src/scripting/decorate/thingdef_exp.cpp +++ b/src/scripting/decorate/thingdef_exp.cpp @@ -139,27 +139,27 @@ static FxExpression *ParseExpressionM (FScanner &sc, PClassActor *cls) break; case TK_LShiftEq: - exp = new FxBinaryInt(TK_LShift, left, nullptr); + exp = new FxShift(TK_LShift, left, nullptr); break; case TK_RShiftEq: - exp = new FxBinaryInt(TK_RShift, left, nullptr); + exp = new FxShift(TK_RShift, left, nullptr); break; case TK_URShiftEq: - exp = new FxBinaryInt(TK_URShift, left, nullptr); + exp = new FxShift(TK_URShift, left, nullptr); break; case TK_AndEq: - exp = new FxBinaryInt('&', left, nullptr); + exp = new FxBitOp('&', left, nullptr); break; case TK_XorEq: - exp = new FxBinaryInt('^', left, nullptr); + exp = new FxBitOp('^', left, nullptr); break; case TK_OrEq: - exp = new FxBinaryInt('|', left, nullptr); + exp = new FxBitOp('|', left, nullptr); break; default: @@ -207,7 +207,7 @@ static FxExpression *ParseExpressionJ (FScanner &sc, PClassActor *cls) while (sc.CheckToken('|')) { FxExpression *right = ParseExpressionI (sc, cls); - tmp = new FxBinaryInt('|', tmp, right); + tmp = new FxBitOp('|', tmp, right); } return tmp; } @@ -219,7 +219,7 @@ static FxExpression *ParseExpressionI (FScanner &sc, PClassActor *cls) while (sc.CheckToken('^')) { FxExpression *right = ParseExpressionH (sc, cls); - tmp = new FxBinaryInt('^', tmp, right); + tmp = new FxBitOp('^', tmp, right); } return tmp; } @@ -231,7 +231,7 @@ static FxExpression *ParseExpressionH (FScanner &sc, PClassActor *cls) while (sc.CheckToken('&')) { FxExpression *right = ParseExpressionG (sc, cls); - tmp = new FxBinaryInt('&', tmp, right); + tmp = new FxBitOp('&', tmp, right); } return tmp; } @@ -272,7 +272,7 @@ static FxExpression *ParseExpressionE (FScanner &sc, PClassActor *cls) { int token = sc.TokenType; FxExpression *right = ParseExpressionD (sc, cls); - tmp = new FxBinaryInt(token, tmp, right); + tmp = new FxShift(token, tmp, right); } if (!sc.End) sc.UnGet(); return tmp; diff --git a/src/scripting/zscript/zcc_compile.cpp b/src/scripting/zscript/zcc_compile.cpp index baf76027b..206e2dd88 100644 --- a/src/scripting/zscript/zcc_compile.cpp +++ b/src/scripting/zscript/zcc_compile.cpp @@ -2902,10 +2902,12 @@ FxExpression *ZCCCompiler::ConvertNode(ZCC_TreeNode *ast) case PEX_LeftShift: case PEX_RightShift: case PEX_URightShift: + return new FxShift(tok, left, right); + case PEX_BitAnd: case PEX_BitOr: case PEX_BitXor: - return new FxBinaryInt(tok, left, right); + return new FxBitOp(tok, left, right); case PEX_BoolOr: case PEX_BoolAnd: @@ -2937,10 +2939,12 @@ FxExpression *ZCCCompiler::ConvertNode(ZCC_TreeNode *ast) case PEX_LshAssign: case PEX_RshAssign: case PEX_URshAssign: + return ModifyAssign(new FxShift(tok, new FxAssignSelf(*ast), right), left); + case PEX_AndAssign: case PEX_OrAssign: case PEX_XorAssign: - return ModifyAssign(new FxBinaryInt(tok, new FxAssignSelf(*ast), right), left); + return ModifyAssign(new FxBitOp(tok, new FxAssignSelf(*ast), right), left); case PEX_LTGTEQ: return new FxLtGtEq(left, right);