Merge pull request #10971 from ethereum/onlyWarnAboutVariables

Only warn about variables being shadowed in inline assembly.
This commit is contained in:
Harikrishnan Mulackal 2021-02-19 16:17:02 +01:00 committed by GitHub
commit 6fd5ea01d1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
18 changed files with 123 additions and 79 deletions

View File

@ -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.

View File

@ -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;

View File

@ -92,9 +92,6 @@ public:
/// @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;

View File

@ -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;

View File

@ -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"(

View File

@ -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

View File

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

View File

@ -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.

View File

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

View File

@ -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.

View File

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

View File

@ -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".

View File

@ -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.

View File

@ -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.

View File

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

View File

@ -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)
}
}
}

View File

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

View File

@ -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.