diff --git a/crypto/func/auto-tests/tests/invalid-bitwise-1.fc b/crypto/func/auto-tests/tests/invalid-bitwise-1.fc new file mode 100644 index 00000000..545681d5 --- /dev/null +++ b/crypto/func/auto-tests/tests/invalid-bitwise-1.fc @@ -0,0 +1,9 @@ +int main(int flags) { + return flags & 0xFF != 0; +} + +{- +@compilation_should_fail +@stderr & has lower precedence than != +@stderr Use parenthesis +-} diff --git a/crypto/func/auto-tests/tests/invalid-bitwise-2.fc b/crypto/func/auto-tests/tests/invalid-bitwise-2.fc new file mode 100644 index 00000000..eaf83beb --- /dev/null +++ b/crypto/func/auto-tests/tests/invalid-bitwise-2.fc @@ -0,0 +1,8 @@ +int justTrue() { return true; } + +const a = justTrue() | 1 < 9; + +{- +@compilation_should_fail +@stderr | has lower precedence than < +-} diff --git a/crypto/func/auto-tests/tests/invalid-bitwise-3.fc b/crypto/func/auto-tests/tests/invalid-bitwise-3.fc new file mode 100644 index 00000000..b083acd6 --- /dev/null +++ b/crypto/func/auto-tests/tests/invalid-bitwise-3.fc @@ -0,0 +1,8 @@ +int justTrue() { return true; } + +const a = justTrue() | (1 < 9) | justTrue() != true; + +{- +@compilation_should_fail +@stderr | has lower precedence than != +-} diff --git a/crypto/func/auto-tests/tests/invalid-bitwise-4.fc b/crypto/func/auto-tests/tests/invalid-bitwise-4.fc new file mode 100644 index 00000000..c13f73d5 --- /dev/null +++ b/crypto/func/auto-tests/tests/invalid-bitwise-4.fc @@ -0,0 +1,6 @@ +const a = (1) <=> (0) ^ 8; + +{- +@compilation_should_fail +@stderr ^ has lower precedence than <=> +-} diff --git a/crypto/func/auto-tests/tests/invalid-bitwise-5.fc b/crypto/func/auto-tests/tests/invalid-bitwise-5.fc new file mode 100644 index 00000000..1a1d6117 --- /dev/null +++ b/crypto/func/auto-tests/tests/invalid-bitwise-5.fc @@ -0,0 +1,11 @@ +const MAX_SLIPAGE = 100; + +_ main(int jetton_amount, int msg_value, int slippage) { + if (0 == jetton_amount) | (msg_value == 0) | true | false | slippage > MAX_SLIPAGE { + } +} + +{- +@compilation_should_fail +@stderr | has lower precedence than > +-} diff --git a/crypto/func/auto-tests/tests/invalid-bitwise-6.fc b/crypto/func/auto-tests/tests/invalid-bitwise-6.fc new file mode 100644 index 00000000..e2825f95 --- /dev/null +++ b/crypto/func/auto-tests/tests/invalid-bitwise-6.fc @@ -0,0 +1,9 @@ +_ main() { + if ((1 == 1) | (2 == 2) & (3 == 3)) { + } +} + +{- +@compilation_should_fail +@stderr mixing | with & without parenthesis +-} diff --git a/crypto/func/auto-tests/tests/invalid-shift-1.fc b/crypto/func/auto-tests/tests/invalid-shift-1.fc new file mode 100644 index 00000000..2c0d3f23 --- /dev/null +++ b/crypto/func/auto-tests/tests/invalid-shift-1.fc @@ -0,0 +1,8 @@ +_ main(int flags) { + return flags << 1 + 32; +} + +{- +@compilation_should_fail +@stderr << has lower precedence than + +-} diff --git a/crypto/func/auto-tests/tests/op_priority.fc b/crypto/func/auto-tests/tests/op_priority.fc index f8eff74c..ac358f07 100644 --- a/crypto/func/auto-tests/tests/op_priority.fc +++ b/crypto/func/auto-tests/tests/op_priority.fc @@ -1,35 +1,44 @@ int justTrue() { return true; } int test1(int x, int y, int z) method_id(101) { - return x > 0 & y > 0 & z > 0; + return (x > 0) & (y > 0) & (z > 0); } int test2(int x, int y, int z) method_id(102) { - return x > (0 & y > 0 & z > 0); + return x > (0 & (y > 0) & (z > 0)); } int test3(int x, int y, int z) method_id(103) { - if (x < 0 | y < 0) { + if ((x < 0) | (y < 0)) { return z < 0; } - return x > 0 & y > 0; + return (x > 0) & (y > 0); } int test4(int x, int y, int mode) method_id(104) { if (mode == 1) { - return x == 10 | (y == 20); + return (x == 10) | (y == 20); } if (mode == 2) { - return x == 10 | y == 20; + return (x == 10) | (y == 20); } else { return x == (10 | (y == 20)); } } int test5(int status) method_id(105) { - return justTrue() & status == 1 & (justTrue() & status) == 1; + return justTrue() & (status == 1) & ((justTrue() & status) == 1); } -() main() { } +int _ 0, 3 & (3 > 0), 3 & (_<_(3, 0)), + 3 & _, or similar +// (an expression `1 < 2` is expressed as `_<_(1,2)`, see builtins.cpp) +static bool is_comparison_binary_op(const Expr* e_apply) { + const std::string& name = e_apply->sym->name(); + const size_t len = name.size(); + if (len < 3 || len > 5 || name[0] != '_' || name[len-1] != '_') { + return false; // not "_<_" and similar + } + + char c1 = name[1]; + char c2 = name[2]; + // < > <= != == >= <=> + return (len == 3 && (c1 == '<' || c1 == '>')) || + (len == 4 && (c1 == '<' || c1 == '>' || c1 == '!' || c1 == '=') && c2 == '=') || + (len == 5 && (c1 == '<' && c2 == '=' && name[3] == '>')); +} + +// same as above, but to detect bitwise operators: & | ^ +// (in FunC, they are used as logical ones due to absence of a boolean type and && || operators) +static bool is_bitwise_binary_op(const Expr* e_apply) { + const std::string& name = e_apply->sym->name(); + const size_t len = name.size(); + if (len != 3 || name[0] != '_' || name[len-1] != '_') { + return false; + } + + char c1 = name[1]; + return c1 == '&' || c1 == '|' || c1 == '^'; +} + +// same as above, but to detect addition/subtraction +static bool is_add_or_sub_binary_op(const Expr* e_apply) { + const std::string& name = e_apply->sym->name(); + const size_t len = name.size(); + if (len != 3 || name[0] != '_' || name[len-1] != '_') { + return false; + } + + char c1 = name[1]; + return c1 == '+' || c1 == '-'; +} + +static inline std::string get_builtin_operator_name(sym_idx_t sym_builtin) { + std::string underscored = symbols.get_name(sym_builtin); + return underscored.substr(1, underscored.size() - 2); +} + +// fire an error for a case "flags & 0xFF != 0" (equivalent to "flags & 1", probably unexpected) +// it would better be a warning, but we decided to make it a strict error +[[gnu::cold]] static void fire_error_lower_precedence(const SrcLocation& loc, sym_idx_t op_lower, sym_idx_t op_higher) { + std::string name_lower = get_builtin_operator_name(op_lower); + std::string name_higher = get_builtin_operator_name(op_higher); + throw src::ParseError(loc, name_lower + " has lower precedence than " + name_higher + + ", probably this code won't work as you expected. " + "Use parenthesis: either (... " + name_lower + " ...) to evaluate it first, or (... " + name_higher + " ...) to suppress this error."); +} + +// fire an error for a case "arg1 & arg2 | arg3" +[[gnu::cold]] static void fire_error_mix_bitwise_and_or(const SrcLocation& loc, sym_idx_t op1, sym_idx_t op2) { + std::string name1 = get_builtin_operator_name(op1); + std::string name2 = get_builtin_operator_name(op2); + throw src::ParseError(loc, "mixing " + name1 + " with " + name2 + " without parenthesis" + ", probably this code won't work as you expected. " + "Use parenthesis to emphasize operator precedence."); +} + +// diagnose when bitwise operators are used in a probably wrong way due to tricky precedence +// example: "flags & 0xFF != 0" is equivalent to "flags & 1", most likely it's unexpected +// the only way to suppress this error for the programmer is to use parenthesis +static void diagnose_bitwise_precedence(const SrcLocation& loc, sym_idx_t bitwise_sym, const Expr* lhs, const Expr* rhs) { + // handle "0 != flags & 0xFF" (lhs = "0 != flags") + if (!lhs->is_inside_parenthesis() && + lhs->cls == Expr::_Apply && lhs->e_type->is_int() && // fast false if 100% not + is_comparison_binary_op(lhs)) { + fire_error_lower_precedence(loc, bitwise_sym, lhs->sym->sym_idx); + // there is a tiny bug: "flags & _!=_(0xFF,0)" will also suggest to wrap rhs into parenthesis + } + + // handle "flags & 0xFF != 0" (rhs = "0xFF != 0") + if (!rhs->is_inside_parenthesis() && + rhs->cls == Expr::_Apply && rhs->e_type->is_int() && + is_comparison_binary_op(rhs)) { + fire_error_lower_precedence(loc, bitwise_sym, rhs->sym->sym_idx); + } + + // handle "arg1 & arg2 | arg3" (lhs = "arg1 & arg2") + if (!lhs->is_inside_parenthesis() && + lhs->cls == Expr::_Apply && lhs->e_type->is_int() && + is_bitwise_binary_op(lhs) && + lhs->sym->sym_idx != bitwise_sym) { + fire_error_mix_bitwise_and_or(loc, lhs->sym->sym_idx, bitwise_sym); + } +} + +// diagnose "a << 8 + 1" (equivalent to "a << 9", probably unexpected) +static void diagnose_addition_in_bitshift(const SrcLocation& loc, sym_idx_t bitshift_sym, const Expr* rhs) { + if (!rhs->is_inside_parenthesis() && + rhs->cls == Expr::_Apply && rhs->e_type->is_int() && + is_add_or_sub_binary_op(rhs)) { + fire_error_lower_precedence(loc, bitshift_sym, rhs->sym->sym_idx); + } +} + /* * * PARSE SOURCE @@ -268,7 +371,7 @@ void parse_const_decl(Lexer& lex) { CodeBlob code; // Handles processing and resolution of literals and consts auto x = parse_expr(lex, code, false); // also does lex.next() ! - if (x->flags != Expr::_IsRvalue) { + if (!x->is_rvalue()) { lex.cur().error("expression is not strictly Rvalue"); } if ((wanted_type == Expr::_Const) && (x->cls == Expr::_Apply)) @@ -447,6 +550,7 @@ Expr* parse_expr100(Lexer& lex, CodeBlob& code, bool nv) { } Expr* res = parse_expr(lex, code, nv); if (lex.tp() == ')') { + res->flags |= Expr::_IsInsideParenthesis; lex.expect(clbr); return res; } @@ -858,6 +962,7 @@ Expr* parse_expr17(Lexer& lex, CodeBlob& code, bool nv) { lex.next(); auto x = parse_expr20(lex, code, false); x->chk_rvalue(lex.cur()); + diagnose_addition_in_bitshift(loc, name, x); res = new Expr{Expr::_Apply, name, {res, x}}; res->here = loc; res->set_val(t); @@ -901,6 +1006,9 @@ Expr* parse_expr14(Lexer& lex, CodeBlob& code, bool nv) { lex.next(); auto x = parse_expr15(lex, code, false); x->chk_rvalue(lex.cur()); + // diagnose tricky bitwise precedence, like "flags & 0xFF != 0" (& has lower precedence) + diagnose_bitwise_precedence(loc, name, res, x); + res = new Expr{Expr::_Apply, name, {res, x}}; res->here = loc; res->set_val(t);