confusing error message when attempting to append to list #166

New Issue

sb10q · 2022-01-13T21:26:27+08:00

sb10q commented

2022-01-13 21:26:27 +08:00

It's pretty guaranteed that users will try to append to lists:

def run() -> int32:
    data = []
    data.append(3)
    return 0

Unfortunately the error message is pretty bad:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: "Cannot unify record[append=var7] with list[var6] at classes.py: line 3 column 5"', nac3standalone/src/main.rs:224:35

Side note: Can we get rid of those "varXX" references in error messages once and for all? I don't think it's a good idea to ever print that.

It's pretty guaranteed that users will try to append to lists: ```python def run() -> int32: data = [] data.append(3) return 0 ``` Unfortunately the error message is pretty bad: ``` thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: "Cannot unify record[append=var7] with list[var6] at classes.py: line 3 column 5"', nac3standalone/src/main.rs:224:35 ``` Side note: Can we get rid of those "varXX" references in error messages once and for all? I don't think it's a good idea to ever print that.

pca006132 referenced this issue

2022-02-21 18:43:18 +08:00

Better error messages #195

sb10q commented

2022-02-22 16:13:55 +08:00

The message is better now, but non-deterministic.

Sometimes I get:

`list[var56]::append` field does not exist at /home/sb/nac3/nac3artiq/demo/demo.py: line 15 column 9

and sometimes:

list[var60]::append` field does not exist at /home/sb/nac3/nac3artiq/demo/demo.py: line 15 column 9

What about removing [varXX] completely? It brings nothing to the user and just makes the error more confusing.

We probably also want to say "field or method" and not just "field".

The Python notation for attributes is also . and not ::. Using the former would make the message clearer.

The message is better now, but non-deterministic. Sometimes I get: ``` `list[var56]::append` field does not exist at /home/sb/nac3/nac3artiq/demo/demo.py: line 15 column 9 ``` and sometimes: ``` list[var60]::append` field does not exist at /home/sb/nac3/nac3artiq/demo/demo.py: line 15 column 9 ``` What about removing ``[varXX]`` completely? It brings nothing to the user and just makes the error more confusing. We probably also want to say "field or method" and not just "field". The Python notation for attributes is also ``.`` and not ``::``. Using the former would make the message clearer.

pca006132 commented

2022-02-22 17:03:11 +08:00

We could remove the type variable, but in general the type variable would be useful for diagnostics for other cases. Making the type variable name deterministic is hard because the execution order is indeterministic and we have to rely on something to name it.

sb10q commented

2022-02-22 17:04:25 +08:00

Maybe make it optional? It then could be enabled with an environment variable that nac3artiq reads. Similar to ARTIQ_DUMP_LLVM etc. in the legacy compiler.

sb10q commented

2022-02-22 17:05:12 +08:00

Assuming by "diagnostics" you mean "figuring out compiler bugs".

pca006132 commented

2022-02-22 17:06:14 +08:00

Maybe make it optional? It then could be enabled with an environment variable that nac3artiq reads. Similar to ARTIQ_DUMP_LLVM etc. in the legacy compiler.

Well the problem is that, it may be helpful to print something like list[int32], but for more complicated types that have several generic parameters with some type variables, we will have to print the type variable in some way... Or we could just omit the generic parameters in the default case.

> Maybe make it optional? It then could be enabled with an environment variable that nac3artiq reads. Similar to ARTIQ_DUMP_LLVM etc. in the legacy compiler. Well the problem is that, it may be helpful to print something like `list[int32]`, but for more complicated types that have several generic parameters with some type variables, we will have to print the type variable in some way... Or we could just omit the generic parameters in the default case.

pca006132 commented

2022-02-22 17:19:47 +08:00

Assuming by "diagnostics" you mean "figuring out compiler bugs".

No, I mean for users to figure out why the type is wrong... You need the generic parameters for more complicated cases.

> Assuming by "diagnostics" you mean "figuring out compiler bugs". No, I mean for users to figure out why the type is wrong... You need the generic parameters for more complicated cases.

sb10q commented

2022-02-23 06:57:20 +08:00

In what situation is varXX useful in the error messages?
Maybe it should be also renamed to typevarXX? var is just too generic and it is unclear what it means.

In what situation is ``varXX`` useful in the error messages? Maybe it should be also renamed to ``typevarXX``? ``var`` is just too generic and it is unclear what it means.

sb10q commented

2022-04-12 09:28:42 +08:00

ca07cb66cd

https://git.m-labs.hk/M-Labs/nac3/commit/ca07cb66cd1cc341bd64a8d8bc0e46cfbe5383aa

sb10q closed this issue

2022-04-12 10:36:16 +08:00

sb10q added this to the Alpha milestone 2022-04-12 11:09:02 +08:00

Sign in to join this conversation.