Add unimplemented assert for modifiers in the IR

Also improved relevant tests for modifiers to be more strict and detect if the modifier body was skipped.

libsolidity/codegen/ir/IRGenerator.cpp

@@ -249,6 +249,7 @@ string IRGenerator::generateFunction(FunctionDefinition const& _function)
 {
 	string functionName = IRNames::function(_function);
 	return m_context.functionCollector().createFunction(functionName, [&]() {
+		solUnimplementedAssert(_function.modifiers().empty(), "Modifiers not implemented yet.");
 		Whiskers t(R"(
 			function <functionName>(<params>)<?+retParams> -> <retParams></+retParams> {
 				<initReturnVariables>
@@ -521,8 +522,16 @@ void IRGenerator::generateImplicitConstructors(ContractDefinition const& _contra
 			)");
 			vector<string> params;
 			if (contract->constructor())
+			{
+				for (auto const& modifierInvocation: contract->constructor()->modifiers())
+					// This can be ContractDefinition too for super arguments. That is supported.
+					solUnimplementedAssert(
+						!dynamic_cast<ModifierDefinition const*>(modifierInvocation->name()->annotation().referencedDeclaration),
+						"Modifiers not implemented yet."
+					);
 				for (ASTPointer<VariableDeclaration> const& varDecl: contract->constructor()->parameters())
 					params += m_context.addLocalVariable(*varDecl).stackSlots();
+			}
 			t("params", joinHumanReadable(params));
 			vector<string> baseParams = listAllParams(baseConstructorParams);
 			t("baseParams", joinHumanReadable(baseParams));

test/libsolidity/SolidityEndToEndTest.cpp

@@ -4481,30 +4481,6 @@ BOOST_AUTO_TEST_CASE(non_payable_throw)
 	BOOST_CHECK_EQUAL(balanceAt(m_contractAddress), 0);
 }
 
-BOOST_AUTO_TEST_CASE(no_nonpayable_circumvention_by_modifier)
-{
-	char const* sourceCode = R"(
-		contract C {
-			modifier tryCircumvent {
-				if (false) _; // avoid the function, we should still not accept ether
-			}
-			function f() tryCircumvent public returns (uint) {
-				return msgvalue();
-			}
-			function msgvalue() internal returns (uint) {
-				return msg.value;
-			}
-		}
-	)";
-	ALSO_VIA_YUL(
-		DISABLE_EWASM_TESTRUN()
-
-		compileAndRun(sourceCode);
-		ABI_CHECK(callContractFunctionWithValue("f()", 27), encodeArgs());
-		BOOST_CHECK_EQUAL(balanceAt(m_contractAddress), 0);
-	)
-}
-
 BOOST_AUTO_TEST_CASE(mem_resize_is_not_paid_at_call)
 {
 	// This tests that memory resize for return values is not paid during the call, which would

test/libsolidity/semanticTests/inlineAssembly/inline_assembly_in_modifiers.sol

@@ -11,10 +11,23 @@ contract C {
     function f() public m returns (bool) {
         return true;
     }
+
+    modifier n {
+        uint256 a = 1;
+        assembly {
+            a := 2
+        }
+        if (a != 2)
+            _;
+        revert();
+    }
+
+    function g() public n returns (bool) {
+        // This statement should never execute.
+        return true;
+    }
 }
 
-// ====
-// compileViaYul: also
-// compileToEwasm: also
 // ----
 // f() -> true
+// g() -> FAILURE

test/libsolidity/semanticTests/modifiers/break_in_modifier.sol

@@ -3,7 +3,8 @@ contract C {
     modifier run() {
         for (uint256 i = 0; i < 10; i++) {
             _;
-            break;
+            if (i == 1)
+                break;
         }
     }
 
@@ -13,10 +14,7 @@ contract C {
         x = t;
     }
 }
-// ====
-// compileViaYul: also
-// compileToEwasm: also
 // ----
 // x() -> 0
 // f() ->
-// x() -> 1
+// x() -> 2

test/libsolidity/semanticTests/modifiers/stacked_return_with_modifiers.sol

@@ -1,22 +1,21 @@
 contract C {
     uint256 public x;
-    modifier run() {
+    modifier m() {
         for (uint256 i = 0; i < 10; i++) {
             _;
-            break;
+            ++x;
+            return;
         }
     }
 
-    function f() public run {
-        uint256 k = x;
-        uint256 t = k + 1;
-        x = t;
+    function f() public m m m returns (uint) {
+        for (uint256 i = 0; i < 10; i++) {
+            ++x;
+            return 42;
+        }
     }
 }
-// ====
-// compileViaYul: also
-// compileToEwasm: also
 // ----
 // x() -> 0
-// f() ->
-// x() -> 1
+// f() -> 42
+// x() -> 4

test/libsolidity/semanticTests/payable/no_nonpayable_circumvention_by_modifier.sol

@@ -0,0 +1,18 @@
+contract C {
+    modifier tryCircumvent {
+        if (false) _; // avoid the function, we should still not accept ether
+    }
+    function f() tryCircumvent public returns (uint) {
+        return msgvalue();
+    }
+    function msgvalue() internal returns (uint) {
+        return msg.value;
+    }
+    // TODO: remove this helper function once isoltest supports balance checking
+    function balance() external returns (uint) {
+        return address(this).balance;
+    }
+}
+// ----
+// f(), 27 wei -> FAILURE
+// balance() -> 0