mirror of
https://github.com/ethereum/solidity
synced 2023-10-03 13:03:40 +00:00
Merge pull request #1479 from ethereum/function_variable_mixin
Disallow mixin of functions and attributes under the same name
This commit is contained in:
commit
bde0b40634
@ -8,6 +8,7 @@ Bugfixes:
|
||||
* Remappings: Prefer longer context over longer prefix.
|
||||
* Type checker, code generator: enable access to events of base contracts' names.
|
||||
* Imports: ``import ".dir/a"`` is not a relative path. Relative paths begin with directory ``.`` or ``..``.
|
||||
* Type checker, disallow inheritances of different kinds (e.g. a function and a modifier) of members of the same name
|
||||
|
||||
### 0.4.7 (2016-12-15)
|
||||
|
||||
|
@ -877,6 +877,13 @@ cannot be resolved.
|
||||
A simple rule to remember is to specify the base classes in
|
||||
the order from "most base-like" to "most derived".
|
||||
|
||||
Inheriting Different Kinds of Members of the Same Name
|
||||
======================================================
|
||||
|
||||
When the inheritance results in a contract with a function and a modifier of the same name, it is considered as an error.
|
||||
This error is produced also by an event and a modifier of the same name, and a function and an event of the same name.
|
||||
As an exception, a state variable accessor can override a public function.
|
||||
|
||||
.. index:: ! contract;abstract, ! abstract contract
|
||||
|
||||
******************
|
||||
|
@ -44,10 +44,19 @@ Declaration const* DeclarationContainer::conflictingDeclaration(
|
||||
|
||||
if (dynamic_cast<FunctionDefinition const*>(&_declaration))
|
||||
{
|
||||
// check that all other declarations with the same name are functions
|
||||
// check that all other declarations with the same name are functions or a public state variable
|
||||
for (Declaration const* declaration: declarations)
|
||||
if (!dynamic_cast<FunctionDefinition const*>(declaration))
|
||||
{
|
||||
if (dynamic_cast<FunctionDefinition const*>(declaration))
|
||||
continue;
|
||||
if (auto variableDeclaration = dynamic_cast<VariableDeclaration const*>(declaration))
|
||||
{
|
||||
if (variableDeclaration->isStateVariable() && !variableDeclaration->isConstant() && variableDeclaration->isPublic())
|
||||
continue;
|
||||
return declaration;
|
||||
}
|
||||
return declaration;
|
||||
}
|
||||
}
|
||||
else if (declarations.size() == 1 && declarations.front() == &_declaration)
|
||||
return nullptr;
|
||||
|
@ -260,10 +260,16 @@ vector<Declaration const*> NameAndTypeResolver::cleanedDeclarations(
|
||||
for (auto it = _declarations.begin(); it != _declarations.end(); ++it)
|
||||
{
|
||||
solAssert(*it, "");
|
||||
// the declaration is functionDefinition while declarations > 1
|
||||
FunctionDefinition const& functionDefinition = dynamic_cast<FunctionDefinition const&>(**it);
|
||||
FunctionType functionType(functionDefinition);
|
||||
for (auto parameter: functionType.parameterTypes() + functionType.returnParameterTypes())
|
||||
// the declaration is functionDefinition or a VariableDeclaration while declarations > 1
|
||||
solAssert(dynamic_cast<FunctionDefinition const*>(*it) || dynamic_cast<VariableDeclaration const*>(*it),
|
||||
"Found overloading involving something not a function or a variable");
|
||||
|
||||
shared_ptr<FunctionType const> functionType { (*it)->functionType(false) };
|
||||
if (!functionType)
|
||||
functionType = (*it)->functionType(true);
|
||||
solAssert(functionType, "failed to determine the function type of the overloaded");
|
||||
|
||||
for (auto parameter: functionType->parameterTypes() + functionType->returnParameterTypes())
|
||||
if (!parameter)
|
||||
reportFatalDeclarationError(_identifier.location(), "Function type can not be used in this context");
|
||||
|
||||
@ -272,8 +278,10 @@ vector<Declaration const*> NameAndTypeResolver::cleanedDeclarations(
|
||||
uniqueFunctions.end(),
|
||||
[&](Declaration const* d)
|
||||
{
|
||||
FunctionType newFunctionType(dynamic_cast<FunctionDefinition const&>(*d));
|
||||
return functionType.hasEqualArgumentTypes(newFunctionType);
|
||||
shared_ptr<FunctionType const> newFunctionType { d->functionType(false) };
|
||||
if (!newFunctionType)
|
||||
newFunctionType = d->functionType(true);
|
||||
return newFunctionType && functionType->hasEqualArgumentTypes(*newFunctionType);
|
||||
}
|
||||
))
|
||||
uniqueFunctions.push_back(*it);
|
||||
@ -289,7 +297,39 @@ void NameAndTypeResolver::importInheritedScope(ContractDefinition const& _base)
|
||||
for (auto const& declaration: nameAndDeclaration.second)
|
||||
// Import if it was declared in the base, is not the constructor and is visible in derived classes
|
||||
if (declaration->scope() == &_base && declaration->isVisibleInDerivedContracts())
|
||||
m_currentScope->registerDeclaration(*declaration);
|
||||
if (!m_currentScope->registerDeclaration(*declaration))
|
||||
{
|
||||
SourceLocation firstDeclarationLocation;
|
||||
SourceLocation secondDeclarationLocation;
|
||||
Declaration const* conflictingDeclaration = m_currentScope->conflictingDeclaration(*declaration);
|
||||
solAssert(conflictingDeclaration, "");
|
||||
|
||||
// Usual shadowing is not an error
|
||||
if (dynamic_cast<VariableDeclaration const*>(declaration) && dynamic_cast<VariableDeclaration const*>(conflictingDeclaration))
|
||||
continue;
|
||||
|
||||
// Usual shadowing is not an error
|
||||
if (dynamic_cast<ModifierDefinition const*>(declaration) && dynamic_cast<ModifierDefinition const*>(conflictingDeclaration))
|
||||
continue;
|
||||
|
||||
if (declaration->location().start < conflictingDeclaration->location().start)
|
||||
{
|
||||
firstDeclarationLocation = declaration->location();
|
||||
secondDeclarationLocation = conflictingDeclaration->location();
|
||||
}
|
||||
else
|
||||
{
|
||||
firstDeclarationLocation = conflictingDeclaration->location();
|
||||
secondDeclarationLocation = declaration->location();
|
||||
}
|
||||
|
||||
reportDeclarationError(
|
||||
secondDeclarationLocation,
|
||||
"Identifier already declared.",
|
||||
firstDeclarationLocation,
|
||||
"The previous declaration is here:"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
void NameAndTypeResolver::linearizeBaseContracts(ContractDefinition& _contract)
|
||||
|
@ -1500,8 +1500,23 @@ bool TypeChecker::visit(Identifier const& _identifier)
|
||||
if (!annotation.referencedDeclaration)
|
||||
{
|
||||
if (!annotation.argumentTypes)
|
||||
fatalTypeError(_identifier.location(), "Unable to determine overloaded type.");
|
||||
if (annotation.overloadedDeclarations.empty())
|
||||
{
|
||||
// The identifier should be a public state variable shadowing other functions
|
||||
vector<Declaration const*> candidates;
|
||||
|
||||
for (Declaration const* declaration: annotation.overloadedDeclarations)
|
||||
{
|
||||
if (VariableDeclaration const* variableDeclaration = dynamic_cast<decltype(variableDeclaration)>(declaration))
|
||||
candidates.push_back(declaration);
|
||||
}
|
||||
if (candidates.empty())
|
||||
fatalTypeError(_identifier.location(), "No matching declaration found after variable lookup.");
|
||||
else if (candidates.size() == 1)
|
||||
annotation.referencedDeclaration = candidates.front();
|
||||
else
|
||||
fatalTypeError(_identifier.location(), "No unique declaration found after variable lookup.");
|
||||
}
|
||||
else if (annotation.overloadedDeclarations.empty())
|
||||
fatalTypeError(_identifier.location(), "No candidates for overload resolution found.");
|
||||
else if (annotation.overloadedDeclarations.size() == 1)
|
||||
annotation.referencedDeclaration = *annotation.overloadedDeclarations.begin();
|
||||
|
@ -274,6 +274,45 @@ TypeDeclarationAnnotation& EnumDefinition::annotation() const
|
||||
return static_cast<TypeDeclarationAnnotation&>(*m_annotation);
|
||||
}
|
||||
|
||||
shared_ptr<FunctionType> FunctionDefinition::functionType(bool _internal) const
|
||||
{
|
||||
if (_internal)
|
||||
{
|
||||
switch (visibility())
|
||||
{
|
||||
case Declaration::Visibility::Default:
|
||||
solAssert(false, "visibility() should not return Default");
|
||||
case Declaration::Visibility::Private:
|
||||
case Declaration::Visibility::Internal:
|
||||
case Declaration::Visibility::Public:
|
||||
return make_shared<FunctionType>(*this, _internal);
|
||||
case Declaration::Visibility::External:
|
||||
return {};
|
||||
default:
|
||||
solAssert(false, "visibility() should not return a Visibility");
|
||||
}
|
||||
}
|
||||
else
|
||||
{
|
||||
switch (visibility())
|
||||
{
|
||||
case Declaration::Visibility::Default:
|
||||
solAssert(false, "visibility() should not return Default");
|
||||
case Declaration::Visibility::Private:
|
||||
case Declaration::Visibility::Internal:
|
||||
return {};
|
||||
case Declaration::Visibility::Public:
|
||||
case Declaration::Visibility::External:
|
||||
return make_shared<FunctionType>(*this, _internal);
|
||||
default:
|
||||
solAssert(false, "visibility() should not return a Visibility");
|
||||
}
|
||||
}
|
||||
|
||||
// To make the compiler happy
|
||||
return {};
|
||||
}
|
||||
|
||||
TypePointer FunctionDefinition::type() const
|
||||
{
|
||||
return make_shared<FunctionType>(*this);
|
||||
@ -308,6 +347,14 @@ TypePointer EventDefinition::type() const
|
||||
return make_shared<FunctionType>(*this);
|
||||
}
|
||||
|
||||
std::shared_ptr<FunctionType> EventDefinition::functionType(bool _internal) const
|
||||
{
|
||||
if (_internal)
|
||||
return make_shared<FunctionType>(*this);
|
||||
else
|
||||
return {};
|
||||
}
|
||||
|
||||
EventDefinitionAnnotation& EventDefinition::annotation() const
|
||||
{
|
||||
if (!m_annotation)
|
||||
@ -365,6 +412,28 @@ TypePointer VariableDeclaration::type() const
|
||||
return annotation().type;
|
||||
}
|
||||
|
||||
shared_ptr<FunctionType> VariableDeclaration::functionType(bool _internal) const
|
||||
{
|
||||
if (_internal)
|
||||
return {};
|
||||
switch (visibility())
|
||||
{
|
||||
case Declaration::Visibility::Default:
|
||||
solAssert(false, "visibility() should not return Default");
|
||||
case Declaration::Visibility::Private:
|
||||
case Declaration::Visibility::Internal:
|
||||
return {};
|
||||
case Declaration::Visibility::Public:
|
||||
case Declaration::Visibility::External:
|
||||
return make_shared<FunctionType>(*this);
|
||||
default:
|
||||
solAssert(false, "visibility() should not return a Visibility");
|
||||
}
|
||||
|
||||
// To make the compiler happy
|
||||
return {};
|
||||
}
|
||||
|
||||
VariableDeclarationAnnotation& VariableDeclaration::annotation() const
|
||||
{
|
||||
if (!m_annotation)
|
||||
|
@ -171,6 +171,10 @@ public:
|
||||
/// This can only be called once types of variable declarations have already been resolved.
|
||||
virtual TypePointer type() const = 0;
|
||||
|
||||
/// @param _internal false indicates external interface is concerned, true indicates internal interface is concerned.
|
||||
/// @returns null when it is not accessible as a function.
|
||||
virtual std::shared_ptr<FunctionType> functionType(bool /*_internal*/) const { return {}; }
|
||||
|
||||
protected:
|
||||
virtual Visibility defaultVisibility() const { return Visibility::Public; }
|
||||
|
||||
@ -581,6 +585,10 @@ public:
|
||||
|
||||
virtual TypePointer type() const override;
|
||||
|
||||
/// @param _internal false indicates external interface is concerned, true indicates internal interface is concerned.
|
||||
/// @returns null when it is not accessible as a function.
|
||||
virtual std::shared_ptr<FunctionType> functionType(bool /*_internal*/) const override;
|
||||
|
||||
virtual FunctionDefinitionAnnotation& annotation() const override;
|
||||
|
||||
private:
|
||||
@ -643,6 +651,10 @@ public:
|
||||
|
||||
virtual TypePointer type() const override;
|
||||
|
||||
/// @param _internal false indicates external interface is concerned, true indicates internal interface is concerned.
|
||||
/// @returns null when it is not accessible as a function.
|
||||
virtual std::shared_ptr<FunctionType> functionType(bool /*_internal*/) const override;
|
||||
|
||||
virtual VariableDeclarationAnnotation& annotation() const override;
|
||||
|
||||
protected:
|
||||
@ -740,6 +752,7 @@ public:
|
||||
bool isAnonymous() const { return m_anonymous; }
|
||||
|
||||
virtual TypePointer type() const override;
|
||||
virtual std::shared_ptr<FunctionType> functionType(bool /*_internal*/) const override;
|
||||
|
||||
virtual EventDefinitionAnnotation& annotation() const override;
|
||||
|
||||
|
@ -4869,60 +4869,6 @@ BOOST_AUTO_TEST_CASE(proper_order_of_overwriting_of_attributes)
|
||||
BOOST_CHECK(callContractFunction("ok()") == encodeArgs(false));
|
||||
}
|
||||
|
||||
BOOST_AUTO_TEST_CASE(proper_overwriting_accessor_by_function)
|
||||
{
|
||||
// bug #1798
|
||||
char const* sourceCode = R"(
|
||||
contract attribute {
|
||||
bool ok = false;
|
||||
}
|
||||
contract func {
|
||||
function ok() returns (bool) { return true; }
|
||||
}
|
||||
|
||||
contract attr_func is attribute, func {
|
||||
function checkOk() returns (bool) { return ok(); }
|
||||
}
|
||||
contract func_attr is func, attribute {
|
||||
function checkOk() returns (bool) { return ok; }
|
||||
}
|
||||
)";
|
||||
compileAndRun(sourceCode, 0, "attr_func");
|
||||
BOOST_CHECK(callContractFunction("ok()") == encodeArgs(true));
|
||||
compileAndRun(sourceCode, 0, "func_attr");
|
||||
BOOST_CHECK(callContractFunction("checkOk()") == encodeArgs(false));
|
||||
}
|
||||
|
||||
|
||||
BOOST_AUTO_TEST_CASE(overwriting_inheritance)
|
||||
{
|
||||
// bug #1798
|
||||
char const* sourceCode = R"(
|
||||
contract A {
|
||||
function ok() returns (uint) { return 1; }
|
||||
}
|
||||
contract B {
|
||||
function ok() returns (uint) { return 2; }
|
||||
}
|
||||
contract C {
|
||||
uint ok = 6;
|
||||
}
|
||||
contract AB is A, B {
|
||||
function ok() returns (uint) { return 4; }
|
||||
}
|
||||
contract reversedE is C, AB {
|
||||
function checkOk() returns (uint) { return ok(); }
|
||||
}
|
||||
contract E is AB, C {
|
||||
function checkOk() returns (uint) { return ok; }
|
||||
}
|
||||
)";
|
||||
compileAndRun(sourceCode, 0, "reversedE");
|
||||
BOOST_CHECK(callContractFunction("checkOk()") == encodeArgs(4));
|
||||
compileAndRun(sourceCode, 0, "E");
|
||||
BOOST_CHECK(callContractFunction("checkOk()") == encodeArgs(6));
|
||||
}
|
||||
|
||||
BOOST_AUTO_TEST_CASE(struct_assign_reference_to_struct)
|
||||
{
|
||||
char const* sourceCode = R"(
|
||||
|
@ -1056,7 +1056,9 @@ BOOST_AUTO_TEST_CASE(modifier_overrides_function)
|
||||
contract A { modifier mod(uint a) { _; } }
|
||||
contract B is A { function mod(uint a) { } }
|
||||
)";
|
||||
CHECK_ERROR(text, TypeError, "");
|
||||
// Error: Identifier already declared.
|
||||
// Error: Override changes modifier to function.
|
||||
CHECK_ERROR_ALLOW_MULTI(text, DeclarationError, "");
|
||||
}
|
||||
|
||||
BOOST_AUTO_TEST_CASE(function_overrides_modifier)
|
||||
@ -1065,7 +1067,9 @@ BOOST_AUTO_TEST_CASE(function_overrides_modifier)
|
||||
contract A { function mod(uint a) { } }
|
||||
contract B is A { modifier mod(uint a) { _; } }
|
||||
)";
|
||||
CHECK_ERROR(text, TypeError, "");
|
||||
// Error: Identifier already declared.
|
||||
// Error: Override changes function to modifier.
|
||||
CHECK_ERROR_ALLOW_MULTI(text, DeclarationError, "");
|
||||
}
|
||||
|
||||
BOOST_AUTO_TEST_CASE(modifier_returns_value)
|
||||
@ -4304,6 +4308,25 @@ BOOST_AUTO_TEST_CASE(illegal_override_payable_nonpayable)
|
||||
CHECK_ERROR(text, TypeError, "");
|
||||
}
|
||||
|
||||
BOOST_AUTO_TEST_CASE(function_variable_mixin)
|
||||
{
|
||||
// bug #1798 (cpp-ethereum), related to #1286 (solidity)
|
||||
char const* text = R"(
|
||||
contract attribute {
|
||||
bool ok = false;
|
||||
}
|
||||
contract func {
|
||||
function ok() returns (bool) { return true; }
|
||||
}
|
||||
|
||||
contract attr_func is attribute, func {
|
||||
function checkOk() returns (bool) { return ok(); }
|
||||
}
|
||||
)";
|
||||
CHECK_ERROR(text, DeclarationError, "");
|
||||
}
|
||||
|
||||
|
||||
BOOST_AUTO_TEST_CASE(payable_constant_conflict)
|
||||
{
|
||||
char const* text = R"(
|
||||
|
Loading…
Reference in New Issue
Block a user