Merge pull request #7939 from ethereum/more_tests_override_stuff

Ambiguous override for public state variables
This commit is contained in:
chriseth 2019-12-11 17:16:59 +01:00 committed by GitHub
commit f22bd769ff
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
41 changed files with 837 additions and 375 deletions

File diff suppressed because it is too large Load Diff

View File

@ -21,20 +21,109 @@
#pragma once
#include <libsolidity/ast/ASTForward.h>
#include <libsolidity/ast/ASTEnums.h>
#include <liblangutil/SourceLocation.h>
#include <map>
#include <functional>
#include <set>
#include <variant>
#include <optional>
namespace langutil
{
class ErrorReporter;
struct SourceLocation;
}
namespace dev
{
namespace solidity
{
class FunctionType;
class ModifierType;
/**
* Class that represents a function, public state variable or modifier
* and helps with overload checking.
* Regular comparison is performed based on AST node, while CompareBySignature
* results in two elements being equal when they can override each
* other.
*/
class OverrideProxy
{
public:
OverrideProxy() {}
explicit OverrideProxy(FunctionDefinition const* _fun): m_item{_fun} {}
explicit OverrideProxy(ModifierDefinition const* _mod): m_item{_mod} {}
explicit OverrideProxy(VariableDeclaration const* _var): m_item{_var} {}
bool operator<(OverrideProxy const& _other) const;
struct CompareBySignature
{
bool operator()(OverrideProxy const& _a, OverrideProxy const& _b) const;
};
bool isVariable() const;
bool isFunction() const;
bool isModifier() const;
size_t id() const;
std::shared_ptr<OverrideSpecifier> overrides() const;
std::set<OverrideProxy> baseFunctions() const;
/// This stores the item in the list of base items.
void storeBaseFunction(OverrideProxy const& _base) const;
std::string const& name() const;
ContractDefinition const& contract() const;
std::string const& contractName() const;
Visibility visibility() const;
StateMutability stateMutability() const;
bool virtualSemantics() const;
/// @returns receive / fallback / function (only the latter for modifiers and variables);
langutil::Token functionKind() const;
FunctionType const* functionType() const;
ModifierType const* modifierType() const;
langutil::SourceLocation const& location() const;
std::string astNodeName() const;
std::string astNodeNameCapitalized() const;
std::string distinguishingProperty() const;
/// @returns true if this AST elements supports the feature of being unimplemented
/// and is actually not implemented.
bool unimplemented() const;
/**
* Struct to help comparing override items about whether they override each other.
* Does not produce a total order.
*/
struct OverrideComparator
{
std::string name;
std::optional<langutil::Token> functionKind;
std::optional<std::vector<std::string>> parameterTypes;
bool operator<(OverrideComparator const& _other) const;
};
/// @returns a structure used to compare override items with regards to whether
/// they override each other.
OverrideComparator const& overrideComparator() const;
private:
std::variant<
FunctionDefinition const*,
ModifierDefinition const*,
VariableDeclaration const*
> m_item;
std::shared_ptr<OverrideComparator> mutable m_comparator;
};
/**
* Component that verifies override properties.
@ -51,67 +140,56 @@ public:
void check(ContractDefinition const& _contract);
private:
/**
* Comparator that compares
* - functions such that equality means that the functions override each other
* - modifiers by name
* - contracts by AST id.
*/
struct LessFunction
struct CompareByID
{
bool operator()(ModifierDefinition const* _a, ModifierDefinition const* _b) const;
bool operator()(FunctionDefinition const* _a, FunctionDefinition const* _b) const;
bool operator()(ContractDefinition const* _a, ContractDefinition const* _b) const;
};
using FunctionMultiSet = std::multiset<FunctionDefinition const*, LessFunction>;
using ModifierMultiSet = std::multiset<ModifierDefinition const*, LessFunction>;
void checkIllegalOverrides(ContractDefinition const& _contract);
/// Performs various checks related to @a _overriding overriding @a _super like
/// different return type, invalid visibility change, etc.
/// Works on functions, modifiers and public state variables.
/// Also stores @a _super as a base function of @a _function in its AST annotations.
template<class T, class U>
void checkOverride(T const& _overriding, U const& _super);
void checkOverride(OverrideProxy const& _overriding, OverrideProxy const& _super);
void overrideListError(
CallableDeclaration const& _callable,
std::set<ContractDefinition const*, LessFunction> _secondary,
OverrideProxy const& _item,
std::set<ContractDefinition const*, CompareByID> _secondary,
std::string const& _message1,
std::string const& _message2
);
void overrideError(
Declaration const& _overriding,
Declaration const& _super,
std::string _message,
std::string _secondaryMsg = "Overridden function is here:"
std::string const& _message,
std::string const& _secondaryMsg = "Overridden function is here:"
);
void overrideError(
OverrideProxy const& _overriding,
OverrideProxy const& _super,
std::string const& _message,
std::string const& _secondaryMsg = "Overridden function is here:"
);
/// Checks for functions in different base contracts which conflict with each
/// other and thus need to be overridden explicitly.
void checkAmbiguousOverrides(ContractDefinition const& _contract) const;
void checkAmbiguousOverridesInternal(std::set<
CallableDeclaration const*,
std::function<bool(CallableDeclaration const*, CallableDeclaration const*)>
> _baseCallables, langutil::SourceLocation const& _location) const;
void checkAmbiguousOverridesInternal(std::set<OverrideProxy> _baseCallables, langutil::SourceLocation const& _location) const;
/// Resolves an override list of UserDefinedTypeNames to a list of contracts.
std::set<ContractDefinition const*, LessFunction> resolveOverrideList(OverrideSpecifier const& _overrides) const;
std::set<ContractDefinition const*, CompareByID> resolveOverrideList(OverrideSpecifier const& _overrides) const;
template <class T>
void checkOverrideList(
std::multiset<T const*, LessFunction> const& _funcSet,
T const& _function
);
using OverrideProxyBySignatureMultiSet = std::multiset<OverrideProxy, OverrideProxy::CompareBySignature>;
/// Returns all functions of bases that have not yet been overwritten.
void checkOverrideList(OverrideProxy _item, OverrideProxyBySignatureMultiSet const& _inherited);
/// Returns all functions of bases (including public state variables) that have not yet been overwritten.
/// May contain the same function multiple times when used with shared bases.
FunctionMultiSet const& inheritedFunctions(ContractDefinition const& _contract) const;
ModifierMultiSet const& inheritedModifiers(ContractDefinition const& _contract) const;
OverrideProxyBySignatureMultiSet const& inheritedFunctions(ContractDefinition const& _contract) const;
OverrideProxyBySignatureMultiSet const& inheritedModifiers(ContractDefinition const& _contract) const;
langutil::ErrorReporter& m_errorReporter;
/// Cache for inheritedFunctions().
std::map<ContractDefinition const*, FunctionMultiSet> mutable m_inheritedFunctions;
std::map<ContractDefinition const*, ModifierMultiSet> mutable m_contractBaseModifiers;
std::map<ContractDefinition const*, OverrideProxyBySignatureMultiSet> mutable m_inheritedFunctions;
std::map<ContractDefinition const*, OverrideProxyBySignatureMultiSet> mutable m_inheritedModifiers;
};
}

View File

@ -128,6 +128,8 @@ struct VariableDeclarationAnnotation: ASTAnnotation
{
/// Type of variable (type of identifier referencing this variable).
TypePointer type = nullptr;
/// The set of functions this (public state) variable overrides.
std::set<CallableDeclaration const*> baseFunctions;
};
struct StatementAnnotation: ASTAnnotation, DocumentedAnnotation

View File

@ -386,6 +386,8 @@ bool ASTJsonConverter::visit(VariableDeclaration const& _node)
};
if (m_inEvent)
attributes.emplace_back("indexed", _node.isIndexed());
if (!_node.annotation().baseFunctions.empty())
attributes.emplace_back(make_pair("baseFunctions", getContainerIds(_node.annotation().baseFunctions, true)));
setJsonNode(_node, "VariableDeclaration", std::move(attributes));
return false;
}

View File

@ -5,4 +5,4 @@ contract D is C {
fallback() external {}
}
// ----
// TypeError: (66-88): Overriding function is missing 'override' specifier.
// TypeError: (66-88): Overriding function is missing "override" specifier.

View File

@ -7,4 +7,4 @@ contract E is D {
fallback() external {}
}
// ----
// TypeError: (86-108): Overriding function is missing 'override' specifier.
// TypeError: (86-108): Overriding function is missing "override" specifier.

View File

@ -5,4 +5,4 @@ contract D is C {
receive() external payable {}
}
// ----
// TypeError: (73-102): Overriding function is missing 'override' specifier.
// TypeError: (73-102): Overriding function is missing "override" specifier.

View File

@ -7,4 +7,4 @@ contract E is D {
receive() external payable {}
}
// ----
// TypeError: (93-122): Overriding function is missing 'override' specifier.
// TypeError: (93-122): Overriding function is missing "override" specifier.

View File

@ -1,5 +1,5 @@
contract B { function f() virtual public {} }
contract C is B { function f() public view {} }
// ----
// TypeError: (64-91): Overriding function is missing 'override' specifier.
// TypeError: (64-91): Overriding function is missing "override" specifier.
// TypeError: (64-91): Overriding function changes state mutability from "nonpayable" to "view".

View File

@ -7,5 +7,5 @@ contract B is I {
function f() public pure returns (uint, uint) {}
}
// ----
// TypeError: (182-230): Overriding function is missing 'override' specifier.
// TypeError: (182-230): Overriding function is missing "override" specifier.
// TypeError: (182-230): Overriding function return types differ.

View File

@ -0,0 +1,13 @@
interface I {
}
contract A is I
{
uint public f;
}
abstract contract B is I
{
function f() external virtual returns (uint);
}
abstract contract C is A, B {}
// ----
// TypeError: (128-158): Derived contract must override function "f". Two or more base classes define function with same name and parameter types. Since one of the bases defines a public state variable which cannot be overridden, you have to change the inheritance layout or the names of the functions.

View File

@ -0,0 +1,12 @@
interface I {
function f() external returns (uint);
}
abstract contract A is I
{
uint public override f;
}
abstract contract B is I
{
}
// This is fine because `f` is not implemented in `I` and `A.f` is the only mention below `I`.
abstract contract C is A, B {}

View File

@ -0,0 +1,14 @@
interface I {
function f() external returns (uint);
}
contract A is I
{
uint public override f;
}
abstract contract B is I
{
function f() external virtual override returns (uint);
}
abstract contract C is A, B {}
// ----
// TypeError: (185-215): Derived contract must override function "f". Two or more base classes define function with same name and parameter types. Since one of the bases defines a public state variable which cannot be overridden, you have to change the inheritance layout or the names of the functions.

View File

@ -0,0 +1,14 @@
interface I {
function f() external returns (uint);
}
contract A is I
{
uint public override f;
}
abstract contract B is I
{
function f() external virtual override returns (uint) { return 2; }
}
abstract contract C is A, B {}
// ----
// TypeError: (198-228): Derived contract must override function "f". Two or more base classes define function with same name and parameter types. Since one of the bases defines a public state variable which cannot be overridden, you have to change the inheritance layout or the names of the functions.

View File

@ -0,0 +1,13 @@
contract I {
function f() external pure virtual returns (uint) { return 1; }
}
contract A is I
{
}
contract B is I
{
}
contract C is A, B
{
uint public override f;
}

View File

@ -0,0 +1,14 @@
contract I {
function f() external pure virtual returns (uint) { return 1; }
}
contract A is I
{
uint public override f;
}
contract B is I
{
function f() external pure virtual override returns (uint) { return 2; }
}
contract C is A, B {}
// ----
// TypeError: (219-240): Derived contract must override function "f". Two or more base classes define function with same name and parameter types. Since one of the bases defines a public state variable which cannot be overridden, you have to change the inheritance layout or the names of the functions.

View File

@ -0,0 +1,13 @@
contract I {
function f() external pure virtual returns (uint) { return 1; }
}
contract A is I
{
uint public override f;
}
contract B is I
{
}
contract C is A, B {}
// ----
// TypeError: (145-166): Derived contract must override function "f". Two or more base classes define function with same name and parameter types. Since one of the bases defines a public state variable which cannot be overridden, you have to change the inheritance layout or the names of the functions.

View File

@ -7,5 +7,5 @@ abstract contract X is A {
function test2() external override(A) returns (uint256) {}
}
// ----
// TypeError: (153-198): Overriding function is missing 'override' specifier.
// TypeError: (153-198): Overriding function is missing "override" specifier.
// TypeError: (76-122): Trying to override non-virtual function. Did you forget to add "virtual"?

View File

@ -18,5 +18,5 @@ abstract contract X is A, B, C, D {
function foo() internal override(A, C) virtual returns (uint256);
}
// ----
// TypeError: (533-550): Invalid contract specified in override list: C.
// TypeError: (603-617): Function needs to specify overridden contracts B and D.
// TypeError: (533-550): Invalid contract specified in override list: "C".
// TypeError: (603-617): Function needs to specify overridden contracts "B" and "D".

View File

@ -6,4 +6,4 @@ abstract contract X is A {
}
// ----
// DeclarationError: (73-100): Identifier already declared.
// TypeError: (84-92): Public state variable has override specified but does not override anything.
// TypeError: (23-41): Cannot override public state variable.

View File

@ -0,0 +1,21 @@
interface A {
function foo() external returns (uint);
function goo() external returns (uint);
}
interface B {
function foo() external returns (uint);
function goo() external returns (uint);
}
contract X is A, B {
uint public override(A, B) foo;
function goo() external virtual override(A, B) returns (uint) {}
}
abstract contract T is A {
function foo() external virtual override returns (uint);
function goo() external virtual override returns (uint);
}
contract Y is X, T {
}
// ----
// TypeError: (484-506): Derived contract must override function "foo". Two or more base classes define function with same name and parameter types. Since one of the bases defines a public state variable which cannot be overridden, you have to change the inheritance layout or the names of the functions.
// TypeError: (484-506): Derived contract must override function "goo". Two or more base classes define function with same name and parameter types.

View File

@ -0,0 +1,11 @@
interface A {
function foo() external returns (uint);
}
interface B {
function foo() external returns (uint);
}
contract X is A, B {
uint public override(A) foo;
}
// ----
// TypeError: (154-165): Public state variable needs to specify overridden contract "B".

View File

@ -5,4 +5,5 @@ contract X is A {
uint public foo;
}
// ----
// TypeError: (100-115): Overriding public state variable is missing "override" specifier.
// TypeError: (100-115): Public state variables can only override functions with external visibility.

View File

@ -0,0 +1,9 @@
contract A {
uint public foo;
}
contract X is A {
uint public override foo;
}
// ----
// DeclarationError: (55-79): Identifier already declared.
// TypeError: (17-32): Cannot override public state variable.

View File

@ -0,0 +1,10 @@
interface A {
function foo() external returns (uint);
}
interface B {
function foo() external returns (uint);
}
contract X is A, B {
uint public override(A, B) foo;
}
// ----

View File

@ -0,0 +1,12 @@
interface A {
function foo() external returns (uint);
}
interface B {
function foo() external returns (uint);
}
contract X is A, B {
uint public override(A, B) foo;
}
contract Y is X {
}
// ----

View File

@ -0,0 +1,10 @@
interface A {
function foo() external returns (uint);
}
contract B {
uint public foo;
}
contract X is A, B {
}
// ----
// TypeError: (96-118): Derived contract must override function "foo". Two or more base classes define function with same name and parameter types. Since one of the bases defines a public state variable which cannot be overridden, you have to change the inheritance layout or the names of the functions.

View File

@ -0,0 +1,9 @@
interface A {
function foo() external returns (uint);
}
interface B {}
contract X is A {
uint public override(A, B) foo;
}
// ----
// TypeError: (106-120): Invalid contract specified in override list: "B".

View File

@ -8,4 +8,4 @@ contract X is A, B {
uint public override foo;
}
// ----
// TypeError: (162-211): Derived contract must override function "foo". Two or more base classes define function with same name and parameter types.
// TypeError: (196-204): Public state variable needs to specify overridden contracts "A" and "B".

View File

@ -9,3 +9,5 @@ contract X is A, B {
}
// ----
// DeclarationError: (136-160): Identifier already declared.
// TypeError: (14-29): Cannot override public state variable.
// TypeError: (148-156): Public state variable needs to specify overridden contracts "A" and "B".

View File

@ -11,4 +11,4 @@ contract X is B, C {
uint public override foo;
}
// ----
// TypeError: (271-320): Derived contract must override function "foo". Two or more base classes define function with same name and parameter types.
// TypeError: (305-313): Public state variable needs to specify overridden contracts "B" and "C".

View File

@ -12,4 +12,5 @@ contract X is B, C {
}
// ----
// DeclarationError: (245-269): Identifier already declared.
// TypeError: (223-272): Derived contract must override function "foo". Two or more base classes define function with same name and parameter types.
// TypeError: (100-124): Cannot override public state variable.
// TypeError: (257-265): Public state variable needs to specify overridden contracts "B" and "C".

View File

@ -0,0 +1,17 @@
contract A {
function foo() external virtual pure returns(uint) { return 5; }
}
contract B is A {
uint public override foo;
}
contract C is A {
function foo() external virtual override pure returns(uint) { return 5; }
}
contract X is B, C {
uint public override(A, C) foo;
}
// ----
// DeclarationError: (245-275): Identifier already declared.
// TypeError: (100-124): Cannot override public state variable.
// TypeError: (257-271): Public state variable needs to specify overridden contract "B".
// TypeError: (257-271): Invalid contract specified in override list: "A".

View File

@ -8,4 +8,3 @@ contract X is A, B {
uint public override(A, B) foo;
}
// ----
// TypeError: (162-217): Derived contract must override function "foo". Two or more base classes define function with same name and parameter types.

View File

@ -1,5 +1,5 @@
contract B { function f() virtual public view {} }
contract C is B { function f() public {} }
// ----
// TypeError: (69-91): Overriding function is missing 'override' specifier.
// TypeError: (69-91): Overriding function is missing "override" specifier.
// TypeError: (69-91): Overriding function changes state mutability from "view" to "nonpayable".

View File

@ -6,3 +6,5 @@ contract C is A {
}
// ----
// DeclarationError: (50-87): Identifier already declared.
// TypeError: (50-87): Overriding function is missing "override" specifier.
// TypeError: (14-27): Cannot override public state variable.

View File

@ -2,4 +2,4 @@ contract A { function mod(uint a) public { } }
contract B is A { modifier mod(uint a) { _; } }
// ----
// DeclarationError: (65-92): Identifier already declared.
// TypeError: (65-92): Override changes function to modifier.
// TypeError: (65-92): Override changes function or public state variable to modifier.

View File

@ -2,5 +2,5 @@ contract A { modifier mod(uint a) { _; } }
contract B is A { modifier mod(uint8 a) { _; } }
// ----
// TypeError: (61-89): Override changes modifier signature.
// TypeError: (61-89): Overriding modifier is missing 'override' specifier.
// TypeError: (61-89): Overriding modifier is missing "override" specifier.
// TypeError: (13-40): Trying to override non-virtual modifier. Did you forget to add "virtual"?

View File

@ -1,5 +1,5 @@
contract B { function f() virtual internal {} }
contract C is B { function f() public {} }
// ----
// TypeError: (66-88): Overriding function is missing 'override' specifier.
// TypeError: (66-88): Overriding function is missing "override" specifier.
// TypeError: (66-88): Overriding function visibility differs.

View File

@ -1,5 +1,5 @@
contract B { function f() payable virtual public {} }
contract C is B { function f() public {} }
// ----
// TypeError: (72-94): Overriding function is missing 'override' specifier.
// TypeError: (72-94): Overriding function is missing "override" specifier.
// TypeError: (72-94): Overriding function changes state mutability from "payable" to "nonpayable".

View File

@ -1,5 +1,5 @@
contract B { function f() virtual public {} }
contract C is B { function f() payable public {} }
// ----
// TypeError: (64-94): Overriding function is missing 'override' specifier.
// TypeError: (64-94): Overriding function is missing "override" specifier.
// TypeError: (64-94): Overriding function changes state mutability from "nonpayable" to "payable".