From 8b564a7be71088e98fd6ef572e96262a8da9f42f Mon Sep 17 00:00:00 2001
From: chriseth <chris@ethereum.org>
Date: Tue, 4 Aug 2020 13:18:41 +0200
Subject: [PATCH] Fix reads checks for immutable variables.

---
 Changelog.md                                  |  1 +
 libsolidity/analysis/ImmutableValidator.cpp   | 19 +++++++++++--------
 .../syntaxTests/immutable/complex.sol         |  8 ++++++++
 .../immutable/conditionally_initialized.sol   |  2 +-
 .../ctor_indirect_initialization.sol          |  2 +-
 .../immutable/ctor_modifier_args.sol          |  2 +-
 .../ctor_modifier_initialization.sol          |  2 +-
 .../syntaxTests/immutable/decrement.sol       |  4 ++--
 .../syntaxTests/immutable/delete.sol          |  4 ++--
 .../immutable/delete_and_initialize.sol       |  9 +++++++++
 .../function_pointer_initializing.sol         |  3 +--
 .../syntaxTests/immutable/increment.sol       |  4 ++--
 .../immutable/increment_decrement.sol         | 11 +++++++++++
 .../immutable/inheritance_ctor_argument.sol   |  2 +-
 ...e_ctor_inherit_specifier_argument_init.sol |  2 +-
 .../immutable/inheritance_wrong_ctor.sol      |  3 +--
 .../immutable/loop_initialized.sol            |  2 +-
 .../syntaxTests/immutable/unary.sol           |  8 ++++++++
 .../writing_after_initialization.sol          |  3 +--
 .../writing_after_initialization_modifier.sol |  3 +--
 20 files changed, 65 insertions(+), 29 deletions(-)
 create mode 100644 test/libsolidity/syntaxTests/immutable/complex.sol
 create mode 100644 test/libsolidity/syntaxTests/immutable/delete_and_initialize.sol
 create mode 100644 test/libsolidity/syntaxTests/immutable/increment_decrement.sol
 create mode 100644 test/libsolidity/syntaxTests/immutable/unary.sol

diff --git a/Changelog.md b/Changelog.md
index cc92dd025..3ea0f116e 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -13,6 +13,7 @@ Compiler Features:
 
 Bugfixes:
  * AST: Remove ``null`` member values also when the compiler is used in standard-json-mode.
+ * Immutables: Properly treat complex assignment and increment/decrement as both reading and writing and thus disallow it everywhere for immutable variables.
  * Optimizer: Keep side-effects of ``x`` in ``byte(a, shr(b, x))`` even if the constants ``a`` and ``b`` would make the expression zero unconditionally. This optimizer rule is very hard if not impossible to trigger in a way that it can result in invalid code, though.
  * SMTChecker: Fix internal error in BMC function inlining.
  * SMTChecker: Fix internal error on array implicit conversion.
diff --git a/libsolidity/analysis/ImmutableValidator.cpp b/libsolidity/analysis/ImmutableValidator.cpp
index 5af640e58..3e89417cd 100644
--- a/libsolidity/analysis/ImmutableValidator.cpp
+++ b/libsolidity/analysis/ImmutableValidator.cpp
@@ -163,41 +163,44 @@ void ImmutableValidator::analyseVariableReference(VariableDeclaration const& _va
 	if (!_variableReference.isStateVariable() || !_variableReference.immutable())
 		return;
 
-	if (_expression.annotation().willBeWrittenTo && _expression.annotation().lValueOfOrdinaryAssignment)
+	// If this is not an ordinary assignment, we write and read at the same time.
+	bool write = _expression.annotation().willBeWrittenTo;
+	bool read = !_expression.annotation().willBeWrittenTo || !_expression.annotation().lValueOfOrdinaryAssignment;
+	if (write)
 	{
 		if (!m_currentConstructor)
 			m_errorReporter.typeError(
 				1581_error,
 				_expression.location(),
-				"Immutable variables can only be initialized inline or assigned directly in the constructor."
+				"Cannot write to immutable here: Immutable variables can only be initialized inline or assigned directly in the constructor."
 			);
 		else if (m_currentConstructor->annotation().contract->id() != _variableReference.annotation().contract->id())
 			m_errorReporter.typeError(
 				7484_error,
 				_expression.location(),
-				"Immutable variables must be initialized in the constructor of the contract they are defined in."
+				"Cannot write to immutable here: Immutable variables must be initialized in the constructor of the contract they are defined in."
 			);
 		else if (m_inLoop)
 			m_errorReporter.typeError(
 				6672_error,
 				_expression.location(),
-				"Immutable variables can only be initialized once, not in a while statement."
+				"Cannot write to immutable here: Immutable variables cannot be initialized inside a loop."
 			);
 		else if (m_inBranch)
 			m_errorReporter.typeError(
 				4599_error,
 				_expression.location(),
-				"Immutable variables must be initialized unconditionally, not in an if statement."
+				"Cannot write to immutable here: Immutable variables cannot be initialized inside an if statement."
 			);
-
-		if (!m_initializedStateVariables.emplace(&_variableReference).second)
+		else if (m_initializedStateVariables.count(&_variableReference))
 			m_errorReporter.typeError(
 				1574_error,
 				_expression.location(),
 				"Immutable state variable already initialized."
 			);
+		m_initializedStateVariables.emplace(&_variableReference);
 	}
-	else if (m_inConstructionContext)
+	if (read && m_inConstructionContext)
 		m_errorReporter.typeError(
 			7733_error,
 			_expression.location(),
diff --git a/test/libsolidity/syntaxTests/immutable/complex.sol b/test/libsolidity/syntaxTests/immutable/complex.sol
new file mode 100644
index 000000000..b624430ea
--- /dev/null
+++ b/test/libsolidity/syntaxTests/immutable/complex.sol
@@ -0,0 +1,8 @@
+contract A {
+  int immutable a;
+  constructor() { a = 5; }
+  function f() public { a += 7; }
+}
+
+// ----
+// TypeError 1581: (83-84): Cannot write to immutable here: Immutable variables can only be initialized inline or assigned directly in the constructor.
diff --git a/test/libsolidity/syntaxTests/immutable/conditionally_initialized.sol b/test/libsolidity/syntaxTests/immutable/conditionally_initialized.sol
index b4808b844..052a1b597 100644
--- a/test/libsolidity/syntaxTests/immutable/conditionally_initialized.sol
+++ b/test/libsolidity/syntaxTests/immutable/conditionally_initialized.sol
@@ -6,4 +6,4 @@ contract C {
     }
 }
 // ----
-// TypeError 4599: (86-87): Immutable variables must be initialized unconditionally, not in an if statement.
+// TypeError 4599: (86-87): Cannot write to immutable here: Immutable variables cannot be initialized inside an if statement.
diff --git a/test/libsolidity/syntaxTests/immutable/ctor_indirect_initialization.sol b/test/libsolidity/syntaxTests/immutable/ctor_indirect_initialization.sol
index f91e98333..e95bf5740 100644
--- a/test/libsolidity/syntaxTests/immutable/ctor_indirect_initialization.sol
+++ b/test/libsolidity/syntaxTests/immutable/ctor_indirect_initialization.sol
@@ -9,4 +9,4 @@ contract C {
     }
 }
 // ----
-// TypeError 1581: (119-120): Immutable variables can only be initialized inline or assigned directly in the constructor.
+// TypeError 1581: (119-120): Cannot write to immutable here: Immutable variables can only be initialized inline or assigned directly in the constructor.
diff --git a/test/libsolidity/syntaxTests/immutable/ctor_modifier_args.sol b/test/libsolidity/syntaxTests/immutable/ctor_modifier_args.sol
index 3f2461456..0a966f7c3 100644
--- a/test/libsolidity/syntaxTests/immutable/ctor_modifier_args.sol
+++ b/test/libsolidity/syntaxTests/immutable/ctor_modifier_args.sol
@@ -9,4 +9,4 @@ contract C {
     function f(uint a) internal pure {}
 }
 // ----
-// TypeError 1581: (59-60): Immutable variables can only be initialized inline or assigned directly in the constructor.
+// TypeError 1581: (59-60): Cannot write to immutable here: Immutable variables can only be initialized inline or assigned directly in the constructor.
diff --git a/test/libsolidity/syntaxTests/immutable/ctor_modifier_initialization.sol b/test/libsolidity/syntaxTests/immutable/ctor_modifier_initialization.sol
index 14f47364a..5970ab247 100644
--- a/test/libsolidity/syntaxTests/immutable/ctor_modifier_initialization.sol
+++ b/test/libsolidity/syntaxTests/immutable/ctor_modifier_initialization.sol
@@ -8,4 +8,4 @@ contract C {
     }
 }
 // ----
-// TypeError 1581: (102-103): Immutable variables can only be initialized inline or assigned directly in the constructor.
+// TypeError 1581: (102-103): Cannot write to immutable here: Immutable variables can only be initialized inline or assigned directly in the constructor.
diff --git a/test/libsolidity/syntaxTests/immutable/decrement.sol b/test/libsolidity/syntaxTests/immutable/decrement.sol
index 3c34dd3b7..5717f05c8 100644
--- a/test/libsolidity/syntaxTests/immutable/decrement.sol
+++ b/test/libsolidity/syntaxTests/immutable/decrement.sol
@@ -1,8 +1,8 @@
 contract C {
-    uint immutable x = 3;
+    uint immutable x;
     constructor() {
         x--;
     }
 }
 // ----
-// TypeError 7733: (67-68): 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.
+// TypeError 7733: (63-64): 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.
diff --git a/test/libsolidity/syntaxTests/immutable/delete.sol b/test/libsolidity/syntaxTests/immutable/delete.sol
index 58e9235f3..541fceb53 100644
--- a/test/libsolidity/syntaxTests/immutable/delete.sol
+++ b/test/libsolidity/syntaxTests/immutable/delete.sol
@@ -1,8 +1,8 @@
 contract C {
-    uint immutable x = 3;
+    uint immutable x;
     constructor() {
         delete x;
     }
 }
 // ----
-// TypeError 7733: (74-75): 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.
+// TypeError 7733: (70-71): 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.
diff --git a/test/libsolidity/syntaxTests/immutable/delete_and_initialize.sol b/test/libsolidity/syntaxTests/immutable/delete_and_initialize.sol
new file mode 100644
index 000000000..859a30a77
--- /dev/null
+++ b/test/libsolidity/syntaxTests/immutable/delete_and_initialize.sol
@@ -0,0 +1,9 @@
+contract C {
+    uint immutable x = 3;
+    constructor() {
+        delete x;
+    }
+}
+// ----
+// TypeError 1574: (74-75): Immutable state variable already initialized.
+// TypeError 7733: (74-75): 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.
diff --git a/test/libsolidity/syntaxTests/immutable/function_pointer_initializing.sol b/test/libsolidity/syntaxTests/immutable/function_pointer_initializing.sol
index 629d80f66..cc175d083 100644
--- a/test/libsolidity/syntaxTests/immutable/function_pointer_initializing.sol
+++ b/test/libsolidity/syntaxTests/immutable/function_pointer_initializing.sol
@@ -10,5 +10,4 @@ contract C is B(C.f) {
     function f() internal returns(uint) { return x = 2; }
 }
 // ----
-// TypeError 1581: (200-201): Immutable variables can only be initialized inline or assigned directly in the constructor.
-// TypeError 1574: (200-201): Immutable state variable already initialized.
+// TypeError 1581: (200-201): Cannot write to immutable here: Immutable variables can only be initialized inline or assigned directly in the constructor.
diff --git a/test/libsolidity/syntaxTests/immutable/increment.sol b/test/libsolidity/syntaxTests/immutable/increment.sol
index 00c20422c..11959ebc7 100644
--- a/test/libsolidity/syntaxTests/immutable/increment.sol
+++ b/test/libsolidity/syntaxTests/immutable/increment.sol
@@ -1,8 +1,8 @@
 contract C {
-    uint immutable x = 3;
+    uint immutable x;
     constructor() {
         x++;
     }
 }
 // ----
-// TypeError 7733: (67-68): 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.
+// TypeError 7733: (63-64): 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.
diff --git a/test/libsolidity/syntaxTests/immutable/increment_decrement.sol b/test/libsolidity/syntaxTests/immutable/increment_decrement.sol
new file mode 100644
index 000000000..ccfaf1920
--- /dev/null
+++ b/test/libsolidity/syntaxTests/immutable/increment_decrement.sol
@@ -0,0 +1,11 @@
+contract C {
+  uint immutable x;
+  uint immutable y;
+  constructor() {
+    ++x;
+    --y;
+  }
+}
+// ----
+// TypeError 7733: (77-78): 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.
+// TypeError 7733: (86-87): 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.
diff --git a/test/libsolidity/syntaxTests/immutable/inheritance_ctor_argument.sol b/test/libsolidity/syntaxTests/immutable/inheritance_ctor_argument.sol
index 695319378..bc084400e 100644
--- a/test/libsolidity/syntaxTests/immutable/inheritance_ctor_argument.sol
+++ b/test/libsolidity/syntaxTests/immutable/inheritance_ctor_argument.sol
@@ -11,4 +11,4 @@ contract C is B {
     constructor() B(y = 3) { }
 }
 // ----
-// TypeError 1581: (148-149): Immutable variables can only be initialized inline or assigned directly in the constructor.
+// TypeError 1581: (148-149): Cannot write to immutable here: Immutable variables can only be initialized inline or assigned directly in the constructor.
diff --git a/test/libsolidity/syntaxTests/immutable/inheritance_ctor_inherit_specifier_argument_init.sol b/test/libsolidity/syntaxTests/immutable/inheritance_ctor_inherit_specifier_argument_init.sol
index 8e3d67145..503f182d6 100644
--- a/test/libsolidity/syntaxTests/immutable/inheritance_ctor_inherit_specifier_argument_init.sol
+++ b/test/libsolidity/syntaxTests/immutable/inheritance_ctor_inherit_specifier_argument_init.sol
@@ -10,4 +10,4 @@ contract C is B(C.y = 3) {
     uint immutable y;
 }
 // ----
-// TypeError 1581: (104-107): Immutable variables can only be initialized inline or assigned directly in the constructor.
+// TypeError 1581: (104-107): Cannot write to immutable here: Immutable variables can only be initialized inline or assigned directly in the constructor.
diff --git a/test/libsolidity/syntaxTests/immutable/inheritance_wrong_ctor.sol b/test/libsolidity/syntaxTests/immutable/inheritance_wrong_ctor.sol
index 4f431474b..748d9da55 100644
--- a/test/libsolidity/syntaxTests/immutable/inheritance_wrong_ctor.sol
+++ b/test/libsolidity/syntaxTests/immutable/inheritance_wrong_ctor.sol
@@ -8,5 +8,4 @@ contract C is B {
     }
 }
 // ----
-// TypeError 7484: (88-89): Immutable variables must be initialized in the constructor of the contract they are defined in.
-// TypeError 1574: (88-89): Immutable state variable already initialized.
+// TypeError 7484: (88-89): Cannot write to immutable here: Immutable variables must be initialized in the constructor of the contract they are defined in.
diff --git a/test/libsolidity/syntaxTests/immutable/loop_initialized.sol b/test/libsolidity/syntaxTests/immutable/loop_initialized.sol
index b831f0f0b..d8c8ca47f 100644
--- a/test/libsolidity/syntaxTests/immutable/loop_initialized.sol
+++ b/test/libsolidity/syntaxTests/immutable/loop_initialized.sol
@@ -6,4 +6,4 @@ contract C {
     }
 }
 // ----
-// TypeError 6672: (88-89): Immutable variables can only be initialized once, not in a while statement.
+// TypeError 6672: (88-89): Cannot write to immutable here: Immutable variables cannot be initialized inside a loop.
diff --git a/test/libsolidity/syntaxTests/immutable/unary.sol b/test/libsolidity/syntaxTests/immutable/unary.sol
new file mode 100644
index 000000000..f3c75c09b
--- /dev/null
+++ b/test/libsolidity/syntaxTests/immutable/unary.sol
@@ -0,0 +1,8 @@
+contract A {
+  int immutable a;
+  constructor() { a = 5; }
+  function f() public { --a; }
+}
+
+// ----
+// TypeError 1581: (85-86): Cannot write to immutable here: Immutable variables can only be initialized inline or assigned directly in the constructor.
diff --git a/test/libsolidity/syntaxTests/immutable/writing_after_initialization.sol b/test/libsolidity/syntaxTests/immutable/writing_after_initialization.sol
index 05d366ead..4b2b91a15 100644
--- a/test/libsolidity/syntaxTests/immutable/writing_after_initialization.sol
+++ b/test/libsolidity/syntaxTests/immutable/writing_after_initialization.sol
@@ -6,5 +6,4 @@ contract C {
     }
 }
 // ----
-// TypeError 1581: (76-77): Immutable variables can only be initialized inline or assigned directly in the constructor.
-// TypeError 1574: (76-77): Immutable state variable already initialized.
+// TypeError 1581: (76-77): Cannot write to immutable here: Immutable variables can only be initialized inline or assigned directly in the constructor.
diff --git a/test/libsolidity/syntaxTests/immutable/writing_after_initialization_modifier.sol b/test/libsolidity/syntaxTests/immutable/writing_after_initialization_modifier.sol
index cdfd47443..d450a37d2 100644
--- a/test/libsolidity/syntaxTests/immutable/writing_after_initialization_modifier.sol
+++ b/test/libsolidity/syntaxTests/immutable/writing_after_initialization_modifier.sol
@@ -8,5 +8,4 @@ contract C {
     }
 }
 // ----
-// TypeError 1581: (111-112): Immutable variables can only be initialized inline or assigned directly in the constructor.
-// TypeError 1574: (111-112): Immutable state variable already initialized.
+// TypeError 1581: (111-112): Cannot write to immutable here: Immutable variables can only be initialized inline or assigned directly in the constructor.