jyn514 / saltwater

A C compiler written in Rust, with a focus on good error messages.
BSD 3-Clause "New" or "Revised" License
293 stars 27 forks source link

Don't emit further type errors if already encountered type error #502

Closed ryankeleti closed 4 years ago

ryankeleti commented 4 years ago

This fixes #458 by checking for Type::Error before reporting other errors. Also undeclared identifiers are now set to be lvals.

Not sure if my code is the most idiomatic / if I got all cases, but helped to get a better understanding of the analyzer.

jyn514 commented 4 years ago
testing tests/runner-tests/expr/explicit_casts/6.c
thread 'run_all' panicked at 'verification error: - inst1 (return v0): arg 0 (v0) has type i64, must match function signature of i32
note: while compiling function u0:0() -> i32 system_v {
block0:
    v0 = iconst.i64 5
    return v0
}
$ cat tests/runner-tests/expr/explicit_casts/6.c
// fail
int main() { return (void)5; }

Looks like somewhere is returning the valid expression after an invalid operation instead returning Type::Error.

jyn514 commented 4 years ago
testing tests/runner-tests/decls/test_initializers/8.c
thread 'run_all' panicked at 'assertion failed: errs.len() == n', tests/utils/mod.rs:168:37

Looks like you brought down the number of cascading errors in one of the test cases :)

$ cargo run tests/runner-tests/decls/test_initializers/8.c
   Compiling saltwater v0.11.0 (/home/joshua/src/rust/saltwater)
    Finished dev [unoptimized + debuginfo] target(s) in 5.74s
     Running `/home/joshua/.local/lib/cargo/target/debug/swcc tests/runner-tests/decls/test_initializers/8.c`
tests/runner-tests/decls/test_initializers/8.c:4:5 warning: implicit int is deprecated and may be removed in a future release
int a[](*fp)() = {0};
    ^^^^^^^^^^
tests/runner-tests/decls/test_initializers/8.c:2:1 error: invalid program: variable has incomplete type 'void'
void i = {1};
^^^^^^
tests/runner-tests/decls/test_initializers/8.c:3:1 error: invalid program: variable has incomplete type 'void'
void i = 1;
^^^^^^
tests/runner-tests/decls/test_initializers/8.c:3:6 error: invalid program: redefinition of 'i'
void i = 1;
     ^
tests/runner-tests/decls/test_initializers/8.c:4:5 error: invalid program: functions cannot return 'int ()'
int a[](*fp)() = {0};
    ^^^^^^^^^^
tests/runner-tests/decls/test_initializers/8.c:4:5 error: invalid program: arrays cannot contain functions (got 'int (int *fp)()'). help: try storing array of pointer to function: (*int (int *fp)()[])(...)
int a[](*fp)() = {0};
    ^^^^^^^^^^

Previously it also gave the following errors:

tests/runner-tests/decls/test_initializers/8.c:2:11 error: invalid program: cannot implicitly convert 'long' to '<type error>'
void i = {1};
          ^
tests/runner-tests/decls/test_initializers/8.c:3:10 error: invalid program: cannot implicitly convert 'long' to '<type error>'
void i = 1;
         ^
tests/runner-tests/decls/test_initializers/8.c:4:19 error: invalid program: cannot implicitly convert 'long' to '<type error>'
int a[](*fp)() = {0};
                  ^

Should be fine to update it to expect 5 errors instead of 8.

ryankeleti commented 4 years ago

Maybe something to look into: gcc reports only once for each undeclared identifier in the current function, while clang more or less has the behavior from this PR. Also sorry for merge commit!

jyn514 commented 4 years ago

Maybe something to look into: gcc reports only once for each undeclared identifier in the current function, while clang more or less has the behavior from this PR. Also sorry for merge commit!

Ooh, that does sound nice. Maybe the PureAnalyzer could store a list of undeclared identifiers and not emit the error if the id is already in the list? (not necessarily in this PR though)

ryankeleti commented 4 years ago

Ooh, that does sound nice. Maybe the PureAnalyzer could store a list of undeclared identifiers and not emit the error if the id is already in the list? (not necessarily in this PR though)

I like that!