Merge pull request #12523 from ethereum/immutable-ctor-fail-12379

Fix wrong error with immutables when base contract c'tor uses return

Changelog.md

@@ -11,6 +11,7 @@ Compiler Features:
 
 Bugfixes:
  * Control Flow Graph: Perform proper virtual lookup for modifiers for uninitialized variable and unreachable code analysis.
+ * Immutables: Fix wrong error when the constructor of a base contract uses ``return`` and the parent contract contains immutable variables.
 
 
 Solc-Js:

libsolidity/analysis/ImmutableValidator.cpp

@@ -29,7 +29,7 @@ void ImmutableValidator::analyze()
 {
 	m_inCreationContext = true;
 
-	auto linearizedContracts = m_currentContract.annotation().linearizedBaseContracts | ranges::views::reverse;
+	auto linearizedContracts = m_mostDerivedContract.annotation().linearizedBaseContracts | ranges::views::reverse;
 
 	for (ContractDefinition const* contract: linearizedContracts)
 		for (VariableDeclaration const* stateVar: contract->stateVariables())
@@ -62,7 +62,7 @@ void ImmutableValidator::analyze()
 			visitCallableIfNew(*modDef);
 	}
 
-	checkAllVariablesInitialized(m_currentContract.location());
+	checkAllVariablesInitialized(m_mostDerivedContract.location());
 }
 
 bool ImmutableValidator::visit(Assignment const& _assignment)
@@ -137,7 +137,7 @@ void ImmutableValidator::endVisit(IdentifierPath const& _identifierPath)
 	if (auto const callableDef = dynamic_cast<CallableDeclaration const*>(_identifierPath.annotation().referencedDeclaration))
 		visitCallableIfNew(
 			*_identifierPath.annotation().requiredLookup == VirtualLookup::Virtual ?
-			callableDef->resolveVirtual(m_currentContract) :
+			callableDef->resolveVirtual(m_mostDerivedContract) :
 			*callableDef
 		);
 
@@ -147,7 +147,7 @@ void ImmutableValidator::endVisit(IdentifierPath const& _identifierPath)
 void ImmutableValidator::endVisit(Identifier const& _identifier)
 {
 	if (auto const callableDef = dynamic_cast<CallableDeclaration const*>(_identifier.annotation().referencedDeclaration))
-		visitCallableIfNew(*_identifier.annotation().requiredLookup == VirtualLookup::Virtual ? callableDef->resolveVirtual(m_currentContract) : *callableDef);
+		visitCallableIfNew(*_identifier.annotation().requiredLookup == VirtualLookup::Virtual ? callableDef->resolveVirtual(m_mostDerivedContract) : *callableDef);
 	if (auto const varDecl = dynamic_cast<VariableDeclaration const*>(_identifier.annotation().referencedDeclaration))
 		analyseVariableReference(*varDecl, _identifier);
 }
@@ -160,15 +160,18 @@ void ImmutableValidator::endVisit(Return const& _return)
 
 bool ImmutableValidator::analyseCallable(CallableDeclaration const& _callableDeclaration)
 {
-	FunctionDefinition const* prevConstructor = m_currentConstructor;
+	ScopedSaveAndRestore constructorGuard{m_currentConstructor, {}};
-	m_currentConstructor = nullptr;
+	ScopedSaveAndRestore constructorContractGuard{m_currentConstructorContract, {}};
 
 	if (FunctionDefinition const* funcDef = dynamic_cast<decltype(funcDef)>(&_callableDeclaration))
 	{
 		ASTNode::listAccept(funcDef->modifiers(), *this);
 
 		if (funcDef->isConstructor())
+		{
+			m_currentConstructorContract = funcDef->annotation().contract;
 			m_currentConstructor = funcDef;
+		}
 
 		if (funcDef->isImplemented())
 			funcDef->body().accept(*this);
@@ -177,8 +180,6 @@ bool ImmutableValidator::analyseCallable(CallableDeclaration const& _callableDec
 		if (modDef->isImplemented())
 			modDef->body().accept(*this);
 
-	m_currentConstructor = prevConstructor;
-
 	return false;
 }
 
@@ -253,7 +254,8 @@ void ImmutableValidator::analyseVariableReference(VariableDeclaration const& _va
 
 void ImmutableValidator::checkAllVariablesInitialized(solidity::langutil::SourceLocation const& _location)
 {
-	for (ContractDefinition const* contract: m_currentContract.annotation().linearizedBaseContracts)
+	for (ContractDefinition const* contract: m_mostDerivedContract.annotation().linearizedBaseContracts | ranges::views::reverse)
+	{
 		for (VariableDeclaration const* varDecl: contract->stateVariables())
 			if (varDecl->immutable())
 				if (!util::contains(m_initializedStateVariables, varDecl))
@@ -263,6 +265,11 @@ void ImmutableValidator::checkAllVariablesInitialized(solidity::langutil::Source
 						solidity::langutil::SecondarySourceLocation().append("Not initialized: ", varDecl->location()),
 						"Construction control flow ends without initializing all immutable state variables."
 					);
+
+		// Don't check further than the current c'tors contract
+		if (contract == m_currentConstructorContract)
+			break;
+	}
 }
 
 void ImmutableValidator::visitCallableIfNew(Declaration const& _declaration)

libsolidity/analysis/ImmutableValidator.h

@@ -42,7 +42,7 @@ class ImmutableValidator: private ASTConstVisitor
 
 public:
 	ImmutableValidator(langutil::ErrorReporter& _errorReporter, ContractDefinition const& _contractDefinition):
-		m_currentContract(_contractDefinition),
+		m_mostDerivedContract(_contractDefinition),
 		m_errorReporter(_errorReporter)
 	{ }
 
@@ -66,7 +66,7 @@ private:
 
 	void visitCallableIfNew(Declaration const& _declaration);
 
-	ContractDefinition const& m_currentContract;
+	ContractDefinition const& m_mostDerivedContract;
 
 	CallableDeclarationSet m_visitedCallables;
 
@@ -74,6 +74,7 @@ private:
 	langutil::ErrorReporter& m_errorReporter;
 
 	FunctionDefinition const* m_currentConstructor = nullptr;
+	ContractDefinition const* m_currentConstructorContract = nullptr;
 	bool m_inLoop = false;
 	bool m_inBranch = false;
 	bool m_inCreationContext = true;

test/libsolidity/syntaxTests/immutable/base_ctor_return_no_immutables.sol

@@ -0,0 +1,10 @@
+contract Parent {
+    constructor() {
+        return;
+    }
+}
+
+contract Child is Parent {
+    uint public immutable baked = 123;
+}
+