Merge pull request #10307 from ethereum/fixQualifiedModifierLookup

Fix qualified modifier lookup.

docs/contracts/function-modifiers.rst

@@ -100,6 +100,11 @@ if they are marked ``virtual``. For details, please see
         }
     }
 
+If you want to access a modifier ``m`` defined in a contract ``C``, you can use ``C.m`` to
+reference it without virtual lookup. It is only possible to use modifiers defined in the current
+contract or its base contracts. Modifiers can also be defined in libraries but their use is
+limited to functions of the same library.
+
 Multiple modifiers are applied to a function by specifying them in a
 whitespace-separated list and are evaluated in the order presented.
 

docs/grammar/Solidity.g4

@@ -72,7 +72,7 @@ inheritanceSpecifierList:
  * Inheritance specifier for contracts and interfaces.
  * Can optionally supply base constructor arguments.
  */
-inheritanceSpecifier: name=userDefinedTypeName arguments=callArgumentList?;
+inheritanceSpecifier: name=identifierPath arguments=callArgumentList?;
 
 /**
  * Declarations that can be used in contracts, interfaces and libraries.
@@ -98,15 +98,15 @@ namedArgument: name=identifier Colon value=expression;
  */
 callArgumentList: LParen ((expression (Comma expression)*)? | LBrace (namedArgument (Comma namedArgument)*)? RBrace) RParen;
 /**
- * Qualified name of a user defined type.
+ * Qualified name.
  */
-userDefinedTypeName: identifier (Period identifier)*;
+identifierPath: identifier (Period identifier)*;
 
 /**
  * Call to a modifier. If the modifier takes no arguments, the argument list can be skipped entirely
  * (including opening and closing parentheses).
  */
-modifierInvocation: identifier callArgumentList?;
+modifierInvocation: identifierPath callArgumentList?;
 /**
  * Visibility for functions and function types.
  */
@@ -144,7 +144,7 @@ stateMutability: Pure | View | Payable;
  * In cases where there are ambiguous declarations in several base contracts being overridden,
  * a complete list of base contracts has to be given.
  */
-overrideSpecifier: Override (LParen overrides+=userDefinedTypeName (Comma overrides+=userDefinedTypeName)* RParen)?;
+overrideSpecifier: Override (LParen overrides+=identifierPath (Comma overrides+=identifierPath)* RParen)?;
 /**
  * The definition of contract, library and interface functions.
  * Depending on the context in which the function is defined, further restrictions may apply,
@@ -269,12 +269,12 @@ eventDefinition:
  * Using directive to bind library functions to types.
  * Can occur within contracts and libraries.
  */
-usingDirective: Using userDefinedTypeName For (Mul | typeName) Semicolon;
+usingDirective: Using identifierPath For (Mul | typeName) Semicolon;
 /**
  * A type name can be an elementary type, a function type, a mapping type, a user-defined type
  * (e.g. a contract or struct) or an array type.
  */
-typeName: elementaryTypeName[true] | functionTypeName | mappingType | userDefinedTypeName | typeName LBrack expression? RBrack;
+typeName: elementaryTypeName[true] | functionTypeName | mappingType | identifierPath | typeName LBrack expression? RBrack;
 elementaryTypeName[boolean allowAddressPayable]: Address | {$allowAddressPayable}? Address Payable | Bool | String | Bytes | SignedIntegerType | UnsignedIntegerType | FixedBytes | Fixed | Ufixed;
 functionTypeName
 locals [boolean visibilitySet = false, boolean mutabilitySet = false]
@@ -329,7 +329,6 @@ expression:
 		identifier
 		| literal
 		| elementaryTypeName[false]
-		| userDefinedTypeName
 	  ) # PrimaryExpression
 ;
 
@@ -452,7 +451,7 @@ mappingType: Mapping LParen key=mappingKeyType DoubleArrow value=typeName RParen
 /**
  * Only elementary types or user defined types are viable as mapping keys.
  */
-mappingKeyType: elementaryTypeName[false] | userDefinedTypeName;
+mappingKeyType: elementaryTypeName[false] | identifierPath;
 
 /**
  * A Yul statement within an inline assembly block.

libsolidity/analysis/TypeChecker.cpp

@@ -630,7 +630,19 @@ void TypeChecker::visitManually(
 	vector<ASTPointer<VariableDeclaration>> emptyParameterList;
 	vector<ASTPointer<VariableDeclaration>> const* parameters = nullptr;
 	if (auto modifierDecl = dynamic_cast<ModifierDefinition const*>(declaration))
+	{
 		parameters = &modifierDecl->parameters();
+		if (auto const* modifierContract = dynamic_cast<ContractDefinition const*>(modifierDecl->scope()))
+			if (m_currentContract)
+			{
+				if (!contains(m_currentContract->annotation().linearizedBaseContracts, modifierContract))
+					m_errorReporter.typeError(
+						9428_error,
+						_modifier.location(),
+						"Can only use modifiers defined in the current contract or in base contracts."
+					);
+			}
+	}
 	else
 		// check parameters for Base constructors
 		for (ContractDefinition const* base: _bases)

libsolidity/codegen/ContractCompiler.cpp

@@ -1301,11 +1301,16 @@ void ContractCompiler::appendModifierOrFunctionCode()
 			appendModifierOrFunctionCode();
 		else
 		{
-			solAssert(*modifierInvocation->name().annotation().requiredLookup == VirtualLookup::Virtual, "");
-
-			ModifierDefinition const& modifier = dynamic_cast<ModifierDefinition const&>(
+			ModifierDefinition const& referencedModifier = dynamic_cast<ModifierDefinition const&>(
 				*modifierInvocation->name().annotation().referencedDeclaration
-			).resolveVirtual(m_context.mostDerivedContract());
+			);
+			VirtualLookup lookup = *modifierInvocation->name().annotation().requiredLookup;
+			solAssert(lookup == VirtualLookup::Virtual || lookup == VirtualLookup::Static, "");
+			ModifierDefinition const& modifier =
+				lookup == VirtualLookup::Virtual ?
+				referencedModifier.resolveVirtual(m_context.mostDerivedContract()) :
+				referencedModifier;
+
 			CompilerContext::LocationSetter locationSetter(m_context, modifier);
 			std::vector<ASTPointer<Expression>> const& modifierArguments =
 				modifierInvocation->arguments() ? *modifierInvocation->arguments() : std::vector<ASTPointer<Expression>>();

test/libsolidity/semanticTests/modifiers/access_through_contract_name.sol

@@ -0,0 +1,22 @@
+contract A {
+    uint public x = 7;
+    modifier m virtual { x = 2; _; }
+}
+contract C is A {
+    modifier m override { x = 1; _; }
+
+    function f() public A.m returns (uint) {
+        return 9;
+    }
+    function g() public m returns (uint) {
+        return 10;
+    }
+}
+// ----
+// x() -> 7
+// f() -> 9
+// x() -> 2
+// g() -> 0x0a
+// x() -> 1
+// f() -> 9
+// x() -> 2

test/libsolidity/semanticTests/modifiers/access_through_module_name.sol

@@ -0,0 +1,25 @@
+==== Source: a ====
+import "a" as M;
+contract C {
+    uint public x;
+    modifier m { x = 1; _; }
+
+    function f() public M.M.C.m returns (uint t, uint r) {
+        t = x;
+        x = 3;
+        r = 9;
+    }
+    function g() public m returns (uint t, uint r) {
+        t = x;
+        x = 4;
+        r = 10;
+    }
+}
+// ----
+// x() -> 0x00
+// f() -> 1, 9
+// x() -> 3
+// g() -> 1, 0x0a
+// x() -> 4
+// f() -> 1, 9
+// x() -> 3

test/libsolidity/syntaxTests/modifiers/access_in_library.sol

@@ -0,0 +1,9 @@
+library L {
+    modifier m() { _; }
+}
+contract C {
+    function f() L.m public {
+    }
+}
+// ----
+// TypeError 9428: (68-71): Can only use modifiers defined in the current contract or in base contracts.

test/libsolidity/syntaxTests/modifiers/cross_contract_access.sol

@@ -0,0 +1,9 @@
+contract C {
+    modifier m() { _; }
+}
+contract D {
+    function f() C.m public {
+    }
+}
+// ----
+// TypeError 9428: (69-72): Can only use modifiers defined in the current contract or in base contracts.

test/libsolidity/syntaxTests/modifiers/cross_contract_base.sol

@@ -0,0 +1,8 @@
+contract C {
+    modifier m() { _; }
+}
+contract D is C {
+    function f() C.m public {
+    }
+}
+// ----

test/libsolidity/syntaxTests/modifiers/cross_contract_unrelated.sol

@@ -0,0 +1,11 @@
+contract A {}
+contract C is A {
+    modifier m() { _; }
+}
+contract D is A {
+    function f() C.m public {
+    }
+}
+contract T is D, C {}
+// ----
+// TypeError 9428: (93-96): Can only use modifiers defined in the current contract or in base contracts.

test/libsolidity/syntaxTests/modifiers/library_via_using.sol

@@ -0,0 +1,10 @@
+library L {
+    modifier m() { _; }
+}
+contract C {
+    using L for *;
+    function f() L.m public {
+    }
+}
+// ----
+// TypeError 9428: (87-90): Can only use modifiers defined in the current contract or in base contracts.