Deduplicate compiler warnings based on their source location

[TomAFrench]

After #6926 we still get a number of cases where multiple instances of the same compiler warning/error is emitted pointing at the same line of source code. This tends to just make the output harder to read for very little benefit so I'm inclined to deduplicate these so that we only print them once.

[jfecher]

What are some examples of this?

[TomAFrench]

I think the case where this comes up most frequently is on comptime functions. If you run nargo check on aztec-nr then you should see it. For instance I get the below as part of the warnings emitted.

warning: index is private and not visible from the current module
   ┌─ value-note/src/value_note.nr:21:1
   │
21 │ #[note]
   │ ------- index is private
   │

warning: offset is private and not visible from the current module
   ┌─ value-note/src/value_note.nr:21:1
   │
21 │ #[note]
   │ ------- offset is private
   │

warning: length is private and not visible from the current module
   ┌─ value-note/src/value_note.nr:21:1
   │
21 │ #[note]
   │ ------- length is private
   │

warning: index is private and not visible from the current module
   ┌─ value-note/src/value_note.nr:21:1
   │
21 │ #[note]
   │ ------- index is private
   │

warning: offset is private and not visible from the current module
   ┌─ value-note/src/value_note.nr:21:1
   │
21 │ #[note]
   │ ------- offset is private
   │

warning: length is private and not visible from the current module
   ┌─ value-note/src/value_note.nr:21:1
   │
21 │ #[note]
   │ ------- length is private
   │

warning: index is private and not visible from the current module
   ┌─ value-note/src/value_note.nr:21:1
   │
21 │ #[note]
   │ ------- index is private
   │

warning: offset is private and not visible from the current module
   ┌─ value-note/src/value_note.nr:21:1
   │
21 │ #[note]
   │ ------- offset is private
   │

warning: length is private and not visible from the current module
   ┌─ value-note/src/value_note.nr:21:1
   │
21 │ #[note]
   │ ------- length is private
   │

warning: trait `traits::Empty` which provides `empty` is implemented but not in scope, please import it
   ┌─ value-note/src/value_note.nr:21:1
   │
21 │ #[note]
   │ -------
   │

warning: trait `note::note_interface::NoteInterface` which provides `get_note_type_id` is implemented but not in scope, please import it
   ┌─ value-note/src/value_note.nr:21:1
   │
21 │ #[note]
   │ -------
   │

warning: from_field_unsafe is private and not visible from the current module
   ┌─ value-note/src/value_note.nr:21:1
   │
21 │ #[note]
   │ ------- from_field_unsafe is private
   │

warning: from_field_unsafe is private and not visible from the current module
   ┌─ value-note/src/value_note.nr:21:1
   │
21 │ #[note]
   │ ------- from_field_unsafe is private
   │

warning: from_field_unsafe is private and not visible from the current module
   ┌─ value-note/src/value_note.nr:21:1
   │
21 │ #[note]
   │ ------- from_field_unsafe is private
   │

warning: from_field_unsafe is private and not visible from the current module
   ┌─ value-note/src/value_note.nr:21:1
   │
21 │ #[note]
   │ ------- from_field_unsafe is private
   │

[TomAFrench]

I'm also seeing this with the "brillig-constraints-check" SSA pass where if a brillig function returns an array or similar. you'll end up with a warning emitted for every constituent value.

[jfecher]

I can't help but be worried that removing errors based on their source location alone will end up discarding important errors.

What may be better is a more fine-grained poisoning of errors so that after one is emitted it isn't emitted again for a similar scope. E.g. if you misspell let fao = 3; then refer to it as foo each time, we should only give 1 error that foo isn't in scope rather than 1 for each usage. If you have multiple errors on that line/column though I'd still expect it to be emitted.

For something like the brillig-constraints-check pass, a separate error for each return value does sound useful to me since it says that all those values aren't constrained compared to only one or a few. Something we could do to improve brevity here instead could be to issue them as part of the same error. E.g. let (foo, bar) = my_call();, foo and bar are disconnected from circuit inputs and outputs...

[jfecher]

Another thing that'd be useful for these macro errors specifically is if we kept a history of expansions somehow so we could point to the generated code itself then add a helper like generated from ---> #[note]

[asterite]

Regarding the errors here, I wonder if they should happen exactly where they are. For example:

#[foo] // Currently errors here
comptime fn foo(_: FunctionDefinition) -> Quoted {
    quote {
        fn bar() {
            1 + "a"; // Ideally the error happens here
        }
    }
}

I think the issue right now is that a quoted expression just holds tokens without their spans. The spans are later added as the span of the macro call, which is why we get the error there.

But even if we started tracking spans it wouldn't be correct. For this code:

// main.nr
mod other;
use other::expr;

#[foo]
comptime fn foo(_: FunctionDefinition) -> Quoted {
    let expr = expr();
    quote {
        fn bar() {
            $expr;
        }
    }
}

// other.nr
pub comptime fn expr() -> Quote {
    quote { 1 + "a" }
}

The error would need to happen in other.nr right where the addition happens, but that happens in a different file. So in addition to spans we'd need to track files, but not only in tokens: also in the AST nodes where the errors eventually happen.

It would be a big refactor but it would make using macros much more pleasant and understandable... because I think right now if such an error happens your only option is to guess where the error is.

(I know this is slightly unrelated to the original issue, but it's also slightly related because I think the errors in the original snippet would now happen in different places)