From de92564cc1e1e955c10f1a4f858b0a3cc8697618 Mon Sep 17 00:00:00 2001 From: Robert Bendun Date: Sun, 4 Sep 2022 19:44:33 +0200 Subject: [PATCH] Error reporting improvement for number operations; added power operator --- include/musique.hh | 23 ++++++--- src/builtin_operators.cc | 16 +++++- src/errors.cc | 35 +++++++++++++ src/number.cc | 103 ++++++++++++++++++++++++++------------- src/parser.cc | 3 +- 5 files changed, 137 insertions(+), 43 deletions(-) diff --git a/include/musique.hh b/include/musique.hh index 837a610..70041ce 100644 --- a/include/musique.hh +++ b/include/musique.hh @@ -184,6 +184,16 @@ namespace errors } type; }; + struct Arithmetic + { + enum Type + { + Division_By_Zero, + Fractional_Modulo, + Unable_To_Calculate_Modular_Multiplicative_Inverse + } type; + }; + /// Collection of messages that are considered internal and should not be printed to the end user. namespace internal { @@ -203,6 +213,7 @@ namespace errors /// All possible error types using Details = std::variant< + Arithmetic, Closing_Token_Without_Opening, Expected_Expression_Separator_Before, Failed_Numeric_Parsing, @@ -456,7 +467,7 @@ struct Token }; static constexpr usize Keywords_Count = 6; -static constexpr usize Operators_Count = 15; +static constexpr usize Operators_Count = 16; std::string_view type_name(Token::Type type); @@ -703,17 +714,15 @@ struct Number Number& operator-=(Number const& rhs); Number operator*(Number const& rhs) const; Number& operator*=(Number const& rhs); - Number operator/(Number const& rhs) const; - Number& operator/=(Number const& rhs); - Number operator%(Number const& rhs) const; - Number& operator%=(Number const& rhs); + Result operator/(Number const& rhs) const; + Result operator%(Number const& rhs) const; Number floor() const; ///< Return number rounded down to nearest integer Number ceil() const; ///< Return number rounded up to nearest integer Number round() const; ///< Return number rounded to nearest integer - Number inverse() const; ///< Return number raised to power -1 - Number pow(Number n) const; ///< Return number raised to power `n`. + Result inverse() const; ///< Return number raised to power -1 + Result pow(Number n) const; ///< Return number raised to power `n`. /// Parses source contained by token into a Number instance static Result from(Token token); diff --git a/src/builtin_operators.cc b/src/builtin_operators.cc index f25ca34..5229c6d 100644 --- a/src/builtin_operators.cc +++ b/src/builtin_operators.cc @@ -87,7 +87,6 @@ static Result plus_minus_operator(Interpreter &interpreter, std::vector static Result binary_operator(Interpreter& interpreter, std::vector args) { @@ -97,7 +96,11 @@ static Result binary_operator(Interpreter& interpreter, std::vector) { + return Value::from(Try(Binary_Operation{}(lhs, rhs))); + } else { + return Value::from(Binary_Operation{}(lhs, rhs)); + } } if (may_be_vectorized(args)) { @@ -182,6 +185,14 @@ static Result multiplication_operator(Interpreter &i, std::vector using Operator_Entry = std::tuple; +struct pow_operator +{ + inline Result operator()(Number lhs, Number rhs) + { + return lhs.pow(rhs); + } +}; + /// Operators definition table static constexpr auto Operators = std::array { Operator_Entry { "+", plus_minus_operator> }, @@ -189,6 +200,7 @@ static constexpr auto Operators = std::array { Operator_Entry { "*", multiplication_operator }, Operator_Entry { "/", binary_operator, '/'> }, Operator_Entry { "%", binary_operator, '%'> }, + Operator_Entry { "**", binary_operator }, Operator_Entry { "<", comparison_operator> }, Operator_Entry { ">", comparison_operator> }, diff --git a/src/errors.cc b/src/errors.cc index fa9425c..bdb3914 100644 --- a/src/errors.cc +++ b/src/errors.cc @@ -146,6 +146,15 @@ std::ostream& operator<<(std::ostream& os, Error const& err) [](errors::Expected_Expression_Separator_Before const&) { return "Missing semicolon"; }, [](errors::Literal_As_Identifier const&) { return "Literal used in place of an identifier"; }, [](errors::Out_Of_Range const&) { return "Index out of range"; }, + [](errors::Arithmetic const& err) { + switch (err.type) { + case errors::Arithmetic::Division_By_Zero: return "Division by 0"; + case errors::Arithmetic::Fractional_Modulo: return "Modulo with fractions"; + case errors::Arithmetic::Unable_To_Calculate_Modular_Multiplicative_Inverse: + return "Missing modular inverse"; + default: unreachable(); + } + }, [](errors::Closing_Token_Without_Opening const& err) { return err.type == errors::Closing_Token_Without_Opening::Block ? "Block closing without opening" @@ -368,6 +377,32 @@ std::ostream& operator<<(std::ostream& os, Error const& err) print_error_line(loc); }, + [&](errors::Arithmetic const& err) { + switch (err.type) { + break; case errors::Arithmetic::Division_By_Zero: + os << "Tried to divide by 0 which is undefined operation in math\n"; + os << "\n"; + print_error_line(loc); + + break; case errors::Arithmetic::Fractional_Modulo: + os << "Tried to calculate modulo with fractional modulus which is not defined\n"; + os << "\n"; + + print_error_line(loc); + + os << pretty::begin_comment; + os << "Example code that could raise this error:\n"; + os << " 1 % (1/2)\n"; + os << pretty::end; + + break; case errors::Arithmetic::Unable_To_Calculate_Modular_Multiplicative_Inverse: + os << "Tried to calculate fraction in modular space.\n"; + os << "\n"; + + print_error_line(loc); + } + }, + [&](errors::Unexpected_Keyword const&) { unimplemented(); }, }, err.details); diff --git a/src/number.cc b/src/number.cc index 313af1b..b6291c3 100644 --- a/src/number.cc +++ b/src/number.cc @@ -106,37 +106,72 @@ Number& Number::operator*=(Number const& rhs) return *this; } -Number Number::operator/(Number const& rhs) const +Result Number::operator/(Number const& rhs) const { - return Number{num * rhs.den, den * rhs.num}.simplify(); + if (rhs.num == 0) { + return Error { + .details = errors::Arithmetic { + .type = errors::Arithmetic::Division_By_Zero + } + }; + } + return Number{num * rhs.den, den * rhs.num}.simplify(); } -Number& Number::operator/=(Number const& rhs) +inline auto modular_inverse(Number::value_type a, Number::value_type n) + -> Result { - num *= rhs.den; - den *= rhs.num; - simplify_inplace(); - return *this; -} + using N = Number::value_type; + N t = 0, newt = 1; + N r = n, newr = a; - -Number Number::operator%(Number const& rhs) const -{ - return Number(*this) %= rhs; -} - -Number& Number::operator%=(Number const& rhs) -{ - simplify_inplace(); - auto [rnum, rden] = rhs.simplify(); - - if (den == 1 && rden == 1) { - num %= rnum; - } else { - assert(false, "Modulo for fractions is not supported"); + while (newr != 0) { + auto q = r / newr; + t = std::exchange(newt, t - q * newt); + r = std::exchange(newr, r - q * newr); } - return *this; + if (r > 1) { + return Error { + .details = errors::Arithmetic { + .type = errors::Arithmetic::Unable_To_Calculate_Modular_Multiplicative_Inverse + } + }; + } + + return t < 0 ? t + n : t; +} + + +Result Number::operator%(Number const& rhs) const +{ + if (rhs.num == 0) { + return Error { + .details = errors::Arithmetic { + .type = errors::Arithmetic::Division_By_Zero + } + }; + } + + auto ret = simplify(); + auto [rnum, rden] = rhs.simplify(); + + if (rden != 1) { + return Error { + .details = errors::Arithmetic { + .type = errors::Arithmetic::Fractional_Modulo + } + }; + } + + if (ret.den == 1) { + ret.num %= rnum; + return ret; + } + + return Number( + (Try(modular_inverse(ret.den, rnum)) * ret.num) % rnum + ); } std::ostream& operator<<(std::ostream& os, Number const& n) @@ -252,16 +287,22 @@ Number Number::round() const return impl::round(*this, impl::Rounding_Mode::Round); } -Number Number::inverse() const +Result Number::inverse() const { - assert(num != 0, "Cannot take inverse of fraction with 0 in denominator"); + if (num == 0) { + return Error { + .details = errors::Arithmetic { + .type = errors::Arithmetic::Division_By_Zero + } + }; + } return { den, num }; } namespace impl { // Raise Number to integer power helper - static inline Number pow(Number const& x, decltype(Number::num) n) + static inline Result pow(Number const& x, decltype(Number::num) n) { // We simply raise numerator and denominator to required power // and if n is negative we take inverse. @@ -273,11 +314,11 @@ namespace impl for (auto i = n; i != 0; --i) result.num *= x.num; for (auto i = n; i != 0; --i) result.den *= x.den; - return flip ? result.inverse() : result; + return flip ? Try(result.inverse()) : result; } } -Number Number::pow(Number n) const +Result Number::pow(Number n) const { n.simplify_inplace(); @@ -289,9 +330,5 @@ Number Number::pow(Number n) const // Hard case, we raise this to fractional power. // Essentialy finding n.den root of (x to n.num power). // We need to protect ourselfs against even roots of negative numbers. - // TODO Implement this case properly. - // - // TODO(assert) <- taking power is not always doable so this operation - // should produce error not crash with assertion failure in the future. unimplemented(); } diff --git a/src/parser.cc b/src/parser.cc index f9ab7e0..a7fa2af 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -519,13 +519,14 @@ static usize precedense(std::string_view op) // '.' since it have own precedense rules and is not binary expression but its own kind of expression // // Exclusion of them is marked by subtracting total number of excluded operators. - static_assert(Operators_Count - 1 == 14, "Ensure that all operators have defined precedense below"); + static_assert(Operators_Count - 1 == 15, "Ensure that all operators have defined precedense below"); if (one_of(op, "or")) return 100; if (one_of(op, "and")) return 150; if (one_of(op, "<", ">", "<=", ">=", "==", "!=")) return 200; if (one_of(op, "+", "-")) return 300; if (one_of(op, "*", "/", "%", "&")) return 400; + if (one_of(op, "**")) return 500; unreachable(); }