virtual modifiers (in Abstract contracts) allow empty bodies

This commit is contained in:
hrkrshnn 2020-04-15 16:12:15 +05:30
parent 0c5aa36e46
commit e2e32d372f
18 changed files with 87 additions and 67 deletions

View File

@ -55,7 +55,7 @@ bool ContractLevelChecker::check(ContractDefinition const& _contract)
checkDuplicateEvents(_contract);
m_overrideChecker.check(_contract);
checkBaseConstructorArguments(_contract);
checkAbstractFunctions(_contract);
checkAbstractDefinitions(_contract);
checkExternalTypeClashes(_contract);
checkHashCollisions(_contract);
checkLibraryRequirements(_contract);
@ -156,55 +156,43 @@ void ContractLevelChecker::findDuplicateDefinitions(map<string, vector<T>> const
}
}
void ContractLevelChecker::checkAbstractFunctions(ContractDefinition const& _contract)
void ContractLevelChecker::checkAbstractDefinitions(ContractDefinition const& _contract)
{
// Mapping from name to function definition (exactly one per argument type equality class) and
// flag to indicate whether it is fully implemented.
using FunTypeAndFlag = std::pair<FunctionTypePointer, bool>;
map<string, vector<FunTypeAndFlag>> functions;
// Collects functions, static variable getters and modifiers. If they
// override (unimplemented) base class ones, they are replaced.
set<OverrideProxy, OverrideProxy::CompareBySignature> proxies;
auto registerFunction = [&](Declaration const& _declaration, FunctionTypePointer const& _type, bool _implemented)
auto registerProxy = [&proxies](OverrideProxy const& _overrideProxy)
{
auto& overloads = functions[_declaration.name()];
auto it = find_if(overloads.begin(), overloads.end(), [&](FunTypeAndFlag const& _funAndFlag)
{
return _type->hasEqualParameterTypes(*_funAndFlag.first);
});
if (it == overloads.end())
overloads.emplace_back(_type, _implemented);
else if (_implemented)
it->second = true;
// Overwrite an existing proxy, if it exists.
if (!_overrideProxy.unimplemented())
proxies.erase(_overrideProxy);
proxies.insert(_overrideProxy);
};
// Search from base to derived, collect all functions and update
// the 'implemented' flag.
// Search from base to derived, collect all functions and modifiers and
// update proxies.
for (ContractDefinition const* contract: boost::adaptors::reverse(_contract.annotation().linearizedBaseContracts))
{
for (VariableDeclaration const* v: contract->stateVariables())
if (v->isPartOfExternalInterface())
registerFunction(*v, TypeProvider::function(*v), true);
registerProxy(OverrideProxy(v));
for (FunctionDefinition const* function: contract->definedFunctions())
if (!function->isConstructor())
registerFunction(
*function,
TypeProvider::function(*function)->asCallableFunction(false),
function->isImplemented()
);
registerProxy(OverrideProxy(function));
for (ModifierDefinition const* modifier: contract->functionModifiers())
registerProxy(OverrideProxy(modifier));
}
// Set to not fully implemented if at least one flag is false.
// Note that `_contract.annotation().unimplementedFunctions` has already been
// Note that `_contract.annotation().unimplementedDeclarations` has already been
// pre-filled by `checkBaseConstructorArguments`.
for (auto const& it: functions)
for (auto const& funAndFlag: it.second)
if (!funAndFlag.second)
{
FunctionDefinition const* function = dynamic_cast<FunctionDefinition const*>(&funAndFlag.first->declaration());
solAssert(function, "");
_contract.annotation().unimplementedFunctions.push_back(function);
break;
}
for (auto const& proxy: proxies)
if (proxy.unimplemented())
_contract.annotation().unimplementedDeclarations.push_back(proxy.declaration());
if (_contract.abstract())
{
@ -221,15 +209,16 @@ void ContractLevelChecker::checkAbstractFunctions(ContractDefinition const& _con
if (
_contract.contractKind() == ContractKind::Contract &&
!_contract.abstract() &&
!_contract.annotation().unimplementedFunctions.empty()
!_contract.annotation().unimplementedDeclarations.empty()
)
{
SecondarySourceLocation ssl;
for (auto function: _contract.annotation().unimplementedFunctions)
ssl.append("Missing implementation:", function->location());
for (auto declaration: _contract.annotation().unimplementedDeclarations)
ssl.append("Missing implementation: ", declaration->location());
m_errorReporter.typeError(_contract.location(), ssl,
"Contract \"" + _contract.annotation().canonicalName
+ "\" should be marked as abstract.");
}
}
@ -277,7 +266,7 @@ void ContractLevelChecker::checkBaseConstructorArguments(ContractDefinition cons
if (FunctionDefinition const* constructor = contract->constructor())
if (contract != &_contract && !constructor->parameters().empty())
if (!_contract.annotation().baseConstructorArguments.count(constructor))
_contract.annotation().unimplementedFunctions.push_back(constructor);
_contract.annotation().unimplementedDeclarations.push_back(constructor);
}
void ContractLevelChecker::annotateBaseConstructorArguments(

View File

@ -61,7 +61,8 @@ private:
void checkDuplicateEvents(ContractDefinition const& _contract);
template <class T>
void findDuplicateDefinitions(std::map<std::string, std::vector<T>> const& _definitions, std::string _message);
void checkAbstractFunctions(ContractDefinition const& _contract);
/// Checks for unimplemented functions and modifiers.
void checkAbstractDefinitions(ContractDefinition const& _contract);
/// Checks that the base constructor arguments are properly provided.
/// Fills the list of unimplemented functions in _contract's annotations.
void checkBaseConstructorArguments(ContractDefinition const& _contract);

View File

@ -148,7 +148,8 @@ bool ImmutableValidator::analyseCallable(CallableDeclaration const& _callableDec
funcDef->body().accept(*this);
}
else if (ModifierDefinition const* modDef = dynamic_cast<decltype(modDef)>(&_callableDeclaration))
modDef->body().accept(*this);
if (modDef->isImplemented())
modDef->body().accept(*this);
m_currentConstructor = prevConstructor;

View File

@ -327,6 +327,16 @@ ModifierType const* OverrideProxy::modifierType() const
}, m_item);
}
Declaration const* OverrideProxy::declaration() const
{
return std::visit(GenericVisitor{
[&](FunctionDefinition const* _function) -> Declaration const* { return _function; },
[&](VariableDeclaration const* _variable) -> Declaration const* { return _variable; },
[&](ModifierDefinition const* _modifier) -> Declaration const* { return _modifier; }
}, m_item);
}
SourceLocation const& OverrideProxy::location() const
{
return std::visit(GenericVisitor{
@ -365,7 +375,7 @@ bool OverrideProxy::unimplemented() const
{
return std::visit(GenericVisitor{
[&](FunctionDefinition const* _item) { return !_item->isImplemented(); },
[&](ModifierDefinition const*) { return false; },
[&](ModifierDefinition const* _item) { return !_item->isImplemented(); },
[&](VariableDeclaration const*) { return false; }
}, m_item);
}

View File

@ -85,6 +85,8 @@ public:
FunctionType const* functionType() const;
ModifierType const* modifierType() const;
Declaration const* declaration() const;
langutil::SourceLocation const& location() const;
std::string astNodeName() const;

View File

@ -141,7 +141,7 @@ bool SyntaxChecker::visit(ModifierDefinition const&)
void SyntaxChecker::endVisit(ModifierDefinition const& _modifier)
{
if (!m_placeholderFound)
if (_modifier.isImplemented() && !m_placeholderFound)
m_errorReporter.syntaxError(_modifier.body().location(), "Modifier body does not contain '_'.");
m_placeholderFound = false;
}

View File

@ -289,6 +289,12 @@ void TypeChecker::endVisit(UsingForDirective const& _usingFor)
m_errorReporter.fatalTypeError(_usingFor.libraryName().location(), "Library name expected.");
}
void TypeChecker::endVisit(ModifierDefinition const& _modifier)
{
if (!_modifier.isImplemented() && !_modifier.virtualSemantics())
m_errorReporter.typeError(_modifier.location(), "Modifiers without implementation must be marked virtual.");
}
bool TypeChecker::visit(FunctionDefinition const& _function)
{
bool isLibraryFunction = _function.inContractKind() == ContractKind::Library;

View File

@ -112,6 +112,7 @@ private:
void endVisit(InheritanceSpecifier const& _inheritance) override;
void endVisit(UsingForDirective const& _usingFor) override;
void endVisit(ModifierDefinition const& _modifier) override;
bool visit(FunctionDefinition const& _function) override;
bool visit(VariableDeclaration const& _variable) override;
/// We need to do this manually because we want to pass the bases of the current contract in

View File

@ -969,7 +969,7 @@ private:
/**
* Definition of a function modifier.
*/
class ModifierDefinition: public CallableDeclaration, public StructurallyDocumented
class ModifierDefinition: public CallableDeclaration, public StructurallyDocumented, public ImplementationOptional
{
public:
ModifierDefinition(
@ -980,18 +980,19 @@ public:
ASTPointer<ParameterList> const& _parameters,
bool _isVirtual,
ASTPointer<OverrideSpecifier> const& _overrides,
ASTPointer<Block> _body
ASTPointer<Block> const& _body
):
CallableDeclaration(_id, _location, _name, Visibility::Internal, _parameters, _isVirtual, _overrides),
StructurallyDocumented(_documentation),
m_body(std::move(_body))
ImplementationOptional(_body != nullptr),
m_body(_body)
{
}
void accept(ASTVisitor& _visitor) override;
void accept(ASTConstVisitor& _visitor) const override;
Block const& body() const { return *m_body; }
Block const& body() const { solAssert(m_body, ""); return *m_body; }
TypePointer type() const override;

View File

@ -140,8 +140,8 @@ struct StructDeclarationAnnotation: TypeDeclarationAnnotation
struct ContractDefinitionAnnotation: TypeDeclarationAnnotation, StructurallyDocumentedAnnotation
{
/// List of functions without a body. Can also contain functions from base classes.
std::vector<FunctionDefinition const*> unimplementedFunctions;
/// List of functions and modifiers without a body. Can also contain functions from base classes.
std::vector<Declaration const*> unimplementedDeclarations;
/// List of all (direct and indirect) base contracts in order from derived to
/// base, including the contract itself.
std::vector<ContractDefinition const*> linearizedBaseContracts;

View File

@ -272,7 +272,7 @@ bool ASTJsonConverter::visit(ContractDefinition const& _node)
make_pair("documentation", _node.documentation() ? toJson(*_node.documentation()) : Json::nullValue),
make_pair("contractKind", contractKind(_node.contractKind())),
make_pair("abstract", _node.abstract()),
make_pair("fullyImplemented", _node.annotation().unimplementedFunctions.empty()),
make_pair("fullyImplemented", _node.annotation().unimplementedDeclarations.empty()),
make_pair("linearizedBaseContracts", getContainerIds(_node.annotation().linearizedBaseContracts)),
make_pair("baseContracts", toJson(_node.baseContracts())),
make_pair("contractDependencies", getContainerIds(_node.annotation().contractDependencies, true)),
@ -407,7 +407,7 @@ bool ASTJsonConverter::visit(ModifierDefinition const& _node)
make_pair("parameters", toJson(_node.parameterList())),
make_pair("virtual", _node.markedVirtual()),
make_pair("overrides", _node.overrides() ? toJson(*_node.overrides()) : Json::nullValue),
make_pair("body", toJson(_node.body()))
make_pair("body", _node.isImplemented() ? toJson(_node.body()) : Json::nullValue)
};
if (!_node.annotation().baseFunctions.empty())
attributes.emplace_back(make_pair("baseModifiers", getContainerIds(_node.annotation().baseFunctions, true)));

View File

@ -453,7 +453,7 @@ ASTPointer<ModifierDefinition> ASTJsonImporter::createModifierDefinition(Json::V
createParameterList(member(_node, "parameters")),
memberAsBool(_node, "virtual"),
_node["overrides"].isNull() ? nullptr : createOverrideSpecifier(member(_node, "overrides")),
createBlock(member(_node, "body"))
_node["body"].isNull() ? nullptr: createBlock(member(_node, "body"))
);
}

View File

@ -288,7 +288,8 @@ void ModifierDefinition::accept(ASTVisitor& _visitor)
m_parameters->accept(_visitor);
if (m_overrides)
m_overrides->accept(_visitor);
m_body->accept(_visitor);
if (m_body)
m_body->accept(_visitor);
}
_visitor.endVisit(*this);
}
@ -302,7 +303,8 @@ void ModifierDefinition::accept(ASTConstVisitor& _visitor) const
m_parameters->accept(_visitor);
if (m_overrides)
m_overrides->accept(_visitor);
m_body->accept(_visitor);
if (m_body)
m_body->accept(_visitor);
}
_visitor.endVisit(*this);
}

View File

@ -1390,7 +1390,6 @@ public:
std::string richIdentifier() const override;
bool operator==(Type const& _other) const override;
std::string toString(bool _short) const override;
protected:
std::vector<std::tuple<std::string, TypePointer>> makeStackItems() const override { return {}; }
private:

View File

@ -194,7 +194,8 @@ void SMTEncoder::inlineModifierInvocation(ModifierInvocation const* _invocation,
pushCallStack({_definition, _invocation});
if (auto modifier = dynamic_cast<ModifierDefinition const*>(_definition))
{
modifier->body().accept(*this);
if (modifier->isImplemented())
modifier->body().accept(*this);
popCallStack();
}
else if (auto function = dynamic_cast<FunctionDefinition const*>(_definition))

View File

@ -595,13 +595,13 @@ ASTPointer<ASTNode> Parser::parseFunctionDefinition()
ASTPointer<Block> block;
nodeFactory.markEndPosition();
if (m_scanner->currentToken() != Token::Semicolon)
if (m_scanner->currentToken() == Token::Semicolon)
m_scanner->next();
else
{
block = parseBlock();
nodeFactory.setEndPositionFromNode(block);
}
else
m_scanner->next(); // just consume the ';'
return nodeFactory.createNode<FunctionDefinition>(
name,
header.visibility,
@ -851,9 +851,16 @@ ASTPointer<ModifierDefinition> Parser::parseModifierDefinition()
break;
}
ASTPointer<Block> block;
nodeFactory.markEndPosition();
if (m_scanner->currentToken() != Token::Semicolon)
{
block = parseBlock();
nodeFactory.setEndPositionFromNode(block);
}
else
m_scanner->next(); // just consume the ';'
ASTPointer<Block> block = parseBlock();
nodeFactory.setEndPositionFromNode(block);
return nodeFactory.createNode<ModifierDefinition>(name, documentation, parameters, isVirtual, overrides, block);
}

View File

@ -52,7 +52,7 @@ BOOST_AUTO_TEST_CASE(function_no_implementation)
std::vector<ASTPointer<ASTNode>> nodes = sourceUnit->nodes();
ContractDefinition* contract = dynamic_cast<ContractDefinition*>(nodes[1].get());
BOOST_REQUIRE(contract);
BOOST_CHECK(!contract->annotation().unimplementedFunctions.empty());
BOOST_CHECK(!contract->annotation().unimplementedDeclarations.empty());
BOOST_CHECK(!contract->definedFunctions()[0]->isImplemented());
}
@ -68,10 +68,10 @@ BOOST_AUTO_TEST_CASE(abstract_contract)
ContractDefinition* base = dynamic_cast<ContractDefinition*>(nodes[1].get());
ContractDefinition* derived = dynamic_cast<ContractDefinition*>(nodes[2].get());
BOOST_REQUIRE(base);
BOOST_CHECK(!base->annotation().unimplementedFunctions.empty());
BOOST_CHECK(!base->annotation().unimplementedDeclarations.empty());
BOOST_CHECK(!base->definedFunctions()[0]->isImplemented());
BOOST_REQUIRE(derived);
BOOST_CHECK(derived->annotation().unimplementedFunctions.empty());
BOOST_CHECK(derived->annotation().unimplementedDeclarations.empty());
BOOST_CHECK(derived->definedFunctions()[0]->isImplemented());
}
@ -87,9 +87,9 @@ BOOST_AUTO_TEST_CASE(abstract_contract_with_overload)
ContractDefinition* base = dynamic_cast<ContractDefinition*>(nodes[1].get());
ContractDefinition* derived = dynamic_cast<ContractDefinition*>(nodes[2].get());
BOOST_REQUIRE(base);
BOOST_CHECK(!base->annotation().unimplementedFunctions.empty());
BOOST_CHECK(!base->annotation().unimplementedDeclarations.empty());
BOOST_REQUIRE(derived);
BOOST_CHECK(!derived->annotation().unimplementedFunctions.empty());
BOOST_CHECK(!derived->annotation().unimplementedDeclarations.empty());
}
BOOST_AUTO_TEST_CASE(implement_abstract_via_constructor)
@ -104,7 +104,7 @@ BOOST_AUTO_TEST_CASE(implement_abstract_via_constructor)
BOOST_CHECK_EQUAL(nodes.size(), 3);
ContractDefinition* derived = dynamic_cast<ContractDefinition*>(nodes[2].get());
BOOST_REQUIRE(derived);
BOOST_CHECK(!derived->annotation().unimplementedFunctions.empty());
BOOST_CHECK(!derived->annotation().unimplementedDeclarations.empty());
}
BOOST_AUTO_TEST_CASE(function_canonical_signature)

View File

@ -99,7 +99,7 @@ inline string appendVirtual(FunctionDefinition const& _function)
void AbstractContract::endVisit(ContractDefinition const& _contract)
{
bool isFullyImplemented = _contract.annotation().unimplementedFunctions.empty();
bool isFullyImplemented = _contract.annotation().unimplementedDeclarations.empty();
if (
!isFullyImplemented &&