Merge pull request #11750 from ethereum/immutables-10463

Allow reading of immutables during construction time

Author: chriseth

Changelog.md

@@ -4,6 +4,7 @@ Language Features:
 
 
 Compiler Features:
+ * Immutable variables can be read at construction time once they are initialized.
 
 
 Bugfixes:

docs/contracts/constant-state-variables.rst

@@ -76,11 +76,23 @@ Immutable
 Variables declared as ``immutable`` are a bit less restricted than those
 declared as ``constant``: Immutable variables can be assigned an arbitrary
 value in the constructor of the contract or at the point of their declaration.
-They cannot be read during construction time and can only be assigned once.
+They can be assigned only once and can, from that point on, be read even during
+construction time.
 
 The contract creation code generated by the compiler will modify the
 contract's runtime code before it is returned by replacing all references
 to immutables by the values assigned to the them. This is important if
 you are comparing the
 runtime code generated by the compiler with the one actually stored in the
 blockchain.
+
+.. note::
+  Immutables that are assigned at their declaration are only considered
+  initialized once the constructor of the contract is executing.
+  This means you cannot initialize immutables inline with a value
+  that depends on another immutable. You can do this, however,
+  inside the constructor of the contract.
+
+  This is a safeguard against different interpretations about the order
+  of state variable initialization and constructor execution, especially
+  with regards to inheritance.

libsolidity/analysis/ImmutableValidator.cpp

@@ -27,30 +27,31 @@ using namespace solidity::langutil;
 
 void ImmutableValidator::analyze()
 {
-	m_inConstructionContext = true;
+	m_inCreationContext = true;
 
 	auto linearizedContracts = m_currentContract.annotation().linearizedBaseContracts | ranges::views::reverse;
 
 	for (ContractDefinition const* contract: linearizedContracts)
 		for (VariableDeclaration const* stateVar: contract->stateVariables())
 			if (stateVar->value())
-				m_initializedStateVariables.emplace(stateVar);
+				stateVar->value()->accept(*this);
 
 	for (ContractDefinition const* contract: linearizedContracts)
+		for (std::shared_ptr<InheritanceSpecifier> const& inheritSpec: contract->baseContracts())
+			if (auto args = inheritSpec->arguments())
+				ASTNode::listAccept(*args, *this);
+
+	for (ContractDefinition const* contract: linearizedContracts)
+	{
 		for (VariableDeclaration const* stateVar: contract->stateVariables())
 			if (stateVar->value())
-				stateVar->value()->accept(*this);
+				m_initializedStateVariables.emplace(stateVar);
 
-	for (ContractDefinition const* contract: linearizedContracts)
 		if (contract->constructor())
 			visitCallableIfNew(*contract->constructor());
+	}
 
-	for (ContractDefinition const* contract: linearizedContracts)
-		for (std::shared_ptr<InheritanceSpecifier> const& inheritSpec: contract->baseContracts())
-			if (auto args = inheritSpec->arguments())
-				ASTNode::listAccept(*args, *this);
-
-	m_inConstructionContext = false;
+	m_inCreationContext = false;
 
 	for (ContractDefinition const* contract: linearizedContracts)
 	{
@@ -64,6 +65,15 @@ void ImmutableValidator::analyze()
 	checkAllVariablesInitialized(m_currentContract.location());
 }
 
+bool ImmutableValidator::visit(Assignment const& _assignment)
+{
+	// Need to visit values first (rhs) as they might access other immutables.
+	_assignment.rightHandSide().accept(*this);
+	_assignment.leftHandSide().accept(*this);
+	return false;
+}
+
+
 bool ImmutableValidator::visit(FunctionDefinition const& _functionDefinition)
 {
 	return analyseCallable(_functionDefinition);
@@ -207,19 +217,37 @@ void ImmutableValidator::analyseVariableReference(VariableDeclaration const& _va
 				"Cannot write to immutable here: Immutable variables cannot be initialized inside an if statement."
 			);
 		else if (m_initializedStateVariables.count(&_variableReference))
+		{
+			if (!read)
+				m_errorReporter.typeError(
+					1574_error,
+					_expression.location(),
+					"Immutable state variable already initialized."
+				);
+			else
+				m_errorReporter.typeError(
+					2718_error,
+					_expression.location(),
+					"Immutable variables cannot be modified after initialization."
+				);
+		}
+		else if (read)
 			m_errorReporter.typeError(
-				1574_error,
+				3969_error,
 				_expression.location(),
-				"Immutable state variable already initialized."
+				"Immutable variables must be initialized using an assignment."
 			);
 		m_initializedStateVariables.emplace(&_variableReference);
 	}
-	if (read && m_inConstructionContext)
+	if (
+		read &&
+		m_inCreationContext &&
+		!m_initializedStateVariables.count(&_variableReference)
+	)
 		m_errorReporter.typeError(
 			7733_error,
 			_expression.location(),
-			"Immutable variables cannot be read during contract creation time, which means "
-			"they cannot be read in the constructor or any function or modifier called from it."
+			"Immutable variables cannot be read before they are initialized."
 		);
 }
 

libsolidity/analysis/ImmutableValidator.h

@@ -28,8 +28,9 @@ namespace solidity::frontend
 
 /**
  * Validates access and initialization of immutable variables:
- * must be directly initialized in their respective c'tor
- * can not be read by any function/modifier called by the c'tor (or the c'tor itself)
+ * must be directly initialized in their respective c'tor or inline
+ * cannot be read before being initialized
+ * cannot be read when initializing state variables inline
  * must be initialized outside loops (only one initialization)
  * must be initialized outside ifs (must be initialized unconditionally)
  * must be initialized exactly once (no multiple statements)
@@ -48,6 +49,7 @@ class ImmutableValidator: private ASTConstVisitor
 	void analyze();
 
 private:
+	bool visit(Assignment const& _assignment);
 	bool visit(FunctionDefinition const& _functionDefinition);
 	bool visit(ModifierDefinition const& _modifierDefinition);
 	bool visit(MemberAccess const& _memberAccess);
@@ -74,7 +76,7 @@ class ImmutableValidator: private ASTConstVisitor
 	FunctionDefinition const* m_currentConstructor = nullptr;
 	bool m_inLoop = false;
 	bool m_inBranch = false;
-	bool m_inConstructionContext = false;
+	bool m_inCreationContext = true;
 };
 
 }

libsolidity/codegen/LValue.cpp

@@ -155,9 +155,15 @@ ImmutableItem::ImmutableItem(CompilerContext& _compilerContext, VariableDeclarat
 void ImmutableItem::retrieveValue(SourceLocation const&, bool) const
 {
 	solUnimplementedAssert(m_dataType->isValueType(), "");
-	solAssert(!m_context.runtimeContext(), "Tried to read immutable at construction time.");
-	for (auto&& slotName: m_context.immutableVariableSlotNames(m_variable))
-		m_context.appendImmutable(slotName);
+
+	if (m_context.runtimeContext())
+		CompilerUtils(m_context).loadFromMemory(
+			static_cast<unsigned>(m_context.immutableMemoryOffset(m_variable)),
+			*m_dataType
+		);
+	else
+		for (auto&& slotName: m_context.immutableVariableSlotNames(m_variable))
+			m_context.appendImmutable(slotName);
 }
 
 void ImmutableItem::storeValue(Type const& _sourceType, SourceLocation const&, bool _move) const

libsolidity/codegen/ir/IRGenerationContext.h

@@ -65,13 +65,17 @@ using InternalDispatchMap = std::map<YulArity, DispatchSet>;
 class IRGenerationContext
 {
 public:
+	enum class ExecutionContext { Creation, Deployed };
+
 	IRGenerationContext(
 		langutil::EVMVersion _evmVersion,
+		ExecutionContext _executionContext,
 		RevertStrings _revertStrings,
 		OptimiserSettings _optimiserSettings,
 		std::map<std::string, unsigned> _sourceIndices
 	):
 		m_evmVersion(_evmVersion),
+		m_executionContext(_executionContext),
 		m_revertStrings(_revertStrings),
 		m_optimiserSettings(std::move(_optimiserSettings)),
 		m_sourceIndices(std::move(_sourceIndices))
@@ -139,6 +143,7 @@ class IRGenerationContext
 	YulUtilFunctions utils();
 
 	langutil::EVMVersion evmVersion() const { return m_evmVersion; }
+	ExecutionContext executionContext() const { return m_executionContext; }
 
 	void setArithmetic(Arithmetic _value) { m_arithmetic = _value; }
 	Arithmetic arithmetic() const { return m_arithmetic; }
@@ -162,8 +167,11 @@ class IRGenerationContext
 
 	std::map<std::string, unsigned> const& sourceIndices() const { return m_sourceIndices; }
 
+	bool immutableRegistered(VariableDeclaration const& _varDecl) const { return m_immutableVariables.count(&_varDecl); }
+
 private:
 	langutil::EVMVersion m_evmVersion;
+	ExecutionContext m_executionContext;
 	RevertStrings m_revertStrings;
 	OptimiserSettings m_optimiserSettings;
 	std::map<std::string, unsigned> m_sourceIndices;

libsolidity/codegen/ir/IRGenerator.cpp

@@ -164,7 +164,7 @@ string IRGenerator::generate(
 		}
 	)");
 
-	resetContext(_contract);
+	resetContext(_contract, ExecutionContext::Creation);
 	for (VariableDeclaration const* var: ContractType(_contract).immutableVariables())
 		m_context.registerImmutableVariable(*var);
 
@@ -211,7 +211,7 @@ string IRGenerator::generate(
 	bool creationInvolvesAssembly = m_context.inlineAssemblySeen();
 	t("memoryInitCreation", memoryInit(!creationInvolvesAssembly));
 
-	resetContext(_contract);
+	resetContext(_contract, ExecutionContext::Deployed);
 
 	// NOTE: Function pointers can be passed from creation code via storage variables. We need to
 	// get all the functions they could point to into the dispatch functions even if they're never
@@ -1049,7 +1049,7 @@ string IRGenerator::memoryInit(bool _useMemoryGuard)
 		).render();
 }
 
-void IRGenerator::resetContext(ContractDefinition const& _contract)
+void IRGenerator::resetContext(ContractDefinition const& _contract, ExecutionContext _context)
 {
 	solAssert(
 		m_context.functionGenerationQueueEmpty(),
@@ -1063,7 +1063,7 @@ void IRGenerator::resetContext(ContractDefinition const& _contract)
 		m_context.internalDispatchClean(),
 		"Reset internal dispatch map without consuming it."
 	);
-	IRGenerationContext newContext(m_evmVersion, m_context.revertStrings(), m_optimiserSettings, m_context.sourceIndices());
+	IRGenerationContext newContext(m_evmVersion, _context, m_context.revertStrings(), m_optimiserSettings, m_context.sourceIndices());
 	newContext.copyFunctionIDsFrom(m_context);
 	m_context = move(newContext);
 

libsolidity/codegen/ir/IRGenerator.h

@@ -39,6 +39,8 @@ class SourceUnit;
 class IRGenerator
 {
 public:
+	using ExecutionContext = IRGenerationContext::ExecutionContext;
+
 	IRGenerator(
 		langutil::EVMVersion _evmVersion,
 		RevertStrings _revertStrings,
@@ -47,7 +49,7 @@ class IRGenerator
 	):
 		m_evmVersion(_evmVersion),
 		m_optimiserSettings(_optimiserSettings),
-		m_context(_evmVersion, _revertStrings, std::move(_optimiserSettings), std::move(_sourceIndices)),
+		m_context(_evmVersion, ExecutionContext::Creation, _revertStrings, std::move(_optimiserSettings), std::move(_sourceIndices)),
 		m_utils(_evmVersion, m_context.revertStrings(), m_context.functionCollector())
 	{}
 
@@ -115,7 +117,7 @@ class IRGenerator
 	/// to perform memory optimizations.
 	std::string memoryInit(bool _useMemoryGuard);
 
-	void resetContext(ContractDefinition const& _contract);
+	void resetContext(ContractDefinition const& _contract, ExecutionContext _context);
 
 	langutil::EVMVersion const m_evmVersion;
 	OptimiserSettings const m_optimiserSettings;

libsolidity/codegen/ir/IRGeneratorForStatements.cpp

@@ -2934,7 +2934,14 @@ IRVariable IRGeneratorForStatements::readFromLValue(IRLValue const& _lvalue)
 			solUnimplementedAssert(_lvalue.type.isValueType(), "");
 			solUnimplementedAssert(_lvalue.type.sizeOnStack() == 1, "");
 			solAssert(_lvalue.type == *_immutable.variable->type(), "");
-			define(result) << "loadimmutable(\"" << to_string(_immutable.variable->id()) << "\")\n";
+			if (m_context.executionContext() == IRGenerationContext::ExecutionContext::Creation)
+				define(result) <<
+					m_utils.readFromMemory(*_immutable.variable->type()) <<
+					"(" <<
+					to_string(m_context.immutableMemoryOffset(*_immutable.variable)) <<
+					")\n";
+			else
+				define(result) << "loadimmutable(\"" << to_string(_immutable.variable->id()) << "\")\n";
 		},
 		[&](IRLValue::Tuple const&) {
 			solAssert(false, "Attempted to read from tuple lvalue.");

test/libsolidity/semanticTests/immutable/assign_from_immutables.sol

@@ -0,0 +1,20 @@
+contract C {
+	uint immutable public a;
+	uint immutable public b;
+	uint immutable public c;
+	uint immutable public d;
+
+	constructor() {
+		a = 1;
+		b = a;
+		c = b;
+		d = c;
+	}
+}
+// ====
+// compileViaYul: also
+// ----
+// a() -> 1
+// b() -> 1
+// c() -> 1
+// d() -> 1