Issue warning for variables called super or this

This commit is contained in:
Mathias Baumann 2019-04-29 18:29:40 +02:00
parent 00172192bf
commit cf35e5ba02
7 changed files with 42 additions and 28 deletions

View File

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

View File

@ -37,16 +37,17 @@ namespace solidity
{
NameAndTypeResolver::NameAndTypeResolver(
vector<Declaration const*> const& _globals,
GlobalContext& _globalContext,
map<ASTNode const*, shared_ptr<DeclarationContainer>>& _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<InheritanceSpecifier> const& baseContract: contract->baseContracts())
if (!resolveNamesAndTypes(*baseContract, true))
success = false;
@ -452,11 +457,13 @@ DeclarationRegistrationHelper::DeclarationRegistrationHelper(
map<ASTNode const*, shared_ptr<DeclarationContainer>>& _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;

View File

@ -23,6 +23,7 @@
#pragma once
#include <libsolidity/analysis/DeclarationContainer.h>
#include <libsolidity/analysis/GlobalContext.h>
#include <libsolidity/analysis/ReferencesResolver.h>
#include <libsolidity/ast/ASTAnnotations.h>
#include <libsolidity/ast/ASTVisitor.h>
@ -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<Declaration const*> const& _globals,
GlobalContext& _globalContext,
std::map<ASTNode const*, std::shared_ptr<DeclarationContainer>>& _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<ASTNode const*, std::shared_ptr<DeclarationContainer>>& _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;
};
}

View File

@ -260,7 +260,7 @@ bool CompilerStack::analyze()
noErrors = false;
m_globalContext = make_shared<GlobalContext>();
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<ASTNode> const& node: source->ast->nodes())
if (ContractDefinition* contract = dynamic_cast<ContractDefinition*>(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

View File

@ -63,7 +63,8 @@ eth::AssemblyItems compileContract(std::shared_ptr<CharStream> _sourceCode)
BOOST_CHECK(!!sourceUnit);
map<ASTNode const*, shared_ptr<DeclarationContainer>> scopes;
NameAndTypeResolver resolver({}, scopes, errorReporter);
GlobalContext globalContext;
NameAndTypeResolver resolver(globalContext, scopes, errorReporter);
solAssert(Error::containsOnlyWarnings(errorReporter.errors()), "");
resolver.registerDeclarations(*sourceUnit);
for (ASTPointer<ASTNode> const& node: sourceUnit->nodes())

View File

@ -94,8 +94,7 @@ Declaration const& resolveDeclaration(
bytes compileFirstExpression(
const string& _sourceCode,
vector<vector<string>> _functions = {},
vector<vector<string>> _localVariables = {},
vector<shared_ptr<MagicVariableDeclaration const>> _globalDeclarations = {}
vector<vector<string>> _localVariables = {}
)
{
ASTPointer<SourceUnit> sourceUnit;
@ -113,15 +112,11 @@ bytes compileFirstExpression(
BOOST_FAIL(msg);
}
vector<Declaration const*> declarations;
declarations.reserve(_globalDeclarations.size() + 1);
for (ASTPointer<Declaration const> const& variable: _globalDeclarations)
declarations.push_back(variable.get());
ErrorList errors;
ErrorReporter errorReporter(errors);
GlobalContext globalContext;
map<ASTNode const*, shared_ptr<DeclarationContainer>> scopes;
NameAndTypeResolver resolver(declarations, scopes, errorReporter);
NameAndTypeResolver resolver(globalContext, scopes, errorReporter);
resolver.registerDeclarations(*sourceUnit);
vector<ContractDefinition const*> 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<MagicVariableDeclaration>("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<MagicVariableDeclaration>("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());

View File

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