explicitly disallow rational comparisons (#1221)

This is not permitted by solc either. Improve the diagnostic, and ensure the diagnostic gets surfaced even used within a overloaded function call. Signed-off-by: Sean Young <[email protected]>
hyperledger · Apr 6, 2023 · 3b03af1 · 3b03af1
1 parent 8cc4d51
commit 3b03af1
Show file tree

Hide file tree

Showing 5 changed files with 136 additions and 110 deletions.
diff --git a/src/sema/expression/arithmetic.rs b/src/sema/expression/arithmetic.rs
@@ -616,9 +616,11 @@ pub(super) fn equal(
     };
 
     if ty.is_rational() {
-        if let Err(diag) = eval_const_rational(&expr, ns) {
-            diagnostics.push(diag);
-        }
+        diagnostics.push(Diagnostic::error(
+            *loc,
+            "cannot use rational numbers with '!=' or '==' operator".into(),
+        ));
+        return Err(());
     }
 
     Ok(expr)

diff --git a/src/sema/expression/resolve_expression.rs b/src/sema/expression/resolve_expression.rs
@@ -25,7 +25,7 @@ use crate::sema::{
     {
         ast::{Builtin, Expression, Namespace, RetrieveType, Type},
         diagnostics::Diagnostics,
-        eval::{check_term_for_constant_overflow, eval_const_rational},
+        eval::check_term_for_constant_overflow,
     },
 };
 use num_bigint::BigInt;
@@ -165,9 +165,11 @@ pub fn expression(
             };
 
             if ty.is_rational() {
-                if let Err(diag) = eval_const_rational(&expr, ns) {
-                    diagnostics.push(diag);
-                }
+                diagnostics.push(Diagnostic::error(
+                    *loc,
+                    "cannot use rational numbers with '>' operator".into(),
+                ));
+                return Err(());
             }
 
             Ok(expr)
@@ -206,9 +208,11 @@ pub fn expression(
             };
 
             if ty.is_rational() {
-                if let Err(diag) = eval_const_rational(&expr, ns) {
-                    diagnostics.push(diag);
-                }
+                diagnostics.push(Diagnostic::error(
+                    *loc,
+                    "cannot use rational numbers with '<' operator".into(),
+                ));
+                return Err(());
             }
 
             Ok(expr)
@@ -246,9 +250,11 @@ pub fn expression(
             };
 
             if ty.is_rational() {
-                if let Err(diag) = eval_const_rational(&expr, ns) {
-                    diagnostics.push(diag);
-                }
+                diagnostics.push(Diagnostic::error(
+                    *loc,
+                    "cannot use rational numbers with '>=' operator".into(),
+                ));
+                return Err(());
             }
 
             Ok(expr)
@@ -286,9 +292,11 @@ pub fn expression(
             };
 
             if ty.is_rational() {
-                if let Err(diag) = eval_const_rational(&expr, ns) {
-                    diagnostics.push(diag);
-                }
+                diagnostics.push(Diagnostic::error(
+                    *loc,
+                    "cannot use rational numbers with '<=' operator".into(),
+                ));
+                return Err(());
             }
 
             Ok(expr)

diff --git a/tests/contract_testcases/solana/rational_comparison.dot b/tests/contract_testcases/solana/rational_comparison.dot
@@ -3,114 +3,52 @@ strict digraph "tests/contract_testcases/solana/rational_comparison.sol" {
 	foo1 [label="function foo1\ncontract: c\ntests/contract_testcases/solana/rational_comparison.sol:3:2-57\nsignature foo1(uint64,uint64)\nvisibility public\nmutability nonpayable"]
 	parameters [label="parameters\nuint64 a\nuint64 b"]
 	returns [label="returns\nbool "]
-	return [label="return\ntests/contract_testcases/solana/rational_comparison.sol:4:3-23"]
-	more_equal [label="more equal\ntests/contract_testcases/solana/rational_comparison.sol:4:10-23"]
-	cast [label="cast rational\ntests/contract_testcases/solana/rational_comparison.sol:4:11-14"]
-	divide [label="divide\nuint64\ntests/contract_testcases/solana/rational_comparison.sol:4:11-14"]
-	variable [label="variable: a\nuint64\ntests/contract_testcases/solana/rational_comparison.sol:4:11-12"]
-	variable_10 [label="variable: b\nuint64\ntests/contract_testcases/solana/rational_comparison.sol:4:13-14"]
-	rational_literal [label="rational rational literal: 1/20\ntests/contract_testcases/solana/rational_comparison.sol:4:19-23"]
 	foo2 [label="function foo2\ncontract: c\ntests/contract_testcases/solana/rational_comparison.sol:6:2-57\nsignature foo2(uint64,uint64)\nvisibility public\nmutability nonpayable"]
-	parameters_13 [label="parameters\nuint64 a\nuint64 b"]
-	returns_14 [label="returns\nbool "]
-	return_15 [label="return\ntests/contract_testcases/solana/rational_comparison.sol:7:3-19"]
-	more [label="more\ntests/contract_testcases/solana/rational_comparison.sol:7:10-19"]
-	rational_literal_17 [label="rational rational literal: 11/5\ntests/contract_testcases/solana/rational_comparison.sol:7:10-15"]
-	cast_18 [label="cast rational\ntests/contract_testcases/solana/rational_comparison.sol:7:18-19"]
-	variable_19 [label="variable: a\nuint64\ntests/contract_testcases/solana/rational_comparison.sol:7:18-19"]
+	parameters_6 [label="parameters\nuint64 a\nuint64 b"]
+	returns_7 [label="returns\nbool "]
 	foo3 [label="function foo3\ncontract: c\ntests/contract_testcases/solana/rational_comparison.sol:9:2-57\nsignature foo3(uint64,uint64)\nvisibility public\nmutability nonpayable"]
-	parameters_21 [label="parameters\nuint64 a\nuint64 b"]
-	returns_22 [label="returns\nbool "]
-	return_23 [label="return\ntests/contract_testcases/solana/rational_comparison.sol:10:3-19"]
-	equal [label="equal\ntests/contract_testcases/solana/rational_comparison.sol:10:10-19"]
-	rational_literal_25 [label="rational rational literal: 1\ntests/contract_testcases/solana/rational_comparison.sol:10:10-11"]
-	rational_literal_26 [label="rational rational literal: 1/20\ntests/contract_testcases/solana/rational_comparison.sol:10:15-19"]
+	parameters_9 [label="parameters\nuint64 a\nuint64 b"]
+	returns_10 [label="returns\nbool "]
 	foo4 [label="function foo4\ncontract: c\ntests/contract_testcases/solana/rational_comparison.sol:12:2-57\nsignature foo4(uint64,uint64)\nvisibility public\nmutability nonpayable"]
-	parameters_28 [label="parameters\nuint64 a\nuint64 b"]
-	returns_29 [label="returns\nbool "]
+	parameters_12 [label="parameters\nuint64 a\nuint64 b"]
+	returns_13 [label="returns\nbool "]
 	foo5 [label="function foo5\ncontract: c\ntests/contract_testcases/solana/rational_comparison.sol:15:2-57\nsignature foo5(uint64,uint64)\nvisibility public\nmutability nonpayable"]
-	parameters_31 [label="parameters\nuint64 a\nuint64 b"]
-	returns_32 [label="returns\nbool "]
-	return_33 [label="return\ntests/contract_testcases/solana/rational_comparison.sol:16:3-26"]
-	less_equal [label="less equal\ntests/contract_testcases/solana/rational_comparison.sol:16:10-26"]
-	cast_35 [label="cast rational\ntests/contract_testcases/solana/rational_comparison.sol:16:11-17"]
-	shift_left [label="shift left\nuint64\ntests/contract_testcases/solana/rational_comparison.sol:16:11-17"]
-	variable_37 [label="variable: a\nuint64\ntests/contract_testcases/solana/rational_comparison.sol:16:11-12"]
-	variable_38 [label="variable: b\nuint64\ntests/contract_testcases/solana/rational_comparison.sol:16:16-17"]
-	rational_literal_39 [label="rational rational literal: 1/20\ntests/contract_testcases/solana/rational_comparison.sol:16:22-26"]
+	parameters_15 [label="parameters\nuint64 a\nuint64 b"]
+	returns_16 [label="returns\nbool "]
 	foo6 [label="function foo6\ncontract: c\ntests/contract_testcases/solana/rational_comparison.sol:18:2-57\nsignature foo6(uint64,uint64)\nvisibility public\nmutability nonpayable"]
-	parameters_41 [label="parameters\nuint64 a\nuint64 b"]
-	returns_42 [label="returns\nbool "]
-	return_43 [label="return\ntests/contract_testcases/solana/rational_comparison.sol:19:3-24"]
-	not [label="not\ntests/contract_testcases/solana/rational_comparison.sol:19:10-24"]
-	equal_45 [label="equal\ntests/contract_testcases/solana/rational_comparison.sol:19:10-24"]
-	rational_literal_46 [label="rational rational literal: 6/5\ntests/contract_testcases/solana/rational_comparison.sol:19:10-13"]
-	cast_47 [label="cast rational\ntests/contract_testcases/solana/rational_comparison.sol:19:18-23"]
-	bitwise_xor [label="bitwise xor\nuint64\ntests/contract_testcases/solana/rational_comparison.sol:19:18-23"]
-	variable_49 [label="variable: a\nuint64\ntests/contract_testcases/solana/rational_comparison.sol:19:18-19"]
-	variable_50 [label="variable: b\nuint64\ntests/contract_testcases/solana/rational_comparison.sol:19:22-23"]
+	parameters_18 [label="parameters\nuint64 a\nuint64 b"]
+	returns_19 [label="returns\nbool "]
 	diagnostic [label="found contract 'c'\nlevel Debug\ntests/contract_testcases/solana/rational_comparison.sol:2:1-21:2"]
-	diagnostic_53 [label="expression not allowed in constant rational number expression\nlevel Error\ntests/contract_testcases/solana/rational_comparison.sol:4:10-23"]
-	diagnostic_54 [label="expression not allowed in constant rational number expression\nlevel Error\ntests/contract_testcases/solana/rational_comparison.sol:7:10-19"]
-	diagnostic_55 [label="expression not allowed in constant rational number expression\nlevel Error\ntests/contract_testcases/solana/rational_comparison.sol:10:10-19"]
-	diagnostic_56 [label="expression not allowed in constant rational number expression\nlevel Error\ntests/contract_testcases/solana/rational_comparison.sol:13:10-11"]
-	diagnostic_57 [label="expression not allowed in constant rational number expression\nlevel Error\ntests/contract_testcases/solana/rational_comparison.sol:16:10-26"]
-	diagnostic_58 [label="expression not allowed in constant rational number expression\nlevel Error\ntests/contract_testcases/solana/rational_comparison.sol:19:10-24"]
+	diagnostic_22 [label="cannot use rational numbers with '>=' operator\nlevel Error\ntests/contract_testcases/solana/rational_comparison.sol:4:10-23"]
+	diagnostic_23 [label="cannot use rational numbers with '>' operator\nlevel Error\ntests/contract_testcases/solana/rational_comparison.sol:7:10-19"]
+	diagnostic_24 [label="cannot use rational numbers with '!=' or '==' operator\nlevel Error\ntests/contract_testcases/solana/rational_comparison.sol:10:10-19"]
+	diagnostic_25 [label="expression not allowed in constant rational number expression\nlevel Error\ntests/contract_testcases/solana/rational_comparison.sol:13:10-11"]
+	diagnostic_26 [label="cannot use rational numbers with '<=' operator\nlevel Error\ntests/contract_testcases/solana/rational_comparison.sol:16:10-26"]
+	diagnostic_27 [label="cannot use rational numbers with '!=' or '==' operator\nlevel Error\ntests/contract_testcases/solana/rational_comparison.sol:19:10-24"]
 	contracts -> contract
 	contract -> foo1 [label="function"]
 	foo1 -> parameters [label="parameters"]
 	foo1 -> returns [label="returns"]
-	foo1 -> return [label="body"]
-	return -> more_equal [label="expr"]
-	more_equal -> cast [label="left"]
-	cast -> divide [label="expr"]
-	divide -> variable [label="left"]
-	divide -> variable_10 [label="right"]
-	more_equal -> rational_literal [label="right"]
 	contract -> foo2 [label="function"]
-	foo2 -> parameters_13 [label="parameters"]
-	foo2 -> returns_14 [label="returns"]
-	foo2 -> return_15 [label="body"]
-	return_15 -> more [label="expr"]
-	more -> rational_literal_17 [label="left"]
-	more -> cast_18 [label="right"]
-	cast_18 -> variable_19 [label="expr"]
+	foo2 -> parameters_6 [label="parameters"]
+	foo2 -> returns_7 [label="returns"]
 	contract -> foo3 [label="function"]
-	foo3 -> parameters_21 [label="parameters"]
-	foo3 -> returns_22 [label="returns"]
-	foo3 -> return_23 [label="body"]
-	return_23 -> equal [label="expr"]
-	equal -> rational_literal_25 [label="left"]
-	equal -> rational_literal_26 [label="right"]
+	foo3 -> parameters_9 [label="parameters"]
+	foo3 -> returns_10 [label="returns"]
 	contract -> foo4 [label="function"]
-	foo4 -> parameters_28 [label="parameters"]
-	foo4 -> returns_29 [label="returns"]
+	foo4 -> parameters_12 [label="parameters"]
+	foo4 -> returns_13 [label="returns"]
 	contract -> foo5 [label="function"]
-	foo5 -> parameters_31 [label="parameters"]
-	foo5 -> returns_32 [label="returns"]
-	foo5 -> return_33 [label="body"]
-	return_33 -> less_equal [label="expr"]
-	less_equal -> cast_35 [label="left"]
-	cast_35 -> shift_left [label="expr"]
-	shift_left -> variable_37 [label="left"]
-	shift_left -> variable_38 [label="right"]
-	less_equal -> rational_literal_39 [label="right"]
+	foo5 -> parameters_15 [label="parameters"]
+	foo5 -> returns_16 [label="returns"]
 	contract -> foo6 [label="function"]
-	foo6 -> parameters_41 [label="parameters"]
-	foo6 -> returns_42 [label="returns"]
-	foo6 -> return_43 [label="body"]
-	return_43 -> not [label="expr"]
-	not -> equal_45 [label="expr"]
-	equal_45 -> rational_literal_46 [label="left"]
-	equal_45 -> cast_47 [label="right"]
-	cast_47 -> bitwise_xor [label="expr"]
-	bitwise_xor -> variable_49 [label="left"]
-	bitwise_xor -> variable_50 [label="right"]
+	foo6 -> parameters_18 [label="parameters"]
+	foo6 -> returns_19 [label="returns"]
 	diagnostics -> diagnostic [label="Debug"]
-	diagnostics -> diagnostic_53 [label="Error"]
-	diagnostics -> diagnostic_54 [label="Error"]
-	diagnostics -> diagnostic_55 [label="Error"]
-	diagnostics -> diagnostic_56 [label="Error"]
-	diagnostics -> diagnostic_57 [label="Error"]
-	diagnostics -> diagnostic_58 [label="Error"]
+	diagnostics -> diagnostic_22 [label="Error"]
+	diagnostics -> diagnostic_23 [label="Error"]
+	diagnostics -> diagnostic_24 [label="Error"]
+	diagnostics -> diagnostic_25 [label="Error"]
+	diagnostics -> diagnostic_26 [label="Error"]
+	diagnostics -> diagnostic_27 [label="Error"]
 }
diff --git a/tests/contract_testcases/solana/rational_comparison2.dot b/tests/contract_testcases/solana/rational_comparison2.dot
@@ -0,0 +1,48 @@
+strict digraph "tests/contract_testcases/solana/rational_comparison2.sol" {
+	contract [label="contract c\ntests/contract_testcases/solana/rational_comparison2.sol:1:1-29:2"]
+	eq [label="function eq\ncontract: c\ntests/contract_testcases/solana/rational_comparison2.sol:2:2-37\nsignature eq()\nvisibility public\nmutability nonpayable"]
+	returns [label="returns\nbool "]
+	ne [label="function ne\ncontract: c\ntests/contract_testcases/solana/rational_comparison2.sol:6:2-37\nsignature ne()\nvisibility public\nmutability nonpayable"]
+	returns_5 [label="returns\nbool "]
+	lt [label="function lt\ncontract: c\ntests/contract_testcases/solana/rational_comparison2.sol:10:2-22\nsignature lt()\nvisibility public\nmutability nonpayable"]
+	le [label="function le\ncontract: c\ntests/contract_testcases/solana/rational_comparison2.sol:14:2-28\nsignature le(bool)\nvisibility public\nmutability nonpayable"]
+	parameters [label="parameters\nbool a"]
+	gt [label="function gt\ncontract: c\ntests/contract_testcases/solana/rational_comparison2.sol:18:2-28\nsignature gt(bool)\nvisibility public\nmutability nonpayable"]
+	parameters_10 [label="parameters\nbool a"]
+	gt_11 [label="function gt\ncontract: c\ntests/contract_testcases/solana/rational_comparison2.sol:22:2-42\nsignature gt(int256)\nvisibility public\nmutability nonpayable"]
+	parameters_12 [label="parameters\nint256 a"]
+	returns_13 [label="returns\nbool "]
+	ge [label="function ge\ncontract: c\ntests/contract_testcases/solana/rational_comparison2.sol:26:2-28\nsignature ge(bool)\nvisibility public\nmutability nonpayable"]
+	parameters_15 [label="parameters\nbool a"]
+	diagnostic [label="found contract 'c'\nlevel Debug\ntests/contract_testcases/solana/rational_comparison2.sol:1:1-29:2"]
+	diagnostic_18 [label="cannot use rational numbers with '!=' or '==' operator\nlevel Error\ntests/contract_testcases/solana/rational_comparison2.sol:3:10-20"]
+	diagnostic_19 [label="cannot use rational numbers with '!=' or '==' operator\nlevel Error\ntests/contract_testcases/solana/rational_comparison2.sol:7:10-20"]
+	diagnostic_20 [label="cannot use rational numbers with '<' operator\nlevel Error\ntests/contract_testcases/solana/rational_comparison2.sol:11:11-20"]
+	diagnostic_21 [label="cannot use rational numbers with '<=' operator\nlevel Error\ntests/contract_testcases/solana/rational_comparison2.sol:15:16-26"]
+	diagnostic_22 [label="cannot use rational numbers with '>' operator\nlevel Error\ntests/contract_testcases/solana/rational_comparison2.sol:19:7-14"]
+	diagnostic_23 [label="cannot use rational numbers with '>' operator\nlevel Error\ntests/contract_testcases/solana/rational_comparison2.sol:23:10-17"]
+	diagnostic_24 [label="cannot use rational numbers with '>=' operator\nlevel Error\ntests/contract_testcases/solana/rational_comparison2.sol:27:6-15"]
+	contracts -> contract
+	contract -> eq [label="function"]
+	eq -> returns [label="returns"]
+	contract -> ne [label="function"]
+	ne -> returns_5 [label="returns"]
+	contract -> lt [label="function"]
+	contract -> le [label="function"]
+	le -> parameters [label="parameters"]
+	contract -> gt [label="function"]
+	gt -> parameters_10 [label="parameters"]
+	contract -> gt_11 [label="function"]
+	gt_11 -> parameters_12 [label="parameters"]
+	gt_11 -> returns_13 [label="returns"]
+	contract -> ge [label="function"]
+	ge -> parameters_15 [label="parameters"]
+	diagnostics -> diagnostic [label="Debug"]
+	diagnostics -> diagnostic_18 [label="Error"]
+	diagnostics -> diagnostic_19 [label="Error"]
+	diagnostics -> diagnostic_20 [label="Error"]
+	diagnostics -> diagnostic_21 [label="Error"]
+	diagnostics -> diagnostic_22 [label="Error"]
+	diagnostics -> diagnostic_23 [label="Error"]
+	diagnostics -> diagnostic_24 [label="Error"]
+}
diff --git a/tests/contract_testcases/solana/rational_comparison2.sol b/tests/contract_testcases/solana/rational_comparison2.sol
@@ -0,0 +1,30 @@
+contract c {
+	function eq() public returns (bool) {
+		return 1.1 == 1.0;
+	}
+
+	function ne() public returns (bool) {
+		return 1.1 != 1.0;
+	}
+
+	function lt() public {
+		require(1.0 < 1.1);
+	}
+
+	function le(bool a) public {
+		require(a && 0.1 <= 1.1);
+	}
+
+	function gt(bool a) public {
+		if (1 > 0.5) { }
+	}
+
+	function gt(int a) public returns (bool) {
+		return a > 1.1;
+	}
+
+	function ge(bool a) public {
+		gt(1 >= 1.02);
+	}
+}
+