From 5ce3c7aff137a8238b69251a9140ba76e0756113 Mon Sep 17 00:00:00 2001 From: Rangi Date: Fri, 25 Dec 2020 20:53:27 -0500 Subject: [PATCH] Logical operators `||` and `&&` are short-circuiting `true || X` short-circuits to 1 without evaluating X `false && X` short-circuits to 0 without evaluating X Fixes #619 --- include/asm/rpn.h | 3 ++ src/asm/lexer.c | 3 ++ src/asm/parser.y | 99 +++++++++++++++++++++++++---------- src/asm/rpn.c | 42 +++++++++++++++ test/asm/short-circuiting.asm | 8 +++ test/asm/short-circuiting.err | 0 test/asm/short-circuiting.out | 4 ++ 7 files changed, 130 insertions(+), 29 deletions(-) create mode 100644 test/asm/short-circuiting.asm create mode 100644 test/asm/short-circuiting.err create mode 100644 test/asm/short-circuiting.out diff --git a/include/asm/rpn.h b/include/asm/rpn.h index c971bf988b..e460c4ba9c 100644 --- a/include/asm/rpn.h +++ b/include/asm/rpn.h @@ -46,6 +46,9 @@ static inline bool rpn_isSymbol(const struct Expression *expr) return expr->isSymbol; } +void rpn_EnterShortCircuitOp(bool startShortCircuit); +void rpn_LeaveShortCircuitOp(void); +bool rpn_IsShortCircuited(void); void rpn_Symbol(struct Expression *expr, char const *tzSym); void rpn_Number(struct Expression *expr, uint32_t i); void rpn_LOGNOT(struct Expression *expr, const struct Expression *src); diff --git a/src/asm/lexer.c b/src/asm/lexer.c index 627829a170..f6429cc659 100644 --- a/src/asm/lexer.c +++ b/src/asm/lexer.c @@ -1346,6 +1346,9 @@ static char const *readInterpolation(void) } } + if (rpn_IsShortCircuited()) + return NULL; + if (i == sizeof(symName)) { warning(WARNING_LONG_STR, "Symbol name too long\n"); i--; diff --git a/src/asm/parser.y b/src/asm/parser.y index e554c9b3ef..9a5dc714c6 100644 --- a/src/asm/parser.y +++ b/src/asm/parser.y @@ -1180,11 +1180,27 @@ relocexpr_no_str : scoped_anon_id { rpn_Symbol(&$$, $1); } | T_OP_LOGICNOT relocexpr %prec NEG { rpn_LOGNOT(&$$, &$2); } - | relocexpr T_OP_LOGICOR relocexpr { - rpn_BinaryOp(RPN_LOGOR, &$$, &$1, &$3); - } - | relocexpr T_OP_LOGICAND relocexpr { - rpn_BinaryOp(RPN_LOGAND, &$$, &$1, &$3); + | relocexpr T_OP_LOGICOR { + // If the first operand is nonzero... + rpn_EnterShortCircuitOp(rpn_isKnown(&$1) && $1.nVal); + } relocexpr { + // ...then short-circuit to 1 without evaluating the second operand. + if (rpn_IsShortCircuited()) + rpn_Number(&$$, 1); + else + rpn_BinaryOp(RPN_LOGOR, &$$, &$1, &$4); + rpn_LeaveShortCircuitOp(); + } + | relocexpr T_OP_LOGICAND { + // If the first operand is zero... + rpn_EnterShortCircuitOp(rpn_isKnown(&$1) && !$1.nVal); + } relocexpr { + // ...then short-circuit to 0 without evaluating the second operand. + if (rpn_IsShortCircuited()) + rpn_Number(&$$, 0); + else + rpn_BinaryOp(RPN_LOGAND, &$$, &$1, &$4); + rpn_LeaveShortCircuitOp(); } | relocexpr T_OP_LOGICEQU relocexpr { rpn_BinaryOp(RPN_LOGEQ, &$$, &$1, &$3); @@ -1350,32 +1366,52 @@ const_no_str : relocexpr_no_str { string : T_STRING | T_OP_STRSUB T_LPAREN string T_COMMA uconst T_COMMA uconst T_RPAREN { - strsubUTF8($$, $3, $5, $7); + if (rpn_IsShortCircuited()) + $$[0] = '\0'; + else + strsubUTF8($$, $3, $5, $7); } | T_OP_STRCAT T_LPAREN T_RPAREN { $$[0] = '\0'; } | T_OP_STRCAT T_LPAREN strcat_args T_RPAREN { - strcpy($$, $3); + if (rpn_IsShortCircuited()) + $$[0] = '\0'; + else + strcpy($$, $3); } | T_OP_STRUPR T_LPAREN string T_RPAREN { - upperstring($$, $3); + if (rpn_IsShortCircuited()) + $$[0] = '\0'; + else + upperstring($$, $3); } | T_OP_STRLWR T_LPAREN string T_RPAREN { - lowerstring($$, $3); + if (rpn_IsShortCircuited()) + $$[0] = '\0'; + else + lowerstring($$, $3); } | T_OP_STRRPL T_LPAREN string T_COMMA string T_COMMA string T_RPAREN { - strrpl($$, sizeof($$), $3, $5, $7); + if (rpn_IsShortCircuited()) + $$[0] = '\0'; + else + strrpl($$, sizeof($$), $3, $5, $7); } | T_OP_STRFMT T_LPAREN strfmt_args T_RPAREN { - strfmt($$, sizeof($$), $3.format, $3.nbArgs, $3.args); + if (rpn_IsShortCircuited()) + $$[0] = '\0'; + else + strfmt($$, sizeof($$), $3.format, $3.nbArgs, $3.args); freeStrFmtArgList(&$3); } ; strcat_args : string | strcat_args T_COMMA string { - if (snprintf($$, sizeof($$), "%s%s", $1, $3) > MAXSTRLEN) + if (rpn_IsShortCircuited()) + $$[0] = '\0'; + else if (snprintf($$, sizeof($$), "%s%s", $1, $3) > MAXSTRLEN) warning(WARNING_LONG_STR, "STRCAT: String too long '%s%s'\n", $1, $3); } @@ -1390,31 +1426,36 @@ strfmt_args : string strfmt_va_args { ; strfmt_va_args : /* empty */ { - initStrFmtArgList(&$$); + if (!rpn_IsShortCircuited()) + initStrFmtArgList(&$$); } | strfmt_va_args T_COMMA relocexpr_no_str { - int32_t value; + if (!rpn_IsShortCircuited()) { + int32_t value; - if (!rpn_isKnown(&$3)) { - error("Expected constant expression: %s\n", - $3.reason); - value = 0; - } else { - value = $3.nVal; - } + if (!rpn_isKnown(&$3)) { + error("Expected constant expression: %s\n", + $3.reason); + value = 0; + } else { + value = $3.nVal; + } - size_t i = nextStrFmtArgListIndex(&$1); + size_t i = nextStrFmtArgListIndex(&$1); - $1.args[i].number = value; - $1.args[i].isNumeric = true; - $$ = $1; + $1.args[i].number = value; + $1.args[i].isNumeric = true; + $$ = $1; + } } | strfmt_va_args T_COMMA string { - size_t i = nextStrFmtArgListIndex(&$1); + if (!rpn_IsShortCircuited()) { + size_t i = nextStrFmtArgListIndex(&$1); - $1.args[i].string = strdup($3); - $1.args[i].isNumeric = false; - $$ = $1; + $1.args[i].string = strdup($3); + $1.args[i].isNumeric = false; + $$ = $1; + } } ; diff --git a/src/asm/rpn.c b/src/asm/rpn.c index 4268edc675..6c744f862b 100644 --- a/src/asm/rpn.c +++ b/src/asm/rpn.c @@ -24,6 +24,9 @@ #include "asm/symbol.h" #include "asm/warning.h" +uint32_t nestedShortCircuitOps; // How many nested short-circuitable operators there currently are +uint32_t shortCircuitAtDepth; // The nested depth at which short-circuiting occurred (0 if N/A) + /* Makes an expression "not known", also setting its error message */ #define makeUnknown(expr_, ...) do { \ struct Expression *_expr = expr_; \ @@ -95,6 +98,25 @@ void rpn_Free(struct Expression *expr) rpn_Init(expr); } +void rpn_EnterShortCircuitOp(bool startShortCircuit) +{ + nestedShortCircuitOps++; + if (startShortCircuit && shortCircuitAtDepth == 0) + shortCircuitAtDepth = nestedShortCircuitOps; +} + +void rpn_LeaveShortCircuitOp(void) +{ + nestedShortCircuitOps--; + if (shortCircuitAtDepth > nestedShortCircuitOps) + shortCircuitAtDepth = 0; +} + +bool rpn_IsShortCircuited(void) +{ + return shortCircuitAtDepth > 0 && shortCircuitAtDepth >= nestedShortCircuitOps; +} + /* * Add symbols, constants and operators to expression */ @@ -106,6 +128,11 @@ void rpn_Number(struct Expression *expr, uint32_t i) void rpn_Symbol(struct Expression *expr, char const *tzSym) { + if (rpn_IsShortCircuited()) { + rpn_Number(expr, 0); + return; + } + struct Symbol *sym = sym_FindScopedSymbol(tzSym); if (sym_IsPC(sym) && !sect_GetSymbolSection()) { @@ -157,6 +184,11 @@ void rpn_BankSelf(struct Expression *expr) void rpn_BankSymbol(struct Expression *expr, char const *tzSym) { + if (rpn_IsShortCircuited()) { + rpn_Number(expr, 0); + return; + } + struct Symbol const *sym = sym_FindScopedSymbol(tzSym); /* The @ symbol is treated differently. */ @@ -189,6 +221,11 @@ void rpn_BankSymbol(struct Expression *expr, char const *tzSym) void rpn_BankSection(struct Expression *expr, char const *tzSectionName) { + if (rpn_IsShortCircuited()) { + rpn_Number(expr, 0); + return; + } + rpn_Init(expr); struct Section *pSection = out_FindSectionByName(tzSectionName); @@ -335,6 +372,11 @@ static bool isDiffConstant(struct Expression const *src1, void rpn_BinaryOp(enum RPNCommand op, struct Expression *expr, const struct Expression *src1, const struct Expression *src2) { + if (rpn_IsShortCircuited()) { + rpn_Number(expr, 0); + return; + } + expr->isSymbol = false; /* First, check if the expression is known */ diff --git a/test/asm/short-circuiting.asm b/test/asm/short-circuiting.asm new file mode 100644 index 0000000000..ca63adc6a7 --- /dev/null +++ b/test/asm/short-circuiting.asm @@ -0,0 +1,8 @@ + + println 1 == 1 || undef_n == 42 + + println 1 == 2 && 0 < STRLEN("{undef_s}") + + println ((1 == 1 || (undef_a || undef_b)) || undef_c) + + println 1 == 2 && 0 < STRLEN(STRFMT("%d%s%v", "{undef_s}")) diff --git a/test/asm/short-circuiting.err b/test/asm/short-circuiting.err new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/asm/short-circuiting.out b/test/asm/short-circuiting.out new file mode 100644 index 0000000000..783503ded0 --- /dev/null +++ b/test/asm/short-circuiting.out @@ -0,0 +1,4 @@ +$1 +$0 +$1 +$0