Apply suggestions from @certik

dylon · Jun 21, 2022 · 66dded6 · 66dded6
1 parent dd526d9
commit 66dded6
Show file tree

Hide file tree

Showing 8 changed files with 25 additions and 26 deletions.
diff --git a/integration_tests/expr_05.py b/integration_tests/expr_05.py
@@ -26,9 +26,9 @@ def main0():
     a = -5345
     b = -534
     assert a % b == -5
-    # a = -123282374
-    # b = 32771
-    # assert test_mod(a, b) == 2128
+    a = -123282374
+    b = 32771
+    assert test_mod(a, b) == 2128
 
     assert 10 | 4 == 14
     assert -105346 | -32771 == -32769

diff --git a/src/libasr/ASR.asdl b/src/libasr/ASR.asdl
@@ -318,7 +318,7 @@ ttype
     | Pointer(ttype type)
     | CPtr()
 
-binop = Add | Sub | Mul | Div | Pow | Mod | BitAnd | BitOr | BitXor | BitLShift | BitRShift
+binop = Add | Sub | Mul | Div | Pow | BitAnd | BitOr | BitXor | BitLShift | BitRShift
 
 logicalbinop = And | Or | Xor | NEqv | Eqv
 

diff --git a/src/libasr/asr_utils.cpp b/src/libasr/asr_utils.cpp
@@ -388,12 +388,6 @@ bool is_op_overloaded(ASR::binopType op, std::string& intrinsic_op_name,
             }
             break;
         }
-        case ASR::binopType::Mod: {
-            if(intrinsic_op_name != "~mod") {
-                result = false;
-            }
-            break;
-        }
         default: {
             throw LFortranException("Binary operator '" + ASRUtils::binop_to_str(op) + "' not supported yet");
         }

diff --git a/src/libasr/asr_utils.h b/src/libasr/asr_utils.h
@@ -261,7 +261,6 @@ static inline std::string binop_to_str(const ASR::binopType t) {
         case (ASR::binopType::Sub): { return " - "; }
         case (ASR::binopType::Mul): { return "*"; }
         case (ASR::binopType::Div): { return "/"; }
-        case (ASR::binopType::Mod): { return "%"; }
         case (ASR::binopType::BitAnd): { return "&"; }
         case (ASR::binopType::BitOr): { return "|"; }
         case (ASR::binopType::BitXor): { return "^"; }

diff --git a/src/libasr/codegen/asr_to_llvm.cpp b/src/libasr/codegen/asr_to_llvm.cpp
@@ -3258,10 +3258,6 @@ class ASRToLLVMVisitor : public ASR::BaseVisitor<ASRToLLVMVisitor>
                 tmp = builder->CreateFPToSI(tmp, type);
                 break;
             };
-            case ASR::binopType::Mod: {
-                tmp = builder->CreateSRem(left_val, right_val);
-                break;
-            }
             case ASR::binopType::BitOr: {
                 tmp = builder->CreateOr(left_val, right_val);
                 break;
@@ -3282,10 +3278,6 @@ class ASRToLLVMVisitor : public ASR::BaseVisitor<ASRToLLVMVisitor>
                 tmp = builder->CreateAShr(left_val, right_val);
                 break;
             }
-            default: {
-                throw CodeGenError("Binary operator '" + ASRUtils::binop_to_str(x.m_op) + "' not supported yet",
-                    x.base.base.loc);
-            }
         }
     }
 

diff --git a/src/lpython/semantics/python_ast_to_asr.cpp b/src/lpython/semantics/python_ast_to_asr.cpp
@@ -1251,7 +1251,6 @@ class CommonVisitor : public AST::BaseVisitor<Derived> {
                     case (ASR::binopType::Mul): { result = left_value * right_value; break; }
                     case (ASR::binopType::Div): { result = left_value / right_value; break; }
                     case (ASR::binopType::Pow): { result = std::pow(left_value, right_value); break; }
-                    case (ASR::binopType::Mod): { result = left_value % right_value; break; }
                     case (ASR::binopType::BitAnd): { result = left_value & right_value; break; }
                     case (ASR::binopType::BitOr): { result = left_value | right_value; break; }
                     case (ASR::binopType::BitXor): { result = left_value ^ right_value; break; }
@@ -1280,7 +1279,7 @@ class CommonVisitor : public AST::BaseVisitor<Derived> {
         } else if (ASRUtils::is_real(*dest_type)) {
 
             if (op == ASR::binopType::BitAnd || op == ASR::binopType::BitOr || op == ASR::binopType::BitXor ||
-                op == ASR::binopType::BitLShift || op == ASR::binopType::BitRShift || op == ASR::binopType::Mod) {
+                op == ASR::binopType::BitLShift || op == ASR::binopType::BitRShift) {
                 throw SemanticError("Unsupported binary operation on floats: '" + ASRUtils::binop_to_str(op) + "'", loc);
             }
 
@@ -1307,7 +1306,7 @@ class CommonVisitor : public AST::BaseVisitor<Derived> {
         } else if (ASRUtils::is_complex(*dest_type)) {
 
             if (op == ASR::binopType::BitAnd || op == ASR::binopType::BitOr || op == ASR::binopType::BitXor ||
-                op == ASR::binopType::BitLShift || op == ASR::binopType::BitRShift || op == ASR::binopType::Mod) {
+                op == ASR::binopType::BitLShift || op == ASR::binopType::BitRShift) {
                 throw SemanticError("Unsupported binary operation on complex: '" + ASRUtils::binop_to_str(op) + "'", loc);
             }
 
@@ -1587,26 +1586,41 @@ class CommonVisitor : public AST::BaseVisitor<Derived> {
         this->visit_expr(*x.m_right);
         ASR::expr_t *right = ASRUtils::EXPR(tmp);
         ASR::binopType op;
+        std::string op_name = "";
         switch (x.m_op) {
             case (AST::operatorType::Add) : { op = ASR::binopType::Add; break; }
             case (AST::operatorType::Sub) : { op = ASR::binopType::Sub; break; }
             case (AST::operatorType::Mult) : { op = ASR::binopType::Mul; break; }
             case (AST::operatorType::Div) : { op = ASR::binopType::Div; break; }
             case (AST::operatorType::FloorDiv) : {op = ASR::binopType::Div; break;}
             case (AST::operatorType::Pow) : { op = ASR::binopType::Pow; break; }
-            case (AST::operatorType::Mod) : { op = ASR::binopType::Mod; break; }
             case (AST::operatorType::BitOr) : { op = ASR::binopType::BitOr; break; }
             case (AST::operatorType::BitAnd) : { op = ASR::binopType::BitAnd; break; }
             case (AST::operatorType::BitXor) : { op = ASR::binopType::BitXor; break; }
             case (AST::operatorType::LShift) : { op = ASR::binopType::BitLShift; break; }
             case (AST::operatorType::RShift) : { op = ASR::binopType::BitRShift; break; }
+            case (AST::operatorType::Mod) : { op_name = "_mod"; break; }
             default : {
                 throw SemanticError("Binary operator type not supported",
                     x.base.base.loc);
             }
         }
         left = cast_helper(ASRUtils::expr_type(right), left);
         right = cast_helper(ASRUtils::expr_type(left), right);
+        Vec<ASR::call_arg_t> args;
+        args.reserve(al, 2);
+        ASR::call_arg_t arg1, arg2;
+        arg1.loc = left->base.loc;
+        arg1.m_value = left;
+        args.push_back(al, arg1);
+        arg2.loc = right->base.loc;
+        arg2.m_value = right;
+        args.push_back(al, arg2);
+        if (op_name != "") {
+            ASR::symbol_t *fn_mod = resolve_intrinsic_function(x.base.base.loc, op_name);
+            tmp = make_call_helper(al, fn_mod, current_scope, args, op_name, x.base.base.loc);
+            return;
+        }
         bool floordiv = (x.m_op == AST::operatorType::FloorDiv);
         make_BinOp_helper(left, right, op, x.base.base.loc, floordiv);
     }

diff --git a/tests/reference/asr-expr_05-3a37324.json b/tests/reference/asr-expr_05-3a37324.json
@@ -2,11 +2,11 @@
     "basename": "asr-expr_05-3a37324",
     "cmd": "lpython --show-asr --no-color {infile} -o {outfile}",
     "infile": "tests/../integration_tests/expr_05.py",
-    "infile_hash": "8be13a1b6c09027486e5fbc4dc299238f9a99ea95685a2cb636752d4",
+    "infile_hash": "fe09da9f78c192cc7a912fd373a636f5a59afc5e76c798fd9e2cbc9d",
     "outfile": null,
     "outfile_hash": null,
     "stdout": "asr-expr_05-3a37324.stdout",
-    "stdout_hash": "37648f0ebdf9a053171005671c1c757d04f66966873286c42d16e8e0",
+    "stdout_hash": "2038bbe6615273d2f9313d731251690baefed1292d687b065ca4a766",
     "stderr": null,
     "stderr_hash": null,
     "returncode": 0

diff --git a/tests/reference/asr-expr_05-3a37324.stdout b/tests/reference/asr-expr_05-3a37324.stdout