diff --git a/Changelog.md b/Changelog.md index 8ffe5df6b..78ada7e3f 100644 --- a/Changelog.md +++ b/Changelog.md @@ -33,6 +33,7 @@ Bugfixes: * SMTChecker: SSA control-flow did not take into account state variables that were modified inside inlined functions that were called inside branches. * Type System: Use correct type name for contracts in event parameters when used in libraries. This affected code generation. * Type System: Allow direct call to base class functions that have overloads. + * Type System: Warn about shadowing builtin variables if user variables are named ``this`` or ``super``. * Yul: Properly register functions and disallow shadowing between function variables and variables in the outside scope. diff --git a/libsolidity/analysis/NameAndTypeResolver.cpp b/libsolidity/analysis/NameAndTypeResolver.cpp index 2b64a3ddb..727dc5f35 100644 --- a/libsolidity/analysis/NameAndTypeResolver.cpp +++ b/libsolidity/analysis/NameAndTypeResolver.cpp @@ -37,16 +37,17 @@ namespace solidity { NameAndTypeResolver::NameAndTypeResolver( - vector const& _globals, + GlobalContext& _globalContext, map>& _scopes, ErrorReporter& _errorReporter ) : m_scopes(_scopes), - m_errorReporter(_errorReporter) + m_errorReporter(_errorReporter), + m_globalContext(_globalContext) { if (!m_scopes[nullptr]) m_scopes[nullptr].reset(new DeclarationContainer()); - for (Declaration const* declaration: _globals) + for (Declaration const* declaration: _globalContext.declarations()) { solAssert(m_scopes[nullptr]->registerDeclaration(*declaration), "Unable to register global declaration."); } @@ -57,7 +58,7 @@ bool NameAndTypeResolver::registerDeclarations(SourceUnit& _sourceUnit, ASTNode // The helper registers all declarations in m_scopes as a side-effect of its construction. try { - DeclarationRegistrationHelper registrar(m_scopes, _sourceUnit, m_errorReporter, _currentScope); + DeclarationRegistrationHelper registrar(m_scopes, _sourceUnit, m_errorReporter, m_globalContext, _currentScope); } catch (langutil::FatalError const&) { @@ -272,6 +273,10 @@ bool NameAndTypeResolver::resolveNamesAndTypesInternal(ASTNode& _node, bool _res setScope(contract->scope()); solAssert(!!m_currentScope, ""); + m_globalContext.setCurrentContract(*contract); + updateDeclaration(*m_globalContext.currentSuper()); + updateDeclaration(*m_globalContext.currentThis()); + for (ASTPointer const& baseContract: contract->baseContracts()) if (!resolveNamesAndTypes(*baseContract, true)) success = false; @@ -452,11 +457,13 @@ DeclarationRegistrationHelper::DeclarationRegistrationHelper( map>& _scopes, ASTNode& _astRoot, ErrorReporter& _errorReporter, + GlobalContext& _globalContext, ASTNode const* _currentScope ): m_scopes(_scopes), m_currentScope(_currentScope), - m_errorReporter(_errorReporter) + m_errorReporter(_errorReporter), + m_globalContext(_globalContext) { _astRoot.accept(*this); solAssert(m_currentScope == _currentScope, "Scopes not correctly closed."); @@ -560,6 +567,10 @@ bool DeclarationRegistrationHelper::visit(ImportDirective& _import) bool DeclarationRegistrationHelper::visit(ContractDefinition& _contract) { + m_globalContext.setCurrentContract(_contract); + m_scopes[nullptr]->registerDeclaration(*m_globalContext.currentThis(), nullptr, false, true); + m_scopes[nullptr]->registerDeclaration(*m_globalContext.currentSuper(), nullptr, false, true); + registerDeclaration(_contract, true); _contract.annotation().canonicalName = currentCanonicalName(); return true; diff --git a/libsolidity/analysis/NameAndTypeResolver.h b/libsolidity/analysis/NameAndTypeResolver.h index a3b78f0e4..e5f5774ab 100644 --- a/libsolidity/analysis/NameAndTypeResolver.h +++ b/libsolidity/analysis/NameAndTypeResolver.h @@ -23,6 +23,7 @@ #pragma once #include +#include #include #include #include @@ -53,7 +54,7 @@ public: /// @param _scopes mapping of scopes to be used (usually default constructed), these /// are filled during the lifetime of this object. NameAndTypeResolver( - std::vector const& _globals, + GlobalContext& _globalContext, std::map>& _scopes, langutil::ErrorReporter& _errorReporter ); @@ -131,6 +132,7 @@ private: DeclarationContainer* m_currentScope = nullptr; langutil::ErrorReporter& m_errorReporter; + GlobalContext& m_globalContext; }; /** @@ -148,6 +150,7 @@ public: std::map>& _scopes, ASTNode& _astRoot, langutil::ErrorReporter& _errorReporter, + GlobalContext& _globalContext, ASTNode const* _currentScope = nullptr ); @@ -200,6 +203,7 @@ private: ASTNode const* m_currentScope = nullptr; VariableScope* m_currentFunction = nullptr; langutil::ErrorReporter& m_errorReporter; + GlobalContext& m_globalContext; }; } diff --git a/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp index 7489a13f8..45764f768 100644 --- a/libsolidity/interface/CompilerStack.cpp +++ b/libsolidity/interface/CompilerStack.cpp @@ -260,7 +260,7 @@ bool CompilerStack::analyze() noErrors = false; m_globalContext = make_shared(); - NameAndTypeResolver resolver(m_globalContext->declarations(), m_scopes, m_errorReporter); + NameAndTypeResolver resolver(*m_globalContext, m_scopes, m_errorReporter); for (Source const* source: m_sourceOrder) if (!resolver.registerDeclarations(*source->ast)) return false; @@ -278,11 +278,8 @@ bool CompilerStack::analyze() for (ASTPointer const& node: source->ast->nodes()) if (ContractDefinition* contract = dynamic_cast(node.get())) { - m_globalContext->setCurrentContract(*contract); - if (!resolver.updateDeclaration(*m_globalContext->currentThis())) return false; - if (!resolver.updateDeclaration(*m_globalContext->currentSuper())) return false; - if (!resolver.resolveNamesAndTypes(*contract)) return false; + if (!resolver.resolveNamesAndTypes(*contract)) return false; // Note that we now reference contracts by their fully qualified names, and // thus contracts can only conflict if declared in the same source file. This // already causes a double-declaration error elsewhere, so we do not report diff --git a/test/libsolidity/Assembly.cpp b/test/libsolidity/Assembly.cpp index f48f022ef..7281fd2eb 100644 --- a/test/libsolidity/Assembly.cpp +++ b/test/libsolidity/Assembly.cpp @@ -63,7 +63,8 @@ eth::AssemblyItems compileContract(std::shared_ptr _sourceCode) BOOST_CHECK(!!sourceUnit); map> scopes; - NameAndTypeResolver resolver({}, scopes, errorReporter); + GlobalContext globalContext; + NameAndTypeResolver resolver(globalContext, scopes, errorReporter); solAssert(Error::containsOnlyWarnings(errorReporter.errors()), ""); resolver.registerDeclarations(*sourceUnit); for (ASTPointer const& node: sourceUnit->nodes()) diff --git a/test/libsolidity/SolidityExpressionCompiler.cpp b/test/libsolidity/SolidityExpressionCompiler.cpp index 36131799e..3cdd70f67 100644 --- a/test/libsolidity/SolidityExpressionCompiler.cpp +++ b/test/libsolidity/SolidityExpressionCompiler.cpp @@ -94,8 +94,7 @@ Declaration const& resolveDeclaration( bytes compileFirstExpression( const string& _sourceCode, vector> _functions = {}, - vector> _localVariables = {}, - vector> _globalDeclarations = {} + vector> _localVariables = {} ) { ASTPointer sourceUnit; @@ -113,15 +112,11 @@ bytes compileFirstExpression( BOOST_FAIL(msg); } - vector declarations; - declarations.reserve(_globalDeclarations.size() + 1); - for (ASTPointer const& variable: _globalDeclarations) - declarations.push_back(variable.get()); - ErrorList errors; ErrorReporter errorReporter(errors); + GlobalContext globalContext; map> scopes; - NameAndTypeResolver resolver(declarations, scopes, errorReporter); + NameAndTypeResolver resolver(globalContext, scopes, errorReporter); resolver.registerDeclarations(*sourceUnit); vector inheritanceHierarchy; @@ -598,10 +593,7 @@ BOOST_AUTO_TEST_CASE(blockhash) } )"; - auto blockhashFun = TypeProvider::function(strings{"uint256"}, strings{"bytes32"}, - FunctionType::Kind::BlockHash, false, StateMutability::View); - - bytes code = compileFirstExpression(sourceCode, {}, {}, {make_shared("blockhash", blockhashFun)}); + bytes code = compileFirstExpression(sourceCode, {}, {}); bytes expectation({uint8_t(Instruction::PUSH1), 0x03, uint8_t(Instruction::BLOCKHASH)}); @@ -617,10 +609,7 @@ BOOST_AUTO_TEST_CASE(gas_left) } } )"; - bytes code = compileFirstExpression( - sourceCode, {}, {}, - {make_shared("gasleft", TypeProvider::function(strings(), strings{"uint256"}, FunctionType::Kind::GasLeft))} - ); + bytes code = compileFirstExpression(sourceCode, {}, {}); bytes expectation = bytes({uint8_t(Instruction::GAS)}); BOOST_CHECK_EQUAL_COLLECTIONS(code.begin(), code.end(), expectation.begin(), expectation.end()); diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/shadowsBuiltin/this_super.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/shadowsBuiltin/this_super.sol new file mode 100644 index 000000000..4fa8a1ff4 --- /dev/null +++ b/test/libsolidity/syntaxTests/nameAndTypeResolution/shadowsBuiltin/this_super.sol @@ -0,0 +1,11 @@ +contract C { + function f() pure public { + uint super = 3; + uint this = 4; + } +} +// ---- +// Warning: (52-62): This declaration shadows a builtin symbol. +// Warning: (76-85): This declaration shadows a builtin symbol. +// Warning: (52-62): Unused local variable. +// Warning: (76-85): Unused local variable.