alliedmodders / sourcepawn

A small, statically typed scripting language.
Other
366 stars 62 forks source link

Enum struct char not detected as non-array #558

Open JoinedSenses opened 3 years ago

JoinedSenses commented 3 years ago

single char, not array member of enum struct is able to be passed as char[] parameter. Reproducible code: Occurs on both 1.10 and 1.11

enum struct Test {
    int x;
    char c;
}

Test g_Test[1];

public void OnPluginStart() {
    f(g_Test[0].c, 32); // overflow and Test::c isnt an array. should produce error 035: argument type mismatch
    PrintToServer("%s", g_Test[0].c); // " is a test"

    char c;
    f(c, 32); // correctly produces error
    PrintToServer("%s", c);
}

void f(char[] str, int size) {
    strcopy(str, size, "this is a test");
}
dvander commented 3 years ago

Thanks for the report - this is definitely a bug.

dvander commented 3 years ago

I can't decide in which way this is a bug. The "32" is incorrect in the example. The correct value - 1 - would work here. But there's still some kind of type error.

"char" is kind of weird because when we added Transitional Syntax, the compiler was in pretty bad shape. We had no way of adding 8-bit stack types. So it became a 32-bit type, and magically 8-bit once in an array. With enum structs, it's part of an array, so it magically becomes 8-bit. This causes it to pass the type checker because it's an 8-bit slot in an array.

However "char" is very much not "char[]", so we'll have to find some way to tell the type checker: this is an 8-bit reference, but not an array.

dvander commented 3 years ago

The same bug actually happens with just a normal field (eg, g_Test[0].x). The problem: spcomp hardcodes a list of how values can be stored in memory. There are three descriptors for indirection: "reference", "arraycell", and "arraychar". A "reference" is a parameter with the & modifier. An "arraycell" is a pointer to an array cell. An "arraychar" is a pointer to an array.

Array indices (x[y]) evaluate to an "arraycell", which has implicit conversion to an "array" argument. This is what makes slices work. Since enum structs are just arrays, x.y works just like x[y], so and field will evaluate to an "arraycell" or "arraychar". Which means the struct can be sliced. Not really what we were aiming for with enum structs.

So why don't we switch x.y to evaluate to a "reference"? The compiler expects "reference" types to only originate from parameters. So we'd have to introduce two new descriptor types.

But I was really hoping to remove all descriptor types. They're bad design, and require special casing all over the codebase. I really don't want to extend this mess to fix a long-standing type checking error. Especially an error that technically produces functioning code.

So I'm moving this to the future fix list and leaving it out of 1.11.