diff --git a/packages/bugc/CHANGELOG.md b/packages/bugc/CHANGELOG.md index 21e94e594..bf7b8d701 100644 --- a/packages/bugc/CHANGELOG.md +++ b/packages/bugc/CHANGELOG.md @@ -43,6 +43,12 @@ support. Changes to the specification itself are tracked in the root - `==`, `!=` and a mapping key with bytes or a string in calldata are now type errors. Before, they compared or hashed the calldata reference's word (offset and length), not the bytes ([#379]). +- `==` and `!=` on arrays, structs and mappings are now type errors, + as in Solidity. Before, an array or struct in memory, or a struct + copied from storage, compared by its address in memory, so equal + contents in two places were unequal; a mapping compared its own + slot, so it always equaled itself; an array in storage failed in IR + generation ([#397]). - A struct member of a struct in storage now takes all the slots its struct needs, as in Solidity, and the member after it starts at the next free slot. Before, it took one slot, so the members after a @@ -581,3 +587,4 @@ First publication. [#388]: https://github.com/ethdebug/format/pull/388 [#391]: https://github.com/ethdebug/format/pull/391 [#395]: https://github.com/ethdebug/format/pull/395 +[#397]: https://github.com/ethdebug/format/pull/397 diff --git a/packages/bugc/src/typechecker/checker.test.ts b/packages/bugc/src/typechecker/checker.test.ts index 92ad77a6b..b7b80d518 100644 --- a/packages/bugc/src/typechecker/checker.test.ts +++ b/packages/bugc/src/typechecker/checker.test.ts @@ -194,6 +194,61 @@ describe("checkProgram", () => { }); }); + it("should type check == and != on elementary types", () => { + const result = check(` + name Test; + storage { [0] owner: address; [1] flag: bool; [2] n: uint256; } + code { + let b1 = owner == msg.sender; + let b2 = flag != true; + let b3 = n == 1; + let b4 = n != n; + } + `); + + expect(result.success).toBe(true); + expect(Result.hasMessages(result)).toBe(false); + }); + + it("should reject == and != on arrays, structs and mappings", () => { + const comparisons: Record = { + "a dynamic array in storage": ["nums == nums", "array"], + "a fixed array in storage": ["fixed != fixed", "array"], + "an array in memory": ["local == local", "array"], + "an array and a storage array": ["local != nums", "array"], + "a struct in storage": ["point == point", "Point"], + "a struct and a mapping value": ["point != points[0]", "Point"], + "a mapping": ["balances == balances", "mapping"], + }; + + for (const [name, [comparison, type]] of Object.entries(comparisons)) { + const result = check(` + name Test; + define { + struct Point { x: uint256; y: uint256; }; + } + storage { + [0] nums: array; + [1] fixed: array; + [4] point: Point; + [6] points: mapping; + [7] balances: mapping; + } + code { + let local: array = [1, 2, 3]; + let b = ${comparison}; + } + `); + + expect(result.success, name).toBe(false); + expect(Result.countErrors(result), name).toBe(1); + expect(result).toHaveMessage({ + severity: Severity.Error, + message: `Cannot compare ${type}`, + }); + } + }); + it("should type check logical operators", () => { const result = check(` name Test; diff --git a/packages/bugc/src/typechecker/expressions.ts b/packages/bugc/src/typechecker/expressions.ts index b78e890d6..2ca394285 100644 --- a/packages/bugc/src/typechecker/expressions.ts +++ b/packages/bugc/src/typechecker/expressions.ts @@ -250,15 +250,11 @@ export const expressionChecker: Pick, "expression"> = { case "==": case "!=": - // Bytes or a string in calldata is a word of offset and - // length, which compares with no other value's - if ( - Type.Elementary.isCalldata(leftType) || - Type.Elementary.isCalldata(rightType) - ) { - const type = Type.Elementary.isCalldata(leftType) - ? leftType - : rightType; + // Arrays, structs and mappings do not compare, as in + // Solidity. Bytes or a string in calldata is a word of + // offset and length, which compares with no other value's + if (isUncomparable(leftType) || isUncomparable(rightType)) { + const type = isUncomparable(leftType) ? leftType : rightType; errors.push( new TypeError( `Cannot compare ${Type.format(type)}`, @@ -1210,6 +1206,20 @@ function isValidCast(fromType: Type, toType: Type): boolean { return false; } +/** + * Whether `==` and `!=` reject an operand of this type: an array, a + * struct, a mapping, or bytes or a string in calldata + */ +function isUncomparable(type: Type | undefined): type is Type { + return ( + !!type && + (Type.isArray(type) || + Type.isStruct(type) || + Type.isMapping(type) || + Type.Elementary.isCalldata(type)) + ); +} + const integerOperators = new Set([ "+", "-", diff --git a/packages/web/docs/implementation-guides/compiler/case-study-bug.mdx b/packages/web/docs/implementation-guides/compiler/case-study-bug.mdx index c1bb09696..eddd3fe19 100644 --- a/packages/web/docs/implementation-guides/compiler/case-study-bug.mdx +++ b/packages/web/docs/implementation-guides/compiler/case-study-bug.mdx @@ -114,7 +114,8 @@ data of each operand. As for a mapping key, this assumes that two different contents do not have the same hash. Both operands must have the same type, in memory: bytes or a string in calldata do not compare, so cast or copy them to memory first. A `bytesN` is a value, -and compares as a word. +and compares as a word. Arrays, structs and mappings do not compare, +as in Solidity: `==` and `!=` on them are type errors. ```bug let a: bytes = msg.data[0:4];