Merge pull request #2687 from ethereum/show-unimplemented-funcs

Show unimplemented function if trying to instantiate an abstract class
This commit is contained in:
Alex Beregszaszi 2017-08-04 19:46:09 +01:00 committed by GitHub
commit a372941a44
8 changed files with 46 additions and 24 deletions

View File

@ -1,6 +1,7 @@
### 0.4.15 (unreleased)
Features:
* Type Checker: Show unimplemented function if trying to instantiate an abstract class.
Bugfixes:
* Code Generator: ``.delegatecall()`` should always return execution outcome.

View File

@ -112,8 +112,6 @@ bool TypeChecker::visit(ContractDefinition const& _contract)
m_errorReporter.typeError(fallbackFunction->returnParameterList()->location(), "Fallback function cannot return values.");
}
}
if (!function->isImplemented())
_contract.annotation().isFullyImplemented = false;
}
for (auto const& n: _contract.subNodes())
@ -188,7 +186,6 @@ void TypeChecker::checkContractAbstractFunctions(ContractDefinition const& _cont
using FunTypeAndFlag = std::pair<FunctionTypePointer, bool>;
map<string, vector<FunTypeAndFlag>> functions;
bool allBaseConstructorsImplemented = true;
// Search from base to derived
for (ContractDefinition const* contract: boost::adaptors::reverse(_contract.annotation().linearizedBaseContracts))
for (FunctionDefinition const* function: contract->definedFunctions())
@ -199,7 +196,7 @@ void TypeChecker::checkContractAbstractFunctions(ContractDefinition const& _cont
if (!function->isImplemented())
// Base contract's constructor is not fully implemented, no way to get
// out of this.
allBaseConstructorsImplemented = false;
_contract.annotation().unimplementedFunctions.push_back(function);
continue;
}
auto& overloads = functions[function->name()];
@ -219,16 +216,15 @@ void TypeChecker::checkContractAbstractFunctions(ContractDefinition const& _cont
it->second = true;
}
if (!allBaseConstructorsImplemented)
_contract.annotation().isFullyImplemented = false;
// Set to not fully implemented if at least one flag is false.
for (auto const& it: functions)
for (auto const& funAndFlag: it.second)
if (!funAndFlag.second)
{
_contract.annotation().isFullyImplemented = false;
return;
FunctionDefinition const* function = dynamic_cast<FunctionDefinition const*>(&funAndFlag.first->declaration());
solAssert(function, "");
_contract.annotation().unimplementedFunctions.push_back(function);
break;
}
}
@ -266,7 +262,8 @@ void TypeChecker::checkContractAbstractConstructors(ContractDefinition const& _c
}
}
if (!argumentsNeeded.empty())
_contract.annotation().isFullyImplemented = false;
for (ContractDefinition const* contract: argumentsNeeded)
_contract.annotation().unimplementedFunctions.push_back(contract->constructor());
}
void TypeChecker::checkContractIllegalOverrides(ContractDefinition const& _contract)
@ -1523,8 +1520,15 @@ void TypeChecker::endVisit(NewExpression const& _newExpression)
if (!contract)
m_errorReporter.fatalTypeError(_newExpression.location(), "Identifier is not a contract.");
if (!contract->annotation().isFullyImplemented)
m_errorReporter.typeError(_newExpression.location(), "Trying to create an instance of an abstract contract.");
if (!contract->annotation().unimplementedFunctions.empty())
m_errorReporter.typeError(
_newExpression.location(),
SecondarySourceLocation().append(
"Missing implementation:",
contract->annotation().unimplementedFunctions.front()->location()
),
"Trying to create an instance of an abstract contract."
);
if (!contract->constructorIsPublic())
m_errorReporter.typeError(_newExpression.location(), "Contract with internal constructor cannot be created directly.");

View File

@ -79,8 +79,9 @@ struct TypeDeclarationAnnotation: ASTAnnotation
struct ContractDefinitionAnnotation: TypeDeclarationAnnotation, DocumentedAnnotation
{
/// Whether all functions are implemented.
bool isFullyImplemented = true;
/// List of functions without a body. Can also contain functions from base classes,
/// especially constructors.
std::vector<FunctionDefinition const*> unimplementedFunctions;
/// 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

@ -253,7 +253,7 @@ bool ASTJsonConverter::visit(ContractDefinition const& _node)
make_pair("name", _node.name()),
make_pair("documentation", _node.documentation() ? Json::Value(*_node.documentation()) : Json::nullValue),
make_pair("contractKind", contractKind(_node.contractKind())),
make_pair("fullyImplemented", _node.annotation().isFullyImplemented),
make_pair("fullyImplemented", _node.annotation().unimplementedFunctions.empty()),
make_pair("linearizedBaseContracts", getContainerIds(_node.annotation().linearizedBaseContracts)),
make_pair("baseContracts", toJson(_node.baseContracts())),
make_pair("contractDependencies", getContainerIds(_node.annotation().contractDependencies)),

View File

@ -639,7 +639,7 @@ void CompilerStack::compileContract(
{
if (
_compiledContracts.count(&_contract) ||
!_contract.annotation().isFullyImplemented ||
!_contract.annotation().unimplementedFunctions.empty() ||
!_contract.constructorIsPublic()
)
return;

View File

@ -151,6 +151,16 @@ void ErrorReporter::syntaxError(SourceLocation const& _location, string const& _
);
}
void ErrorReporter::typeError(SourceLocation const& _location, SecondarySourceLocation const& _secondaryLocation, string const& _description)
{
error(
Error::Type::TypeError,
_location,
_secondaryLocation,
_description
);
}
void ErrorReporter::typeError(SourceLocation const& _location, string const& _description)
{
error(

View File

@ -73,6 +73,12 @@ public:
void syntaxError(SourceLocation const& _location, std::string const& _description);
void typeError(
SourceLocation const& _location,
SecondarySourceLocation const& _secondaryLocation,
std::string const& _description
);
void typeError(SourceLocation const& _location, std::string const& _description);
void fatalTypeError(SourceLocation const& _location, std::string const& _description);

View File

@ -624,7 +624,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().isFullyImplemented);
BOOST_CHECK(!contract->annotation().unimplementedFunctions.empty());
BOOST_CHECK(!contract->definedFunctions()[0]->isImplemented());
}
@ -640,10 +640,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().isFullyImplemented);
BOOST_CHECK(!base->annotation().unimplementedFunctions.empty());
BOOST_CHECK(!base->definedFunctions()[0]->isImplemented());
BOOST_REQUIRE(derived);
BOOST_CHECK(derived->annotation().isFullyImplemented);
BOOST_CHECK(derived->annotation().unimplementedFunctions.empty());
BOOST_CHECK(derived->definedFunctions()[0]->isImplemented());
}
@ -659,9 +659,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().isFullyImplemented);
BOOST_CHECK(!base->annotation().unimplementedFunctions.empty());
BOOST_REQUIRE(derived);
BOOST_CHECK(!derived->annotation().isFullyImplemented);
BOOST_CHECK(!derived->annotation().unimplementedFunctions.empty());
}
BOOST_AUTO_TEST_CASE(create_abstract_contract)
@ -693,7 +693,7 @@ BOOST_AUTO_TEST_CASE(abstract_contract_constructor_args_optional)
BOOST_CHECK_EQUAL(nodes.size(), 4);
ContractDefinition* derived = dynamic_cast<ContractDefinition*>(nodes[3].get());
BOOST_REQUIRE(derived);
BOOST_CHECK(!derived->annotation().isFullyImplemented);
BOOST_CHECK(!derived->annotation().unimplementedFunctions.empty());
}
BOOST_AUTO_TEST_CASE(abstract_contract_constructor_args_not_provided)
@ -712,7 +712,7 @@ BOOST_AUTO_TEST_CASE(abstract_contract_constructor_args_not_provided)
BOOST_CHECK_EQUAL(nodes.size(), 4);
ContractDefinition* derived = dynamic_cast<ContractDefinition*>(nodes[3].get());
BOOST_REQUIRE(derived);
BOOST_CHECK(!derived->annotation().isFullyImplemented);
BOOST_CHECK(!derived->annotation().unimplementedFunctions.empty());
}
BOOST_AUTO_TEST_CASE(redeclare_implemented_abstract_function_as_abstract)
@ -738,7 +738,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().isFullyImplemented);
BOOST_CHECK(!derived->annotation().unimplementedFunctions.empty());
}
BOOST_AUTO_TEST_CASE(function_canonical_signature)