diff --git a/Changelog.md b/Changelog.md index def68a275..1bbf51ff2 100644 --- a/Changelog.md +++ b/Changelog.md @@ -13,9 +13,10 @@ Compiler Features: Bugfixes: + * SMTChecker: Fix false negative caused by ``push`` on storage array references returned by internal functions. * SMTChecker: Fix false positive in external calls from constructors. * SMTChecker: Fix internal error on some multi-source uses of ``abi.*``, cryptographic functions and constants. - * SMTChecker: Fix false negative caused by ``push`` on storage array references returned by internal functions. + * Type Checker: Disallow modifier declarations and definitions in interfaces. diff --git a/docs/contracts/interfaces.rst b/docs/contracts/interfaces.rst index ae5a15e99..a4581a15f 100644 --- a/docs/contracts/interfaces.rst +++ b/docs/contracts/interfaces.rst @@ -6,12 +6,14 @@ Interfaces ********** -Interfaces are similar to abstract contracts, but they cannot have any functions implemented. There are further restrictions: +Interfaces are similar to abstract contracts, but they cannot have any functions implemented. +There are further restrictions: - They cannot inherit from other contracts, but they can inherit from other interfaces. - All declared functions must be external. - They cannot declare a constructor. - They cannot declare state variables. +- They cannot declare modifiers. Some of these restrictions might be lifted in the future. diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 2f545c7ef..f6e89886f 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -314,14 +314,22 @@ void TypeChecker::endVisit(InheritanceSpecifier const& _inheritance) void TypeChecker::endVisit(ModifierDefinition const& _modifier) { - if (_modifier.virtualSemantics()) - if (auto const* contractDef = dynamic_cast(_modifier.scope())) - if (contractDef->isLibrary()) - m_errorReporter.typeError( - 3275_error, - _modifier.location(), - "Modifiers in a library cannot be virtual." - ); + if (auto const* contractDef = dynamic_cast(_modifier.scope())) + { + if (_modifier.virtualSemantics() && contractDef->isLibrary()) + m_errorReporter.typeError( + 3275_error, + _modifier.location(), + "Modifiers in a library cannot be virtual." + ); + + if (contractDef->isInterface()) + m_errorReporter.typeError( + 6408_error, + _modifier.location(), + "Modifiers cannot be defined or declared in interfaces." + ); + } if (!_modifier.isImplemented() && !_modifier.virtualSemantics()) m_errorReporter.typeError(8063_error, _modifier.location(), "Modifiers without implementation must be marked virtual."); diff --git a/test/libsolidity/analysis/FunctionCallGraph.cpp b/test/libsolidity/analysis/FunctionCallGraph.cpp index af6eac2c4..30477aa2e 100644 --- a/test/libsolidity/analysis/FunctionCallGraph.cpp +++ b/test/libsolidity/analysis/FunctionCallGraph.cpp @@ -1114,7 +1114,6 @@ BOOST_AUTO_TEST_CASE(interfaces_and_abstract_contracts) unique_ptr compilerStack = parseAndAnalyzeContracts(R"( interface I { event Ev(uint); - modifier m() virtual; function ext1() external; function ext2() external; @@ -1126,6 +1125,8 @@ BOOST_AUTO_TEST_CASE(interfaces_and_abstract_contracts) } abstract contract C is J { + modifier m() virtual; + function ext3() external override virtual; function ext4() external { inr2();} function inr1() internal virtual; @@ -1167,7 +1168,7 @@ BOOST_AUTO_TEST_CASE(interfaces_and_abstract_contracts) {"Entry", "function C.ext4()"}, {"function C.ext4()", "function C.inr2()"}, {"function C.inr2()", "function C.inr1()"}, - {"function C.inr2()", "modifier I.m"}, + {"function C.inr2()", "modifier C.m"}, }}, {"D", { {"Entry", "function D.ext1()"}, diff --git a/test/libsolidity/syntaxTests/modifiers/definition_in_contract.sol b/test/libsolidity/syntaxTests/modifiers/definition_in_contract.sol new file mode 100644 index 000000000..9b0a805ad --- /dev/null +++ b/test/libsolidity/syntaxTests/modifiers/definition_in_contract.sol @@ -0,0 +1,11 @@ +contract C { + modifier m { _; } + modifier mv virtual { _; } +} + +abstract contract A { + modifier m { _; } + modifier mv virtual { _; } + modifier muv virtual; +} +// ---- diff --git a/test/libsolidity/syntaxTests/modifiers/definition_in_contract_unimplemented.sol b/test/libsolidity/syntaxTests/modifiers/definition_in_contract_unimplemented.sol new file mode 100644 index 000000000..2018ae4ea --- /dev/null +++ b/test/libsolidity/syntaxTests/modifiers/definition_in_contract_unimplemented.sol @@ -0,0 +1,7 @@ +contract C { + modifier mu; + modifier muv virtual; +} +// ---- +// TypeError 3656: (0-57): Contract "C" should be marked as abstract. +// TypeError 8063: (17-29): Modifiers without implementation must be marked virtual. diff --git a/test/libsolidity/syntaxTests/modifiers/definition_in_interface.sol b/test/libsolidity/syntaxTests/modifiers/definition_in_interface.sol new file mode 100644 index 000000000..2f8281bde --- /dev/null +++ b/test/libsolidity/syntaxTests/modifiers/definition_in_interface.sol @@ -0,0 +1,12 @@ +interface I { + modifier m { _; } + modifier mu; + modifier mv virtual { _; } + modifier muv virtual; +} +// ---- +// TypeError 6408: (18-35): Modifiers cannot be defined or declared in interfaces. +// TypeError 6408: (40-52): Modifiers cannot be defined or declared in interfaces. +// TypeError 8063: (40-52): Modifiers without implementation must be marked virtual. +// TypeError 6408: (57-83): Modifiers cannot be defined or declared in interfaces. +// TypeError 6408: (88-109): Modifiers cannot be defined or declared in interfaces. diff --git a/test/libsolidity/syntaxTests/modifiers/definition_in_library.sol b/test/libsolidity/syntaxTests/modifiers/definition_in_library.sol new file mode 100644 index 000000000..6acbf3a0f --- /dev/null +++ b/test/libsolidity/syntaxTests/modifiers/definition_in_library.sol @@ -0,0 +1,5 @@ +library L { + modifier mv virtual { _; } +} +// ---- +// TypeError 3275: (16-42): Modifiers in a library cannot be virtual. diff --git a/test/libsolidity/syntaxTests/modifiers/definition_in_library_unimplemented.sol b/test/libsolidity/syntaxTests/modifiers/definition_in_library_unimplemented.sol new file mode 100644 index 000000000..5fbc35682 --- /dev/null +++ b/test/libsolidity/syntaxTests/modifiers/definition_in_library_unimplemented.sol @@ -0,0 +1,7 @@ +library L { + modifier mu; + modifier muv virtual; +} +// ---- +// TypeError 8063: (16-28): Modifiers without implementation must be marked virtual. +// TypeError 3275: (33-54): Modifiers in a library cannot be virtual. diff --git a/test/libsolidity/syntaxTests/modifiers/definition_in_library_virtual.sol b/test/libsolidity/syntaxTests/modifiers/definition_in_library_virtual.sol new file mode 100644 index 000000000..173a94536 --- /dev/null +++ b/test/libsolidity/syntaxTests/modifiers/definition_in_library_virtual.sol @@ -0,0 +1,4 @@ +library L { + modifier m { _; } +} +// ---- diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/588_interface_function_modifier.sol b/test/libsolidity/syntaxTests/modifiers/use_on_interface_function.sol similarity index 67% rename from test/libsolidity/syntaxTests/nameAndTypeResolution/588_interface_function_modifier.sol rename to test/libsolidity/syntaxTests/modifiers/use_on_interface_function.sol index e506828d0..febdd9615 100644 --- a/test/libsolidity/syntaxTests/nameAndTypeResolution/588_interface_function_modifier.sol +++ b/test/libsolidity/syntaxTests/modifiers/use_on_interface_function.sol @@ -4,3 +4,4 @@ interface I { } // ---- // SyntaxError 5842: (16-60): Functions in interfaces cannot have modifiers. +// TypeError 6408: (63-82): Modifiers cannot be defined or declared in interfaces.