Implement override checking

This commit is contained in:
Mathias Baumann
2019-10-30 17:31:33 +01:00
parent 5ff02c12e2
commit 6c6a9054b2
85 changed files with 965 additions and 204 deletions
+376 -33
View File
@@ -26,6 +26,7 @@
#include <libsolidity/analysis/TypeChecker.h>
#include <liblangutil/ErrorReporter.h>
#include <boost/range/adaptor/reversed.hpp>
#include <boost/algorithm/string/predicate.hpp>
using namespace std;
@@ -33,17 +34,95 @@ using namespace dev;
using namespace langutil;
using namespace dev::solidity;
namespace {
namespace
{
// Helper struct to do a search by name
struct MatchByName
{
string const& m_name;
bool operator()(CallableDeclaration const* _callable)
{
return _callable->name() == m_name;
}
};
vector<ASTPointer<UserDefinedTypeName>> sortByContract(vector<ASTPointer<UserDefinedTypeName>> const& _list)
{
auto sorted = _list;
sort(sorted.begin(), sorted.end(),
[] (ASTPointer<UserDefinedTypeName> _a, ASTPointer<UserDefinedTypeName> _b) {
if (!_a || !_b)
return _a < _b;
Declaration const* aDecl = _a->annotation().referencedDeclaration;
Declaration const* bDecl = _b->annotation().referencedDeclaration;
if (!aDecl || !bDecl)
return aDecl < bDecl;
return aDecl->id() < bDecl->id();
}
);
return sorted;
}
template <class T>
bool hasEqualNameAndParameters(T const& _a, T const& _b)
{
return _a.name() == _b.name() &&
return
_a.name() == _b.name() &&
FunctionType(_a).asCallableFunction(false)->hasEqualParameterTypes(
*FunctionType(_b).asCallableFunction(false)
);
}
vector<ContractDefinition const*> resolveDirectBaseContracts(ContractDefinition const& _contract)
{
vector<ContractDefinition const*> resolvedContracts;
for (ASTPointer<InheritanceSpecifier> const& specifier: _contract.baseContracts())
{
Declaration const* baseDecl =
specifier->name().annotation().referencedDeclaration;
auto contract = dynamic_cast<ContractDefinition const*>(baseDecl);
solAssert(contract, "contract is null");
resolvedContracts.emplace_back(contract);
}
return resolvedContracts;
}
}
bool ContractLevelChecker::LessFunction::operator()(ModifierDefinition const* _a, ModifierDefinition const* _b) const
{
return _a->name() < _b->name();
}
bool ContractLevelChecker::LessFunction::operator()(FunctionDefinition const* _a, FunctionDefinition const* _b) const
{
if (_a->name() != _b->name())
return _a->name() < _b->name();
return boost::lexicographical_compare(
FunctionType(*_a).asCallableFunction(false)->parameterTypes(),
FunctionType(*_b).asCallableFunction(false)->parameterTypes(),
[](auto const& _paramTypeA, auto const& _paramTypeB)
{
return _paramTypeA->richIdentifier() < _paramTypeB->richIdentifier();
}
);
}
bool ContractLevelChecker::LessFunction::operator()(ContractDefinition const* _a, ContractDefinition const* _b) const
{
if (!_a || !_b)
return _a < _b;
return _a->id() < _b->id();
}
bool ContractLevelChecker::check(ContractDefinition const& _contract)
@@ -51,6 +130,7 @@ bool ContractLevelChecker::check(ContractDefinition const& _contract)
checkDuplicateFunctions(_contract);
checkDuplicateEvents(_contract);
checkIllegalOverrides(_contract);
checkAmbiguousOverrides(_contract);
checkAbstractFunctions(_contract);
checkBaseConstructorArguments(_contract);
checkConstructor(_contract);
@@ -145,50 +225,52 @@ void ContractLevelChecker::findDuplicateDefinitions(map<string, vector<T>> const
void ContractLevelChecker::checkIllegalOverrides(ContractDefinition const& _contract)
{
// TODO unify this at a later point. for this we need to put the constness and the access specifier
// into the types
map<string, vector<FunctionDefinition const*>> functions;
map<string, ModifierDefinition const*> modifiers;
FunctionMultiSet const& funcSet = inheritedFunctions(&_contract);
ModifierMultiSet const& modSet = inheritedModifiers(&_contract);
// We search from derived to base, so the stored item causes the error.
for (ContractDefinition const* contract: _contract.annotation().linearizedBaseContracts)
checkModifierOverrides(funcSet, modSet, _contract.functionModifiers());
for (FunctionDefinition const* function: _contract.definedFunctions())
{
for (FunctionDefinition const* function: contract->definedFunctions())
{
if (function->isConstructor())
continue; // constructors can neither be overridden nor override anything
string const& name = function->name();
if (modifiers.count(name))
m_errorReporter.typeError(modifiers[name]->location(), "Override changes function to modifier.");
if (contains_if(modSet, MatchByName{function->name()}))
m_errorReporter.typeError(function->location(), "Override changes modifier to function.");
for (FunctionDefinition const* overriding: functions[name])
checkFunctionOverride(*overriding, *function);
// Skip if not overridable
if (!function->isOverridable())
continue;
functions[name].push_back(function);
}
for (ModifierDefinition const* modifier: contract->functionModifiers())
{
string const& name = modifier->name();
ModifierDefinition const*& override = modifiers[name];
if (!override)
override = modifier;
else if (ModifierType(*override) != ModifierType(*modifier))
m_errorReporter.typeError(override->location(), "Override changes modifier signature.");
if (!functions[name].empty())
m_errorReporter.typeError(override->location(), "Override changes modifier to function.");
}
// No inheriting functions found
if (funcSet.find(function) == funcSet.cend() && function->overrides())
m_errorReporter.typeError(
function->overrides()->location(),
"Function has override specified but does not override anything."
);
checkOverrideList(funcSet, *function);
}
}
void ContractLevelChecker::checkFunctionOverride(FunctionDefinition const& _function, FunctionDefinition const& _super)
bool ContractLevelChecker::checkFunctionOverride(FunctionDefinition const& _function, FunctionDefinition const& _super)
{
FunctionTypePointer functionType = FunctionType(_function).asCallableFunction(false);
FunctionTypePointer superType = FunctionType(_super).asCallableFunction(false);
bool success = true;
if (!functionType->hasEqualParameterTypes(*superType))
return;
return true;
if (!_function.overrides())
{
overrideError(_function, _super, "Overriding function is missing 'override' specifier.");
success = false;
}
if (!functionType->hasEqualReturnTypes(*superType))
{
overrideError(_function, _super, "Overriding function return types differ.");
success = false;
}
if (!_function.annotation().superFunction)
_function.annotation().superFunction = &_super;
@@ -201,9 +283,13 @@ void ContractLevelChecker::checkFunctionOverride(FunctionDefinition const& _func
_super.visibility() == FunctionDefinition::Visibility::External &&
_function.visibility() == FunctionDefinition::Visibility::Public
))
{
overrideError(_function, _super, "Overriding function visibility differs.");
success = false;
}
}
if (_function.stateMutability() != _super.stateMutability())
{
overrideError(
_function,
_super,
@@ -213,9 +299,38 @@ void ContractLevelChecker::checkFunctionOverride(FunctionDefinition const& _func
stateMutabilityToString(_function.stateMutability()) +
"\"."
);
success = false;
}
return success;
}
void ContractLevelChecker::overrideError(FunctionDefinition const& function, FunctionDefinition const& super, string message)
void ContractLevelChecker::overrideListError(FunctionDefinition const& function, set<ContractDefinition const*, LessFunction> _secondary, string const& _message1, string const& _message2)
{
// Using a set rather than a vector so the order is always the same
set<string> names;
SecondarySourceLocation ssl;
for (Declaration const* c: _secondary)
{
ssl.append("This contract: ", c->location());
names.insert(c->name());
}
string contractSingularPlural = "contract ";
if (_secondary.size() > 1)
contractSingularPlural = "contracts ";
m_errorReporter.typeError(
function.overrides() ? function.overrides()->location() : function.location(),
ssl,
_message1 +
contractSingularPlural +
_message2 +
joinHumanReadable(names, ", ", " and ") +
"."
);
}
void ContractLevelChecker::overrideError(CallableDeclaration const& function, CallableDeclaration const& super, string message)
{
m_errorReporter.typeError(
function.location(),
@@ -520,3 +635,231 @@ void ContractLevelChecker::checkBaseABICompatibility(ContractDefinition const& _
);
}
void ContractLevelChecker::checkAmbiguousOverrides(ContractDefinition const& _contract) const
{
vector<FunctionDefinition const*> contractFuncs = _contract.definedFunctions();
auto const resolvedBases = resolveDirectBaseContracts(_contract);
FunctionMultiSet inheritedFuncs = inheritedFunctions(&_contract);;
// Check the sets of the most-inherited functions
for (auto it = inheritedFuncs.cbegin(); it != inheritedFuncs.cend(); it = inheritedFuncs.upper_bound(*it))
{
auto [begin,end] = inheritedFuncs.equal_range(*it);
// Only one function
if (next(begin) == end)
continue;
// Not an overridable function
if (!(*it)->isOverridable())
{
for (begin++; begin != end; begin++)
solAssert(!(*begin)->isOverridable(), "All functions in range expected to be non-overridable!");
continue;
}
// Function has been explicitly overridden
if (contains_if(
contractFuncs,
[&] (FunctionDefinition const* _f) {
return hasEqualNameAndParameters(*_f, **it);
}
))
continue;
set<FunctionDefinition const*> ambiguousFunctions;
SecondarySourceLocation ssl;
for (;begin != end; begin++)
{
ambiguousFunctions.insert(*begin);
ssl.append("Definition here: ", (*begin)->location());
}
// Make sure the functions are not from the same base contract
if (ambiguousFunctions.size() == 1)
continue;
m_errorReporter.typeError(
_contract.location(),
ssl,
"Derived contract must override function \"" +
(*it)->name() +
"\". Function with the same name and parameter types defined in two or more base classes."
);
}
}
set<ContractDefinition const*, ContractLevelChecker::LessFunction> ContractLevelChecker::resolveOverrideList(OverrideSpecifier const& _overrides) const
{
set<ContractDefinition const*, LessFunction> resolved;
for (ASTPointer<UserDefinedTypeName> const& override: _overrides.overrides())
{
Declaration const* decl = override->annotation().referencedDeclaration;
solAssert(decl, "Expected declaration to be resolved.");
// If it's not a contract it will be caught
// in the reference resolver
if (ContractDefinition const* contract = dynamic_cast<decltype(contract)>(decl))
resolved.insert(contract);
}
return resolved;
}
void ContractLevelChecker::checkModifierOverrides(FunctionMultiSet const& _funcSet, ModifierMultiSet const& _modSet, std::vector<ModifierDefinition const*> _modifiers)
{
for (ModifierDefinition const* modifier: _modifiers)
{
if (contains_if(_funcSet, MatchByName{modifier->name()}))
m_errorReporter.typeError(
modifier->location(),
"Override changes function to modifier."
);
auto [begin,end] = _modSet.equal_range(modifier);
// Skip if no modifiers found in bases
if (begin == end)
continue;
if (!modifier->overrides())
overrideError(*modifier, **begin, "Overriding modifier is missing 'override' specifier.");
for (; begin != end; begin++)
if (ModifierType(**begin) != ModifierType(*modifier))
m_errorReporter.typeError(
modifier->location(),
"Override changes modifier signature."
);
}
}
void ContractLevelChecker::checkOverrideList(FunctionMultiSet const& _funcSet, FunctionDefinition const& _function)
{
set<ContractDefinition const*, LessFunction> specifiedContracts =
_function.overrides() ?
resolveOverrideList(*_function.overrides()) :
decltype(specifiedContracts){};
// Check for duplicates in override list
if (_function.overrides() && specifiedContracts.size() != _function.overrides()->overrides().size())
{
// Sort by contract id to find duplicate for error reporting
vector<ASTPointer<UserDefinedTypeName>> list =
sortByContract(_function.overrides()->overrides());
// Find duplicates and output error
for (size_t i = 1; i < list.size(); i++)
{
Declaration const* aDecl = list[i]->annotation().referencedDeclaration;
Declaration const* bDecl = list[i-1]->annotation().referencedDeclaration;
if (!aDecl || !bDecl)
continue;
if (aDecl->id() == bDecl->id())
{
SecondarySourceLocation ssl;
ssl.append("First occurrence here: ", list[i-1]->location());
m_errorReporter.typeError(
list[i]->location(),
ssl,
"Duplicate contract \"" +
joinHumanReadable(list[i]->namePath(), ".") +
"\" found in override list of \"" +
_function.name() +
"\"."
);
}
}
}
decltype(specifiedContracts) expectedContracts;
// Build list of expected contracts
for (auto [begin, end] = _funcSet.equal_range(&_function); begin != end; begin++)
{
// Validate the override
if (!checkFunctionOverride(_function, **begin))
break;
expectedContracts.insert((*begin)->annotation().contract);
}
decltype(specifiedContracts) missingContracts;
decltype(specifiedContracts) surplusContracts;
// If we expect only one contract, no contract needs to be specified
if (expectedContracts.size() > 1)
missingContracts = expectedContracts - specifiedContracts;
surplusContracts = specifiedContracts - expectedContracts;
if (!missingContracts.empty())
overrideListError(
_function,
missingContracts,
"Function needs to specify overridden ",
""
);
if (!surplusContracts.empty())
overrideListError(
_function,
surplusContracts,
"Invalid ",
"specified in override list: "
);
}
ContractLevelChecker::FunctionMultiSet const& ContractLevelChecker::inheritedFunctions(ContractDefinition const* _contract) const
{
if (!m_inheritedFunctions.count(_contract))
{
FunctionMultiSet set;
for (auto const* base: resolveDirectBaseContracts(*_contract))
{
std::set<FunctionDefinition const*, LessFunction> tmpSet =
convertContainer<decltype(tmpSet)>(base->definedFunctions());
for (auto const& func: inheritedFunctions(base))
tmpSet.insert(func);
set += tmpSet;
}
m_inheritedFunctions[_contract] = set;
}
return m_inheritedFunctions[_contract];
}
ContractLevelChecker::ModifierMultiSet const& ContractLevelChecker::inheritedModifiers(ContractDefinition const* _contract) const
{
auto const& result = m_contractBaseModifiers.find(_contract);
if (result != m_contractBaseModifiers.cend())
return result->second;
ModifierMultiSet set;
for (auto const* base: resolveDirectBaseContracts(*_contract))
{
std::set<ModifierDefinition const*, LessFunction> tmpSet =
convertContainer<decltype(tmpSet)>(base->functionModifiers());
for (auto const& mod: inheritedModifiers(base))
tmpSet.insert(mod);
set += tmpSet;
}
return m_contractBaseModifiers[_contract] = set;
}
+34 -3
View File
@@ -23,6 +23,7 @@
#include <libsolidity/ast/ASTForward.h>
#include <map>
#include <set>
namespace langutil
{
@@ -41,6 +42,7 @@ namespace solidity
class ContractLevelChecker
{
public:
/// @param _errorReporter provides the error logging functionality.
explicit ContractLevelChecker(langutil::ErrorReporter& _errorReporter):
m_errorReporter(_errorReporter)
@@ -51,6 +53,16 @@ public:
bool check(ContractDefinition const& _contract);
private:
struct LessFunction
{
bool operator()(ModifierDefinition const* _a, ModifierDefinition const* _b) const;
bool operator()(FunctionDefinition const* _a, FunctionDefinition const* _b) const;
bool operator()(ContractDefinition const* _a, ContractDefinition const* _b) const;
};
using FunctionMultiSet = std::multiset<FunctionDefinition const*, LessFunction>;
using ModifierMultiSet = std::multiset<ModifierDefinition const*, LessFunction>;
/// Checks that two functions defined in this contract with the same name have different
/// arguments and that there is at most one constructor.
void checkDuplicateFunctions(ContractDefinition const& _contract);
@@ -58,10 +70,12 @@ private:
template <class T>
void findDuplicateDefinitions(std::map<std::string, std::vector<T>> const& _definitions, std::string _message);
void checkIllegalOverrides(ContractDefinition const& _contract);
/// Reports a type error with an appropriate message if overridden function signature differs.
/// Returns false and reports a type error with an appropriate
/// message if overridden function signature differs.
/// Also stores the direct super function in the AST annotations.
void checkFunctionOverride(FunctionDefinition const& function, FunctionDefinition const& super);
void overrideError(FunctionDefinition const& function, FunctionDefinition const& super, std::string message);
bool checkFunctionOverride(FunctionDefinition const& _function, FunctionDefinition const& _super);
void overrideListError(FunctionDefinition const& function, std::set<ContractDefinition const*, LessFunction> _secondary, std::string const& _message1, std::string const& _message2);
void overrideError(CallableDeclaration const& function, CallableDeclaration const& super, std::string message);
void checkAbstractFunctions(ContractDefinition const& _contract);
void checkBaseConstructorArguments(ContractDefinition const& _contract);
void annotateBaseConstructorArguments(
@@ -80,8 +94,25 @@ private:
void checkLibraryRequirements(ContractDefinition const& _contract);
/// Checks base contracts for ABI compatibility
void checkBaseABICompatibility(ContractDefinition const& _contract);
/// Checks for functions in different base contracts which conflict with each
/// other and thus need to be overridden explicitly.
void checkAmbiguousOverrides(ContractDefinition const& _contract) const;
/// Resolves an override list of UserDefinedTypeNames to a list of contracts.
std::set<ContractDefinition const*, LessFunction> resolveOverrideList(OverrideSpecifier const& _overrides) const;
void checkModifierOverrides(FunctionMultiSet const& _funcSet, ModifierMultiSet const& _modSet, std::vector<ModifierDefinition const*> _modifiers);
void checkOverrideList(FunctionMultiSet const& _funcSet, FunctionDefinition const& _function);
/// Returns all functions of bases that have not yet been overwritten.
/// May contain the same function multiple times when used with shared bases.
FunctionMultiSet const& inheritedFunctions(ContractDefinition const* _contract) const;
ModifierMultiSet const& inheritedModifiers(ContractDefinition const* _contract) const;
langutil::ErrorReporter& m_errorReporter;
/// Cache for inheritedFunctions().
std::map<ContractDefinition const*, FunctionMultiSet> mutable m_inheritedFunctions;
std::map<ContractDefinition const*, ModifierMultiSet> mutable m_contractBaseModifiers;
};
}