Odin icon indicating copy to clipboard operation
Odin copied to clipboard

Duplicate case doesn't trigger an error when switching on struct value

Open jakubtomsu opened this issue 1 year ago • 4 comments

Context

The checker fails to find duplicate cases in a switch statement when struct value is the input. See the example below.

odin report output:

        Odin:    dev-2024-08:4dd846fa2
        OS:      Windows 10 Professional (version: 22H2), build 19045.4651   
        CPU:     Intel(R) Core(TM) i7-9700 CPU @ 3.00GHz
        RAM:     32681 MiB
        Backend: LLVM 18.1.8

Steps to Reproduce

Foo :: struct {
    a: int,
    b: int,
}

foo := Foo{1, 2}

// Works
switch foo.a {
case 1:
case 1: // Error: Duplicate case '1'
}

// Doesn't work
switch foo {
case {1, 2}:
case {1, 2}: // No error!
}

jakubtomsu avatar Aug 03 '24 17:08 jakubtomsu

Probably not a good resolution which is why this isn't a PR, but something like this should work, essentially we need to hash something more than just the ast pointer:

diff --git a/src/exact_value.cpp b/src/exact_value.cpp
index 1a42a82a9..22b3f9b20 100644
--- a/src/exact_value.cpp
+++ b/src/exact_value.cpp
@@ -8,6 +8,7 @@ struct HashKey;
 struct Type;
 struct Entity;
 gb_internal bool are_types_identical(Type *x, Type *y);
+gb_internal gbString expr_to_string(Ast *expression);
 
 struct Complex128 {
 	f64 real, imag;
@@ -86,8 +87,12 @@ gb_internal uintptr hash_exact_value(ExactValue v) {
 		res = gb_fnv32a(v.value_quaternion, gb_size_of(Quaternion256));
 		break;
 	case ExactValue_Compound:
-		res = ptr_map_hash_key(v.value_compound);
-		break;
+		{
+			gbString str = expr_to_string(v.value_compound);
+			res = gb_fnv32a(str, gb_strlen(str));
+			// res = ptr_map_hash_key(v.value_compound);
+			break;
+		}
 	case ExactValue_Procedure:
 		res = ptr_map_hash_key(v.value_procedure);
 		break;

laytan avatar Aug 11 '24 20:08 laytan

I guess the best way would be to build the binary value of the expression in memory and hash that? Or recursively hash the expression AST itself?

jakubtomsu avatar Aug 12 '24 07:08 jakubtomsu

Not sure really, I think there is no precedent for this in the compiler currently. Maybe hashing the type and expression (as a string) is already enough, maybe not.

Would like @gingerBill 's thoughts here

laytan avatar Aug 12 '24 11:08 laytan

The catching of duplicates is pretty much only possible with basic types, and anythinc compound like that because nigh impossible in the general case.

The hack with the hash can work, but again not always.

gingerBill avatar Aug 18 '24 11:08 gingerBill