aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--Changelog.md1
-rw-r--r--docs/contracts.rst7
-rw-r--r--libsolidity/analysis/DeclarationContainer.cpp13
-rw-r--r--libsolidity/analysis/NameAndTypeResolver.cpp54
-rw-r--r--libsolidity/analysis/TypeChecker.cpp19
-rw-r--r--libsolidity/ast/AST.cpp69
-rw-r--r--libsolidity/ast/AST.h13
-rw-r--r--test/libsolidity/SolidityEndToEndTest.cpp54
-rw-r--r--test/libsolidity/SolidityNameAndTypeResolution.cpp27
9 files changed, 190 insertions, 67 deletions
diff --git a/Changelog.md b/Changelog.md
index 3e999d13..bef98f15 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -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)
diff --git a/docs/contracts.rst b/docs/contracts.rst
index e82b7495..edc42c3d 100644
--- a/docs/contracts.rst
+++ b/docs/contracts.rst
@@ -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
******************
diff --git a/libsolidity/analysis/DeclarationContainer.cpp b/libsolidity/analysis/DeclarationContainer.cpp
index 1599b83a..f8c12c5b 100644
--- a/libsolidity/analysis/DeclarationContainer.cpp
+++ b/libsolidity/analysis/DeclarationContainer.cpp
@@ -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;
diff --git a/libsolidity/analysis/NameAndTypeResolver.cpp b/libsolidity/analysis/NameAndTypeResolver.cpp
index 2a33a501..08323243 100644
--- a/libsolidity/analysis/NameAndTypeResolver.cpp
+++ b/libsolidity/analysis/NameAndTypeResolver.cpp
@@ -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)
diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp
index e414e27c..67c8ac17 100644
--- a/libsolidity/analysis/TypeChecker.cpp
+++ b/libsolidity/analysis/TypeChecker.cpp
@@ -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();
diff --git a/libsolidity/ast/AST.cpp b/libsolidity/ast/AST.cpp
index 78d8949c..6f7a64dc 100644
--- a/libsolidity/ast/AST.cpp
+++ b/libsolidity/ast/AST.cpp
@@ -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)
diff --git a/libsolidity/ast/AST.h b/libsolidity/ast/AST.h
index ab4be1ea..2d092408 100644
--- a/libsolidity/ast/AST.h
+++ b/libsolidity/ast/AST.h
@@ -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;
diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index d8539524..19161831 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -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"(
diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp
index 576421fd..9f6ea2b3 100644
--- a/test/libsolidity/SolidityNameAndTypeResolution.cpp
+++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp
@@ -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"(
generated by cgit (git 2.34.1) at 2024-09-29 10:23:47 +0800