Merge pull request #10971 from ethereum/onlyWarnAboutVariables

Only warn about variables being shadowed in inline assembly.

Changelog.md

@@ -5,6 +5,7 @@ Language Features:
 
 Compiler Features:
  * AST: Export NatSpec comments above each statement as their documentation.
+ * Inline Assembly: Do not warn anymore about variables or functions being shadowed by EVM opcodes.
  * Optimizer: Simple inlining when jumping to small blocks that jump again after a few side-effect free opcodes.
 
 

libsolidity/analysis/NameAndTypeResolver.cpp

@@ -195,27 +195,6 @@ Declaration const* NameAndTypeResolver::pathFromCurrentScope(vector<ASTString> c
 		return nullptr;
 }
 
-void NameAndTypeResolver::warnVariablesNamedLikeInstructions() const
-{
-	for (auto const& instruction: evmasm::c_instructions)
-	{
-		string const instructionName{boost::algorithm::to_lower_copy(instruction.first)};
-		auto declarations = nameFromCurrentScope(instructionName, true);
-		for (Declaration const* const declaration: declarations)
-		{
-			solAssert(!!declaration, "");
-			if (dynamic_cast<MagicVariableDeclaration const* const>(declaration))
-				// Don't warn the user for what the user did not.
-				continue;
-			m_errorReporter.warning(
-				8261_error,
-				declaration->location(),
-				"Variable is shadowed in inline assembly by an instruction of the same name"
-			);
-		}
-	}
-}
-
 void NameAndTypeResolver::warnHomonymDeclarations() const
 {
 	DeclarationContainer::Homonyms homonyms;

libsolidity/analysis/NameAndTypeResolver.h

@@ -92,9 +92,6 @@ class NameAndTypeResolver: private boost::noncopyable
 	/// @note Returns a null pointer if any component in the path was not unique or not found.
 	Declaration const* pathFromCurrentScope(std::vector<ASTString> const& _path) const;
 
-	/// Generate and store warnings about variables that are named like instructions.
-	void warnVariablesNamedLikeInstructions() const;
-
 	/// Generate and store warnings about declarations with the same name.
 	void warnHomonymDeclarations() const;
 

libsolidity/analysis/ReferencesResolver.cpp

@@ -185,8 +185,6 @@ void ReferencesResolver::endVisit(IdentifierPath const& _path)
 
 bool ReferencesResolver::visit(InlineAssembly const& _inlineAssembly)
 {
-	m_resolver.warnVariablesNamedLikeInstructions();
-
 	m_yulAnnotation = &_inlineAssembly.annotation();
 	(*this)(_inlineAssembly.operations());
 	m_yulAnnotation = nullptr;

test/libsolidity/SolidityNameAndTypeResolution.cpp

@@ -379,58 +379,6 @@ BOOST_AUTO_TEST_CASE(warn_nonpresent_pragma)
 	BOOST_CHECK(searchErrorMessage(*sourceAndError.second.front(), "Source file does not specify required compiler version!"));
 }
 
-BOOST_AUTO_TEST_CASE(returndatasize_as_variable)
-{
-	char const* text = R"(
-		contract C { function f() public pure { uint returndatasize; returndatasize; assembly { pop(returndatasize()) }}}
-	)";
-	vector<pair<Error::Type, std::string>> expectations(vector<pair<Error::Type, std::string>>{
-		{Error::Type::Warning, "Variable is shadowed in inline assembly by an instruction of the same name"}
-	});
-	if (!solidity::test::CommonOptions::get().evmVersion().supportsReturndata())
-	{
-		expectations.emplace_back(make_pair(Error::Type::TypeError, std::string("\"returndatasize\" instruction is only available for Byzantium-compatible VMs")));
-		expectations.emplace_back(make_pair(Error::Type::TypeError, std::string("Expected expression to evaluate to one value, but got 0 values instead.")));
-	}
-	CHECK_ALLOW_MULTI(text, expectations);
-}
-
-BOOST_AUTO_TEST_CASE(create2_as_variable)
-{
-	char const* text = R"(
-		contract c { function f() public { uint create2; create2; assembly { pop(create2(0, 0, 0, 0)) } }}
-	)";
-	// This needs special treatment, because the message mentions the EVM version,
-	// so cannot be run via isoltest.
-	vector<pair<Error::Type, std::string>> expectations(vector<pair<Error::Type, std::string>>{
-		{Error::Type::Warning, "Variable is shadowed in inline assembly by an instruction of the same name"}
-	});
-	if (!solidity::test::CommonOptions::get().evmVersion().hasCreate2())
-	{
-		expectations.emplace_back(make_pair(Error::Type::TypeError, std::string("\"create2\" instruction is only available for Constantinople-compatible VMs")));
-		expectations.emplace_back(make_pair(Error::Type::TypeError, std::string("Expected expression to evaluate to one value, but got 0 values instead.")));
-	}
-	CHECK_ALLOW_MULTI(text, expectations);
-}
-
-BOOST_AUTO_TEST_CASE(extcodehash_as_variable)
-{
-	char const* text = R"(
-		contract c { function f() public view { uint extcodehash; extcodehash; assembly { pop(extcodehash(0)) } }}
-	)";
-	// This needs special treatment, because the message mentions the EVM version,
-	// so cannot be run via isoltest.
-	vector<pair<Error::Type, std::string>> expectations(vector<pair<Error::Type, std::string>>{
-		{Error::Type::Warning, "Variable is shadowed in inline assembly by an instruction of the same name"}
-	});
-	if (!solidity::test::CommonOptions::get().evmVersion().hasExtCodeHash())
-	{
-		expectations.emplace_back(make_pair(Error::Type::TypeError, std::string("\"extcodehash\" instruction is only available for Constantinople-compatible VMs")));
-		expectations.emplace_back(make_pair(Error::Type::TypeError, std::string("Expected expression to evaluate to one value, but got 0 values instead.")));
-	}
-	CHECK_ALLOW_MULTI(text, expectations);
-}
-
 BOOST_AUTO_TEST_CASE(getter_is_memory_type)
 {
 	char const* text = R"(

test/libsolidity/semanticTests/inlineAssembly/shadowing_local_function_opcode.sol

@@ -0,0 +1,13 @@
+contract C {
+    function add(uint, uint) public pure returns (uint) { return 7; }
+    function g() public pure returns (uint x, uint y) {
+        x = add(1, 2);
+        assembly {
+            y := add(1, 2)
+        }
+    }
+}
+// ====
+// compileViaYul: also
+// ----
+// g() -> 7, 3

test/libsolidity/syntaxTests/inlineAssembly/create2_as_variable_post_istanbul.sol

@@ -0,0 +1,9 @@
+contract c {
+	function f() public {
+		uint create2; create2;
+		assembly { pop(create2(0, 0, 0, 0)) }
+	}
+}
+// ====
+// EVMVersion: >=constantinople
+// ----

test/libsolidity/syntaxTests/inlineAssembly/create2_as_variable_pre_istanbul.sol

@@ -0,0 +1,11 @@
+contract c {
+	function f() public {
+		uint create2; create2;
+		assembly { pop(create2(0, 0, 0, 0)) }
+	}
+}
+// ====
+// EVMVersion: =byzantium
+// ----
+// TypeError 6166: (78-85): The "create2" instruction is only available for Constantinople-compatible VMs (you are currently compiling for "byzantium").
+// TypeError 3950: (78-97): Expected expression to evaluate to one value, but got 0 values instead.

test/libsolidity/syntaxTests/inlineAssembly/extcodehash_as_variable_post_constantinople.sol

@@ -0,0 +1,9 @@
+contract c {
+	function f() public view {
+		uint extcodehash;
+		extcodehash;
+		assembly { pop(extcodehash(0)) }
+	}
+}
+// ====
+// EVMVersion: >=constantinople

test/libsolidity/syntaxTests/inlineAssembly/extcodehash_as_variable_pre_constantinople.sol

@@ -0,0 +1,12 @@
+contract c {
+	function f() public view {
+		uint extcodehash;
+		extcodehash;
+		assembly { pop(extcodehash(0)) }
+	}
+}
+// ====
+// EVMVersion: =byzantium
+// ----
+// TypeError 7110: (93-104): The "extcodehash" instruction is only available for Constantinople-compatible VMs (you are currently compiling for "byzantium").
+// TypeError 3950: (93-107): Expected expression to evaluate to one value, but got 0 values instead.

test/libsolidity/syntaxTests/inlineAssembly/returndatasize_as_variable_call_post_byzantium.sol.sol

@@ -0,0 +1,11 @@
+contract C {
+    function f() public pure {
+        uint returndatasize;
+        returndatasize;
+        assembly {
+            let x := returndatasize()
+        }
+    }
+}
+// ====
+// EVMVersion: >=byzantium

test/libsolidity/syntaxTests/inlineAssembly/returndatasize_as_variable_post_byzantium.sol

@@ -0,0 +1,13 @@
+contract C {
+    function f() public pure {
+        uint returndatasize;
+        returndatasize;
+        assembly {
+            returndatasize := 2
+        }
+    }
+}
+// ====
+// EVMVersion: >=byzantium
+// ----
+// ParserError 6272: (143-145): Cannot assign to builtin function "returndatasize".

test/libsolidity/syntaxTests/inlineAssembly/returndatasize_as_variable_pre_byzantium.sol

@@ -0,0 +1,6 @@
+contract C { function f() public pure { uint returndatasize; returndatasize; assembly { pop(returndatasize()) }}}
+// ====
+// EVMVersion: =homestead
+// ----
+// TypeError 4778: (92-106): The "returndatasize" instruction is only available for Byzantium-compatible VMs (you are currently compiling for "homestead").
+// TypeError 3950: (92-108): Expected expression to evaluate to one value, but got 0 values instead.

test/libsolidity/syntaxTests/inlineAssembly/returndatasize_as_variable_read_post_byzantium.sol.sol

@@ -0,0 +1,13 @@
+contract C {
+    function f() public pure {
+        uint returndatasize;
+        returndatasize;
+        assembly {
+            let x := returndatasize
+        }
+    }
+}
+// ====
+// EVMVersion: >=byzantium
+// ----
+// ParserError 7104: (137-151): Builtin function "returndatasize" must be called.

test/libsolidity/syntaxTests/inlineAssembly/shadowing/global_function_by_opcode.sol

@@ -0,0 +1,8 @@
+function mload() pure {}
+contract C {
+    function g() public pure {
+        assembly {
+        }
+    }
+}
+// ----

test/libsolidity/syntaxTests/inlineAssembly/shadowing/local_function_by_opcode.sol

@@ -0,0 +1,9 @@
+contract C {
+    function add(uint, uint) public pure returns (uint) { return 7; }
+    function g() public pure returns (uint x, uint y) {
+        x = add(1, 2);
+        assembly {
+            y := add(1, 2)
+        }
+    }
+}

test/libsolidity/syntaxTests/inlineAssembly/shadowing/state_variable_by_opcode.sol

@@ -0,0 +1,8 @@
+contract C {
+    uint mload;
+    function g() public pure {
+        assembly {
+        }
+    }
+}
+// ----

test/libsolidity/syntaxTests/inlineAssembly/shadowing/variable_by_opcode.sol

@@ -9,6 +9,5 @@ contract C {
     }
 }
 // ----
-// Warning 8261: (109-119): Variable is shadowed in inline assembly by an instruction of the same name
 // Warning 2072: (52-62): Unused local variable.
 // Warning 2072: (109-119): Unused local variable.