Error when using no parentheses in modifier-style constructor calls.

This commit is contained in:
Daniel Kirchner 2018-04-10 11:22:26 +02:00
parent b52614116e
commit 3eedbc6a9c
9 changed files with 52 additions and 21 deletions

View File

@ -12,7 +12,8 @@ Features:
* Static Analyzer: Error on duplicated super constructor calls as experimental 0.5.0 feature. * Static Analyzer: Error on duplicated super constructor calls as experimental 0.5.0 feature.
* Syntax Checker: Issue warning for empty structs (or error as experimental 0.5.0 feature). * Syntax Checker: Issue warning for empty structs (or error as experimental 0.5.0 feature).
* General: Introduce new constructor syntax using the ``constructor`` keyword as experimental 0.5.0 feature. * General: Introduce new constructor syntax using the ``constructor`` keyword as experimental 0.5.0 feature.
* Inheritance: Error when using empty parenthesis for base class constructors that require arguments as experimental 0.5.0 feature. * Inheritance: Error when using empty parentheses for base class constructors that require arguments as experimental 0.5.0 feature.
* Inheritance: Error when using no parentheses in modifier-style constructor calls as experimental 0.5.0 feature.
Bugfixes: Bugfixes:
* Code Generator: Allow ``block.blockhash`` without being called. * Code Generator: Allow ``block.blockhash`` without being called.

View File

@ -293,6 +293,8 @@ void TypeChecker::checkContractAbstractFunctions(ContractDefinition const& _cont
void TypeChecker::checkContractBaseConstructorArguments(ContractDefinition const& _contract) void TypeChecker::checkContractBaseConstructorArguments(ContractDefinition const& _contract)
{ {
bool const v050 = _contract.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050);
vector<ContractDefinition const*> const& bases = _contract.annotation().linearizedBaseContracts; vector<ContractDefinition const*> const& bases = _contract.annotation().linearizedBaseContracts;
// Determine the arguments that are used for the base constructors. // Determine the arguments that are used for the base constructors.
@ -302,8 +304,24 @@ void TypeChecker::checkContractBaseConstructorArguments(ContractDefinition const
for (auto const& modifier: constructor->modifiers()) for (auto const& modifier: constructor->modifiers())
{ {
auto baseContract = dynamic_cast<ContractDefinition const*>(&dereference(*modifier->name())); auto baseContract = dynamic_cast<ContractDefinition const*>(&dereference(*modifier->name()));
if (baseContract && baseContract->constructor() && !modifier->arguments().empty()) if (modifier->arguments())
annotateBaseConstructorArguments(_contract, baseContract->constructor(), modifier.get()); {
if (baseContract && baseContract->constructor())
annotateBaseConstructorArguments(_contract, baseContract->constructor(), modifier.get());
}
else
{
if (v050)
m_errorReporter.declarationError(
modifier->location(),
"Modifier-style base constructor call without arguments."
);
else
m_errorReporter.warning(
modifier->location(),
"Modifier-style base constructor call without arguments."
);
}
} }
for (ASTPointer<InheritanceSpecifier> const& base: contract->baseContracts()) for (ASTPointer<InheritanceSpecifier> const& base: contract->baseContracts())
@ -804,7 +822,8 @@ void TypeChecker::visitManually(
vector<ContractDefinition const*> const& _bases vector<ContractDefinition const*> const& _bases
) )
{ {
std::vector<ASTPointer<Expression>> const& arguments = _modifier.arguments(); std::vector<ASTPointer<Expression>> const& arguments =
_modifier.arguments() ? *_modifier.arguments() : std::vector<ASTPointer<Expression>>();
for (ASTPointer<Expression> const& argument: arguments) for (ASTPointer<Expression> const& argument: arguments)
argument->accept(*this); argument->accept(*this);
_modifier.name()->accept(*this); _modifier.name()->accept(*this);
@ -842,7 +861,7 @@ void TypeChecker::visitManually(
); );
return; return;
} }
for (size_t i = 0; i < _modifier.arguments().size(); ++i) for (size_t i = 0; i < arguments.size(); ++i)
if (!type(*arguments[i])->isImplicitlyConvertibleTo(*type(*(*parameters)[i]))) if (!type(*arguments[i])->isImplicitlyConvertibleTo(*type(*(*parameters)[i])))
m_errorReporter.typeError( m_errorReporter.typeError(
arguments[i]->location(), arguments[i]->location(),

View File

@ -762,19 +762,22 @@ public:
ModifierInvocation( ModifierInvocation(
SourceLocation const& _location, SourceLocation const& _location,
ASTPointer<Identifier> const& _name, ASTPointer<Identifier> const& _name,
std::vector<ASTPointer<Expression>> _arguments std::unique_ptr<std::vector<ASTPointer<Expression>>> _arguments
): ):
ASTNode(_location), m_modifierName(_name), m_arguments(_arguments) {} ASTNode(_location), m_modifierName(_name), m_arguments(std::move(_arguments)) {}
virtual void accept(ASTVisitor& _visitor) override; virtual void accept(ASTVisitor& _visitor) override;
virtual void accept(ASTConstVisitor& _visitor) const override; virtual void accept(ASTConstVisitor& _visitor) const override;
ASTPointer<Identifier> const& name() const { return m_modifierName; } ASTPointer<Identifier> const& name() const { return m_modifierName; }
std::vector<ASTPointer<Expression>> const& arguments() const { return m_arguments; } // Returns nullptr if no argument list was given (``mod``).
// If an argument list is given (``mod(...)``), the arguments are returned
// as a vector of expressions. Note that this vector can be empty (``mod()``).
std::vector<ASTPointer<Expression>> const* arguments() const { return m_arguments.get(); }
private: private:
ASTPointer<Identifier> m_modifierName; ASTPointer<Identifier> m_modifierName;
std::vector<ASTPointer<Expression>> m_arguments; std::unique_ptr<std::vector<ASTPointer<Expression>>> m_arguments;
}; };
/** /**

View File

@ -268,7 +268,7 @@ bool ASTJsonConverter::visit(InheritanceSpecifier const& _node)
{ {
setJsonNode(_node, "InheritanceSpecifier", { setJsonNode(_node, "InheritanceSpecifier", {
make_pair("baseName", toJson(_node.name())), make_pair("baseName", toJson(_node.name())),
make_pair("arguments", _node.arguments() ? toJson(*_node.arguments()) : Json::Value(Json::arrayValue)) make_pair("arguments", _node.arguments() ? toJson(*_node.arguments()) : Json::nullValue)
}); });
return false; return false;
} }
@ -378,7 +378,7 @@ bool ASTJsonConverter::visit(ModifierInvocation const& _node)
{ {
setJsonNode(_node, "ModifierInvocation", { setJsonNode(_node, "ModifierInvocation", {
make_pair("modifierName", toJson(*_node.name())), make_pair("modifierName", toJson(*_node.name())),
make_pair("arguments", toJson(_node.arguments())) make_pair("arguments", _node.arguments() ? toJson(*_node.arguments()) : Json::nullValue)
}); });
return false; return false;
} }

View File

@ -264,7 +264,8 @@ void ModifierInvocation::accept(ASTVisitor& _visitor)
if (_visitor.visit(*this)) if (_visitor.visit(*this))
{ {
m_modifierName->accept(_visitor); m_modifierName->accept(_visitor);
listAccept(m_arguments, _visitor); if (m_arguments)
listAccept(*m_arguments, _visitor);
} }
_visitor.endVisit(*this); _visitor.endVisit(*this);
} }
@ -274,7 +275,8 @@ void ModifierInvocation::accept(ASTConstVisitor& _visitor) const
if (_visitor.visit(*this)) if (_visitor.visit(*this))
{ {
m_modifierName->accept(_visitor); m_modifierName->accept(_visitor);
listAccept(m_arguments, _visitor); if (m_arguments)
listAccept(*m_arguments, _visitor);
} }
_visitor.endVisit(*this); _visitor.endVisit(*this);
} }

View File

@ -222,7 +222,7 @@ void ContractCompiler::appendBaseConstructor(FunctionDefinition const& _construc
if (auto inheritanceSpecifier = dynamic_cast<InheritanceSpecifier const*>(baseArgumentNode)) if (auto inheritanceSpecifier = dynamic_cast<InheritanceSpecifier const*>(baseArgumentNode))
arguments = inheritanceSpecifier->arguments(); arguments = inheritanceSpecifier->arguments();
else if (auto modifierInvocation = dynamic_cast<ModifierInvocation const*>(baseArgumentNode)) else if (auto modifierInvocation = dynamic_cast<ModifierInvocation const*>(baseArgumentNode))
arguments = &modifierInvocation->arguments(); arguments = modifierInvocation->arguments();
solAssert(arguments, ""); solAssert(arguments, "");
solAssert(arguments->size() == constructorType.parameterTypes().size(), ""); solAssert(arguments->size() == constructorType.parameterTypes().size(), "");
for (unsigned i = 0; i < arguments->size(); ++i) for (unsigned i = 0; i < arguments->size(); ++i)
@ -897,13 +897,16 @@ void ContractCompiler::appendModifierOrFunctionCode()
); );
ModifierDefinition const& modifier = m_context.resolveVirtualFunctionModifier(nonVirtualModifier); ModifierDefinition const& modifier = m_context.resolveVirtualFunctionModifier(nonVirtualModifier);
CompilerContext::LocationSetter locationSetter(m_context, modifier); CompilerContext::LocationSetter locationSetter(m_context, modifier);
solAssert(modifier.parameters().size() == modifierInvocation->arguments().size(), ""); std::vector<ASTPointer<Expression>> const& modifierArguments =
modifierInvocation->arguments() ? *modifierInvocation->arguments() : std::vector<ASTPointer<Expression>>();
solAssert(modifier.parameters().size() == modifierArguments.size(), "");
for (unsigned i = 0; i < modifier.parameters().size(); ++i) for (unsigned i = 0; i < modifier.parameters().size(); ++i)
{ {
m_context.addVariable(*modifier.parameters()[i]); m_context.addVariable(*modifier.parameters()[i]);
addedVariables.push_back(modifier.parameters()[i].get()); addedVariables.push_back(modifier.parameters()[i].get());
compileExpression( compileExpression(
*modifierInvocation->arguments()[i], *modifierArguments[i],
modifier.parameters()[i]->annotation().type modifier.parameters()[i]->annotation().type
); );
} }

View File

@ -701,17 +701,17 @@ ASTPointer<ModifierInvocation> Parser::parseModifierInvocation()
RecursionGuard recursionGuard(*this); RecursionGuard recursionGuard(*this);
ASTNodeFactory nodeFactory(*this); ASTNodeFactory nodeFactory(*this);
ASTPointer<Identifier> name(parseIdentifier()); ASTPointer<Identifier> name(parseIdentifier());
vector<ASTPointer<Expression>> arguments; unique_ptr<vector<ASTPointer<Expression>>> arguments;
if (m_scanner->currentToken() == Token::LParen) if (m_scanner->currentToken() == Token::LParen)
{ {
m_scanner->next(); m_scanner->next();
arguments = parseFunctionCallListArguments(); arguments.reset(new vector<ASTPointer<Expression>>(parseFunctionCallListArguments()));
nodeFactory.markEndPosition(); nodeFactory.markEndPosition();
expectToken(Token::RParen); expectToken(Token::RParen);
} }
else else
nodeFactory.setEndPositionFromNode(name); nodeFactory.setEndPositionFromNode(name);
return nodeFactory.createNode<ModifierInvocation>(name, arguments); return nodeFactory.createNode<ModifierInvocation>(name, move(arguments));
} }
ASTPointer<Identifier> Parser::parseIdentifier() ASTPointer<Identifier> Parser::parseIdentifier()

View File

@ -1,3 +1,2 @@
contract A { constructor() public { } } contract A { constructor() public { } }
contract B1 is A { constructor() A() public { } } contract B is A { constructor() A() public { } }
contract B2 is A { constructor() A public { } }

View File

@ -0,0 +1,4 @@
contract A { constructor() public { } }
contract B is A { constructor() A public { } }
// ----
// Warning: Modifier-style base constructor call without arguments.