Applied code review and some CI failure fixes.

docs/internals/source_mappings.rst

@@ -65,5 +65,6 @@ This means the following source mappings represent the same information:
 
 ``1:2:1;:9;2:1:2;;``
 
-Important to note is that :ref:`verbatim <yul-verbatim>` verbatim instructions
-do not have individual but only one source mapping.
+Important to note is that when the :ref:`verbatim <yul-verbatim>` builtin is used,
+the source mappings will be invalid: The builtin is considered a single
+instruction instead of potentially multiple.

libevmasm/AssemblyItem.cpp

@@ -87,7 +87,9 @@ size_t AssemblyItem::bytesRequired(size_t _addressLength) const
 	case PushImmutable:
 		return 1 + 32;
 	case AssignImmutable:
-		if (m_immutableOccurrences)
+		if (!m_immutableOccurrences)
+			return 0;
+		else if (m_immutableOccurrences.value() != 0)
 			// (DUP DUP PUSH <n> ADD MSTORE)* (PUSH <n> ADD MSTORE)
 			return (*m_immutableOccurrences - 1) * (5 + 32) + (3 + 32);
 		else
@@ -324,6 +326,27 @@ ostream& solidity::evmasm::operator<<(ostream& _out, AssemblyItem const& _item)
 	return _out;
 }
 
+size_t AssemblyItem::opcodeCount() const noexcept
+{
+	switch (m_type)
+	{
+		case AssemblyItemType::AssignImmutable:
+			// Append empty items if this AssignImmutable was referenced more than once.
+			// For n immutable occurrences the first (n - 1) occurrences will
+			// generate 5 opcodes and the last will generate 3 opcodes,
+			// because it is reusing the 2 top-most elements on the stack.
+			if (!m_immutableOccurrences)
+				// AssignImmutable without any uses does not need to be assigned to at all.
+				return 0;
+			else if (m_immutableOccurrences.value() != 0)
+				return (*m_immutableOccurrences - 1) * 5 + 3;
+			else
+				return 2; // two POP's
+		default:
+			return 1;
+	}
+}
+
 std::string AssemblyItem::computeSourceMapping(
 	AssemblyItems const& _items,
 	map<string, unsigned> const& _sourceIndicesMap
@@ -405,17 +428,8 @@ std::string AssemblyItem::computeSourceMapping(
 			}
 		}
 
-		// Append empty items if this AssignImmutable was referenced more than once.
-		// For n immutable occurrences the first (n - 1) occurrences will
-		// generate 5 opcodes and the last will generate 3 opcodes,
-		// because it is reusing the 2 top-most elements on the stack.
-		if (item.immutableOccurrences())
-		{
-			auto const n = (item.immutableOccurrences() - 1) * 5 + 3;
-			ret += string(n - 1, ';');
-		}
-		else if (item.type() == AssemblyItemType::AssignImmutable)
-			ret += ';'; // two POP's
+		if (item.opcodeCount() > 1)
+			ret += string(item.opcodeCount() - 1, ';');
 
 		prevStart = location.start;
 		prevLength = length;

libevmasm/AssemblyItem.h

@@ -171,7 +171,8 @@ public:
 	size_t m_modifierDepth = 0;
 
 	void setImmutableOccurrences(size_t _n) const { m_immutableOccurrences = _n; }
-	size_t immutableOccurrences() const noexcept { return m_immutableOccurrences.value_or(0); }
+
+	size_t opcodeCount() const noexcept;
 
 private:
 	AssemblyItemType m_type;

test/libevmasm/Assembler.cpp

@@ -21,15 +21,16 @@
  * Tests for the assembler.
  */
 
-#include <libsolutil/JSON.h>
 #include <libevmasm/Assembly.h>
+#include <libsolutil/JSON.h>
+#include <libyul/Exceptions.h>
 
 #include <boost/test/unit_test.hpp>
 
+#include <algorithm>
+#include <memory>
 #include <string>
 #include <tuple>
-#include <memory>
-#include <libyul/Exceptions.h>
 
 using namespace std;
 using namespace solidity::langutil;
@@ -196,22 +197,22 @@ BOOST_AUTO_TEST_CASE(immutables_and_its_source_maps)
 			};
 
 			auto subAsm = make_shared<Assembly>();
-			for (int8_t i = 0; i < numImmutables; ++i)
+			for (char i = 0; i < numImmutables; ++i)
 			{
 				for (int r = 0; r < numActualRefs; ++r)
 				{
 					subAsm->setSourceLocation(SourceLocation{10*i, 10*i + 6 + r, subName});
-					subAsm->appendImmutable(string(1, 'a' + i)); // "a", "b", ...
+					subAsm->appendImmutable(string(1, char('a' + i))); // "a", "b", ...
 				}
 			}
 
 			Assembly assembly;
-			for (int8_t i = 1; i <= numImmutables; ++i)
+			for (char i = 1; i <= numImmutables; ++i)
 			{
 				assembly.setSourceLocation({10*i, 10*i + 3+i, assemblyName});
 				assembly.append(u256(0x71));              // immutble value
 				assembly.append(u256(0));                 // target... modules?
-				assembly.appendImmutableAssignment(string(1, 'a' + i - 1));
+				assembly.appendImmutableAssignment(string(1, char('a' + i - 1)));
 			}
 
 			assembly.appendSubroutine(subAsm);

test/libyul/objectCompiler/function_series.yul

@@ -11,15 +11,15 @@ object "Contract" {
 // ----
 // Assembly:
 //     /* "source":33:48   */
-//   jump(tag_1)
-// tag_2:
-// tag_3:
+//   jump(tag_3)
+// tag_1:
+// tag_4:
 //   jump	// out
 //     /* "source":53:68   */
-// tag_4:
+// tag_2:
 // tag_5:
 //   jump	// out
-// tag_1:
+// tag_3:
 //     /* "source":83:84   */
 //   0x01
 //     /* "source":80:81   */

test/libyul/objectCompiler/immutable_long_name_does_not_end_up_in_bytecode.yul

@@ -17,4 +17,4 @@ object "a" {
 //   assignImmutable("0x85a5b1db611c82c46f5fa18e39ae218397536256c451e5de155a86de843a9ad6")
 // Bytecode: 73123456789012345678901234567890123456789060005050
 // Opcodes: PUSH20 0x1234567890123456789012345678901234567890 PUSH1 0x0 POP POP
-// SourceMappings: 167:42:0:-:0;58:1;32:187
+// SourceMappings: 167:42:0:-:0;58:1;32:187;

test/libyul/objectCompiler/jump_tags.yul

@@ -11,21 +11,21 @@ object "Contract" {
 // ----
 // Assembly:
 //     /* "source":33:54   */
-//   jump(tag_1)
-// tag_2:
+//   jump(tag_3)
+// tag_1:
 //     /* "source":48:52   */
-//   tag_4
+//   tag_5
 //     /* "source":50:51   */
 //   0x01
 //     /* "source":48:52   */
-//   tag_5
+//   tag_2
 //   jump	// in
-// tag_4:
+// tag_5:
 //     /* "source":33:54   */
-// tag_3:
+// tag_4:
 //   jump	// out
 //     /* "source":59:104   */
-// tag_5:
+// tag_2:
 //     /* "source":78:79   */
 //   dup1
 //     /* "source":75:89   */
@@ -46,20 +46,20 @@ object "Contract" {
 //     /* "source":92:101   */
 //   add
 //     /* "source":90:102   */
-//   tag_5
+//   tag_2
 //   jump	// in
 // tag_8:
 //     /* "source":59:104   */
 //   pop
 // tag_6:
 //   jump	// out
-// tag_1:
+// tag_3:
 //     /* "source":109:113   */
 //   tag_9
 //     /* "source":111:112   */
 //   0x01
 //     /* "source":109:113   */
-//   tag_5
+//   tag_2
 //   jump	// in
 // tag_9:
 // Bytecode: 6026565b600b6001600e565b5b565b8015601857506024565b602260028201600e565b505b565b602e6001600e565b