Clean up visibility via contract name and fix ICE on calling unimplemented base function.

Changelog.md

@@ -11,8 +11,11 @@ Compiler Features:
  * General: Support compiling starting from an imported AST. Among others, this can be used for mutation testing.
  * Yul Optimizer: Apply penalty when trying to rematerialize into loops.
 
+
 Bugfixes:
  * Commandline interface: Only activate yul optimizer if ``--optimize`` is given.
+ * Fixes internal compiler error on explicitly calling unimplemented base functions.
+
 
 Build System:
  * Switch to building soljson.js with an embedded base64-encoded wasm binary.

libsolidity/analysis/NameAndTypeResolver.cpp

@@ -625,7 +625,6 @@ bool DeclarationRegistrationHelper::visit(FunctionDefinition& _function)
 {
 	registerDeclaration(_function, true);
 	m_currentFunction = &_function;
-	_function.annotation().contract = m_currentContract;
 	return true;
 }
 
@@ -760,6 +759,7 @@ void DeclarationRegistrationHelper::registerDeclaration(Declaration& _declaratio
 	registerDeclaration(*m_scopes[m_currentScope], _declaration, nullptr, nullptr, warnAboutShadowing, inactive, m_errorReporter);
 
 	_declaration.annotation().scope = m_currentScope;
+	_declaration.annotation().contract = m_currentContract;
 	if (_opensScope)
 		enterNewSubScope(_declaration);
 }

libsolidity/analysis/TypeChecker.cpp

@@ -1695,11 +1695,20 @@ void TypeChecker::typeCheckFunctionCall(
 	{
 		m_errorReporter.typeError(
 			_functionCall.location(),
-			"Cannot call function via contract name."
+			"Cannot call function via contract type name."
 		);
 		return;
 	}
-
+	if (_functionType->kind() == FunctionType::Kind::Internal && _functionType->hasDeclaration())
+		if (auto const* functionDefinition = dynamic_cast<FunctionDefinition const*>(&_functionType->declaration()))
+			// functionDefinition->annotation().contract != m_scope ensures that this is a qualified access,
+			// e.g. ``A.f();`` instead of a simple function call like ``f();`` (the latter is valid for unimplemented
+			// functions).
+			if (functionDefinition->annotation().contract != m_scope && !functionDefinition->isImplemented())
+				m_errorReporter.typeError(
+					_functionCall.location(),
+					"Cannot call unimplemented base function."
+				);
 
 	// Check for unsupported use of bare static call
 	if (
@@ -2660,8 +2669,7 @@ bool TypeChecker::visit(Identifier const& _identifier)
 	);
 	annotation.isLValue = annotation.referencedDeclaration->isLValue();
 	annotation.type = annotation.referencedDeclaration->type();
-	if (!annotation.type)
-		m_errorReporter.fatalTypeError(_identifier.location(), "Declaration referenced before type could be determined.");
+	solAssert(annotation.type, "Declaration referenced before type could be determined.");
 	if (auto variableDeclaration = dynamic_cast<VariableDeclaration const*>(annotation.referencedDeclaration))
 		annotation.isPure = annotation.isConstant = variableDeclaration->isConstant();
 	else if (dynamic_cast<MagicVariableDeclaration const*>(annotation.referencedDeclaration))

libsolidity/ast/AST.cpp

@@ -91,6 +91,11 @@ vector<VariableDeclaration const*> ContractDefinition::stateVariablesIncludingIn
 	return stateVars;
 }
 
+bool ContractDefinition::derivesFrom(ContractDefinition const& _base) const
+{
+	return util::contains(annotation().linearizedBaseContracts, &_base);
+}
+
 map<util::FixedHash<4>, FunctionTypePointer> ContractDefinition::interfaceFunctions() const
 {
 	auto exportedFunctionList = interfaceFunctionList();
@@ -202,36 +207,6 @@ vector<pair<util::FixedHash<4>, FunctionTypePointer>> const& ContractDefinition:
 	return *m_interfaceFunctionList;
 }
 
-vector<Declaration const*> const& ContractDefinition::inheritableMembers() const
-{
-	if (!m_inheritableMembers)
-	{
-		m_inheritableMembers = make_unique<vector<Declaration const*>>();
-		auto addInheritableMember = [&](Declaration const* _decl)
-		{
-			solAssert(_decl, "addInheritableMember got a nullpointer.");
-			if (_decl->isVisibleInDerivedContracts())
-				m_inheritableMembers->push_back(_decl);
-		};
-
-		for (FunctionDefinition const* f: definedFunctions())
-			addInheritableMember(f);
-
-		for (VariableDeclaration const* v: stateVariables())
-			addInheritableMember(v);
-
-		for (StructDefinition const* s: definedStructs())
-			addInheritableMember(s);
-
-		for (EnumDefinition const* e: definedEnums())
-			addInheritableMember(e);
-
-		for (EventDefinition const* e: events())
-			addInheritableMember(e);
-	}
-	return *m_inheritableMembers;
-}
-
 TypePointer ContractDefinition::type() const
 {
 	return TypeProvider::typeType(TypeProvider::contract(*this));
@@ -322,6 +297,13 @@ TypePointer FunctionDefinition::type() const
 	return TypeProvider::function(*this, FunctionType::Kind::Internal);
 }
 
+TypePointer FunctionDefinition::typeViaContractName() const
+{
+	if (annotation().contract->isLibrary())
+		return FunctionType(*this).asCallableFunction(true);
+	return TypeProvider::function(*this, FunctionType::Kind::Declaration);
+}
+
 string FunctionDefinition::externalSignature() const
 {
 	return TypeProvider::function(*this)->externalSignature();

libsolidity/ast/AST.h

@@ -217,19 +217,22 @@ public:
 	Visibility visibility() const { return m_visibility == Visibility::Default ? defaultVisibility() : m_visibility; }
 	bool isPublic() const { return visibility() >= Visibility::Public; }
 	virtual bool isVisibleInContract() const { return visibility() != Visibility::External; }
-	bool isVisibleInDerivedContracts() const { return isVisibleInContract() && visibility() >= Visibility::Internal; }
+	virtual bool isVisibleInDerivedContracts() const { return isVisibleInContract() && visibility() >= Visibility::Internal; }
 	bool isVisibleAsLibraryMember() const { return visibility() >= Visibility::Internal; }
+	virtual bool isVisibleViaContractTypeAccess() const { return false; }
 
 
 	virtual bool isLValue() const { return false; }
 	virtual bool isPartOfExternalInterface() const { return false; }
 
 	/// @returns the type of expressions referencing this declaration.
-	/// The current contract has to be given since this context can change the type, especially of
-	/// contract types.
 	/// This can only be called once types of variable declarations have already been resolved.
 	virtual TypePointer type() const = 0;
 
+	/// @returns the type for members of the containing contract type that refer to this declaration.
+	/// This can only be called once types of variable declarations have already been resolved.
+	virtual TypePointer typeViaContractName() const { return type(); }
+
 	/// @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 FunctionTypePointer functionType(bool /*_internal*/) const { return {}; }
@@ -420,13 +423,16 @@ public:
 	bool isInterface() const { return m_contractKind == ContractKind::Interface; }
 	bool isLibrary() const { return m_contractKind == ContractKind::Library; }
 
+	/// @returns true, if the contract derives from @arg _base.
+	bool derivesFrom(ContractDefinition const& _base) const;
+
 	/// @returns a map of canonical function signatures to FunctionDefinitions
 	/// as intended for use by the ABI.
 	std::map<util::FixedHash<4>, FunctionTypePointer> interfaceFunctions() const;
 	std::vector<std::pair<util::FixedHash<4>, FunctionTypePointer>> const& interfaceFunctionList() const;
 
-	/// @returns a list of the inheritable members of this contract
-	std::vector<Declaration const*> const& inheritableMembers() const;
+	/// @returns a list of all declarations in this contract
+	std::vector<Declaration const*> declarations() const { return filteredNodes<Declaration>(m_subNodes); }
 
 	/// Returns the constructor or nullptr if no constructor was specified.
 	FunctionDefinition const* constructor() const;
@@ -460,7 +466,6 @@ private:
 
 	mutable std::unique_ptr<std::vector<std::pair<util::FixedHash<4>, FunctionTypePointer>>> m_interfaceFunctionList;
 	mutable std::unique_ptr<std::vector<EventDefinition const*>> m_interfaceEvents;
-	mutable std::unique_ptr<std::vector<Declaration const*>> m_inheritableMembers;
 };
 
 class InheritanceSpecifier: public ASTNode
@@ -534,6 +539,9 @@ public:
 
 	TypePointer type() const override;
 
+	bool isVisibleInDerivedContracts() const override { return true; }
+	bool isVisibleViaContractTypeAccess() const override { return true; }
+
 	TypeDeclarationAnnotation& annotation() const override;
 
 private:
@@ -553,6 +561,9 @@ public:
 	void accept(ASTVisitor& _visitor) override;
 	void accept(ASTConstVisitor& _visitor) const override;
 
+	bool isVisibleInDerivedContracts() const override { return true; }
+	bool isVisibleViaContractTypeAccess() const override { return true; }
+
 	std::vector<ASTPointer<EnumValue>> const& members() const { return m_members; }
 
 	TypePointer type() const override;
@@ -715,6 +726,10 @@ public:
 	{
 		return Declaration::isVisibleInContract() && isOrdinary();
 	}
+	bool isVisibleViaContractTypeAccess() const override
+	{
+		return visibility() >= Visibility::Public;
+	}
 	bool isPartOfExternalInterface() const override { return isPublic() && isOrdinary(); }
 
 	/// @returns the external signature of the function
@@ -728,6 +743,7 @@ public:
 	ContractKind inContractKind() const;
 
 	TypePointer type() const override;
+	TypePointer typeViaContractName() 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.
@@ -880,6 +896,8 @@ public:
 
 	TypePointer type() const override;
 
+	Visibility defaultVisibility() const override { return Visibility::Internal; }
+
 	ModifierDefinitionAnnotation& annotation() const override;
 
 private:
@@ -942,6 +960,9 @@ public:
 	TypePointer type() const override;
 	FunctionTypePointer functionType(bool /*_internal*/) const override;
 
+	bool isVisibleInDerivedContracts() const override { return true; }
+	bool isVisibleViaContractTypeAccess() const override { return false; /* TODO */ }
+
 	EventDefinitionAnnotation& annotation() const override;
 
 private:

libsolidity/ast/ASTAnnotations.h

@@ -78,6 +78,9 @@ struct ScopableAnnotation
 	/// The scope this declaration resides in. Can be nullptr if it is the global scope.
 	/// Available only after name and type resolution step.
 	ASTNode const* scope = nullptr;
+	/// Pointer to the contract this declaration resides in. Can be nullptr if the current scope
+	/// is not part of a contract. Available only after name and type resolution step.
+	ContractDefinition const* contract = nullptr;
 };
 
 struct DeclarationAnnotation: ASTAnnotation, ScopableAnnotation
@@ -121,8 +124,6 @@ struct CallableDeclarationAnnotation: DeclarationAnnotation
 
 struct FunctionDefinitionAnnotation: CallableDeclarationAnnotation, DocumentedAnnotation
 {
-	/// Pointer to the contract this function is defined in
-	ContractDefinition const* contract = nullptr;
 };
 
 struct EventDefinitionAnnotation: CallableDeclarationAnnotation, DocumentedAnnotation

libsolidity/ast/Types.cpp

@@ -2957,12 +2957,30 @@ FunctionTypePointer FunctionType::interfaceFunctionType() const
 	);
 }
 
-MemberList::MemberMap FunctionType::nativeMembers(ContractDefinition const*) const
+MemberList::MemberMap FunctionType::nativeMembers(ContractDefinition const* _scope) const
 {
 	switch (m_kind)
 	{
 	case Kind::Declaration:
-		return {{"selector", TypeProvider::fixedBytes(4)}};
+		if (declaration().isPartOfExternalInterface())
+			return {{"selector", TypeProvider::fixedBytes(4)}};
+		else
+			return MemberList::MemberMap();
+	case Kind::Internal:
+		if (
+			auto const* functionDefinition = dynamic_cast<FunctionDefinition const*>(m_declaration);
+			functionDefinition &&
+			_scope &&
+			functionDefinition->annotation().contract &&
+			_scope != functionDefinition->annotation().contract &&
+			functionDefinition->isPartOfExternalInterface()
+		)
+		{
+			solAssert(_scope->derivesFrom(*functionDefinition->annotation().contract), "");
+			return {{"selector", TypeProvider::fixedBytes(4)}};
+		}
+		else
+			return MemberList::MemberMap();
 	case Kind::External:
 	case Kind::Creation:
 	case Kind::BareCall:
@@ -3406,36 +3424,20 @@ MemberList::MemberMap TypeType::nativeMembers(ContractDefinition const* _current
 	if (m_actualType->category() == Category::Contract)
 	{
 		ContractDefinition const& contract = dynamic_cast<ContractType const&>(*m_actualType).contractDefinition();
-		bool isBase = false;
-		if (_currentScope != nullptr)
+		bool inDerivingScope = _currentScope && _currentScope->derivesFrom(contract);
+
+		for (auto const* declaration: contract.declarations())
 		{
-			auto const& currentBases = _currentScope->annotation().linearizedBaseContracts;
-			isBase = (find(currentBases.begin(), currentBases.end(), &contract) != currentBases.end());
-		}
-		if (isBase)
-		{
-			// We are accessing the type of a base contract, so add all public and protected
-			// members. Note that this does not add inherited functions on purpose.
-			for (Declaration const* decl: contract.inheritableMembers())
-				members.emplace_back(decl->name(), decl->type(), decl);
-		}
-		else
-		{
-			bool inLibrary = contract.isLibrary();
-			for (FunctionDefinition const* function: contract.definedFunctions())
-				if (
-					(inLibrary && function->isVisibleAsLibraryMember()) ||
-					(!inLibrary && function->isPartOfExternalInterface())
-				)
-					members.emplace_back(
-						function->name(),
-						FunctionType(*function).asCallableFunction(inLibrary),
-						function
-					);
-			for (auto const& stru: contract.definedStructs())
-				members.emplace_back(stru->name(), stru->type(), stru);
-			for (auto const& enu: contract.definedEnums())
-				members.emplace_back(enu->name(), enu->type(), enu);
+			if (dynamic_cast<ModifierDefinition const*>(declaration))
+				continue;
+
+			if (!contract.isLibrary() && inDerivingScope && declaration->isVisibleInDerivedContracts())
+				members.emplace_back(declaration->name(), declaration->type(), declaration);
+			else if (
+				(contract.isLibrary() && declaration->isVisibleAsLibraryMember()) ||
+				declaration->isVisibleViaContractTypeAccess()
+			)
+				members.emplace_back(declaration->name(), declaration->typeViaContractName(), declaration);
 		}
 	}
 	else if (m_actualType->category() == Category::Enum)

libsolidity/codegen/ExpressionCompiler.cpp

@@ -1300,7 +1300,7 @@ bool ExpressionCompiler::visit(MemberAccess const& _memberAccess)
 		functionType && member == "selector"
 	)
 	{
-		if (functionType->kind() == FunctionType::Kind::Declaration)
+		if (functionType->hasDeclaration())
 		{
 			m_context << functionType->externalIdentifier();
 			/// need to store it as bytes4

test/libsolidity/syntaxTests/nameAndTypeResolution/145_external_base_visibility.sol

@@ -5,4 +5,4 @@ contract derived is base {
     function g() public { base.f(); }
 }
 // ----
-// TypeError: (100-106): Member "f" not found or not visible after argument-dependent lookup in type(contract base).
+// TypeError: (100-108): Cannot call function via contract type name.

test/libsolidity/syntaxTests/types/call_unimplemented_internal_function.sol

@@ -0,0 +1,6 @@
+abstract contract A {
+    function f() public virtual;
+    function g() public {
+        f();
+    }
+}

test/libsolidity/syntaxTests/types/contractTypeType/members/assign_function_via_base_name_to_var.sol

@@ -0,0 +1,14 @@
+contract A {
+    function f() external {}
+    function g() external pure {}
+}
+
+contract B is A {
+    function h() external {
+        function() external f = A.f;
+        function() external pure g = A.g;
+    }
+}
+// ----
+// TypeError: (133-160): Type function A.f() is not implicitly convertible to expected type function () external.
+// TypeError: (170-202): Type function A.g() pure is not implicitly convertible to expected type function () pure external.

test/libsolidity/syntaxTests/types/contractTypeType/members/base_contract.sol

@@ -0,0 +1,11 @@
+contract B {
+    function f() external {}
+    function g() public {}
+}
+contract C is B {
+    function h() public {
+        B.f.selector;
+        B.g.selector;
+        B.g();
+    }
+}

test/libsolidity/syntaxTests/types/contractTypeType/members/base_contract_invalid.sol

@@ -0,0 +1,13 @@
+contract B {
+    function f() external {}
+    function g() internal {}
+}
+contract C is B {
+    function i() public {
+        B.f();
+        B.g.selector;
+    }
+}
+// ----
+// TypeError: (125-130): Cannot call function via contract type name.
+// TypeError: (140-152): Member "selector" not found or not visible after argument-dependent lookup in function ().

test/libsolidity/syntaxTests/types/contractTypeType/members/call_function_via_contract_name.sol

@@ -12,6 +12,6 @@ contract B {
     }
 }
 // ----
-// TypeError: (160-165): Cannot call function via contract name.
-// TypeError: (175-180): Cannot call function via contract name.
-// TypeError: (190-195): Cannot call function via contract name.
+// TypeError: (160-165): Cannot call function via contract type name.
+// TypeError: (175-180): Cannot call function via contract type name.
+// TypeError: (190-195): Cannot call function via contract type name.

test/libsolidity/syntaxTests/types/contractTypeType/members/call_unimplemented_base.sol

@@ -0,0 +1,10 @@
+abstract contract B {
+    function f() public virtual;
+}
+contract C is B {
+    function f() public override {
+        B.f();
+    }
+}
+// ----
+// TypeError: (118-123): Cannot call unimplemented base function.

test/libsolidity/syntaxTests/types/contractTypeType/members/modifier.sol

@@ -0,0 +1,10 @@
+contract A {
+    modifier mod() { _; }
+}
+contract B {
+    function f() public {
+        A.mod;
+    }
+}
+// ----
+// TypeError: (88-93): Member "mod" not found or not visible after argument-dependent lookup in type(contract A).

test/libsolidity/syntaxTests/types/contractTypeType/members/modifier_base.sol

@@ -0,0 +1,10 @@
+contract A {
+    modifier mod() { _; }
+}
+contract B is A {
+    function f() public {
+        A.mod;
+    }
+}
+// ----
+// TypeError: (93-98): Member "mod" not found or not visible after argument-dependent lookup in type(contract A).

test/libsolidity/syntaxTests/types/contractTypeType/members/struct_enum.sol

@@ -0,0 +1,14 @@
+contract A {
+    struct S { uint256 a; }
+    enum E { V }
+}
+contract B {
+    A.S x;
+    A.E e;
+}
+contract C is A {
+    A.S x;
+    S y;
+    A.E e;
+    E f;
+}

test/libsolidity/syntaxTests/types/library_internal_call.sol

@@ -0,0 +1,4 @@
+library L {
+    function a() public pure {}
+    function b() public pure { a(); }
+}