Failed assertions in macros are reported in the comptime code, but not the annotation

[Thunkar]

Aim

Given:

use std::{collections::umap::UHashMap, hash::{BuildHasherDefault, poseidon2::Poseidon2Hasher}};

comptime mut global ARG_STORAGE: UHashMap<FunctionDefinition, str<7>, BuildHasherDefault<Poseidon2Hasher>> = UHashMap::default();

comptime fn this_has_to_run_first(f: FunctionDefinition, arg: str<7>) {
    ARG_STORAGE.insert(f, arg);
}

comptime fn this_has_to_run_after(f: FunctionDefinition) {
    let maybe_arg = ARG_STORAGE.get(f);
    assert(maybe_arg.is_some(), "Function has to run last, please invert the annotation order");
    // Do something with arg
}

#[this_has_to_run_after]
#[this_has_to_run_first("oh noes")]
fn main() {}

I would like a hint in the main function that the attribute that failed to run was this_has_to_run_after

Expected Behavior

LSP should show a squiggly red line on #[this_has_to_run_after]

Bug

LSP shows the error at the failed assertion

To Reproduce

Use the example above

Workaround

None

Workaround Description

No response

Additional Context

No response

Project Impact

Nice-to-have

Blocker Context

No response

Nargo Version

No response

NoirJS Version

No response

Proving Backend Tooling & Version

No response

Would you like to submit a PR for this Issue?

None

Support Needs

No response

[asterite]

In #6277 it'll be like this:

Does that look good?

[Thunkar]

Does that look good?

Certainly an improvement! However, even when running just nargo compile, would it be possible to have some pointer in the annotation "call site" (on top of main in this case)?

This annotation can potentially be used in a lot of functions and the actual comptime fn is probably defined in a different place, which makes the error a lot less useful.

[jfecher]

@asterite I'll also note that if a user doesn't know to bring up the error list (I had to google it) then they may not know there is even more context to see to begin with.

When I first tried in neovim as well I didn't see any change due to the lack of a details list.

[asterite]

However, even when running just nargo compile, would it be possible to have some pointer in the annotation "call site" (on top of main in this case)?

What do you mean? Here's the error I currently get:

error: Function has to run last, please invert the annotation order
   ┌─ src/main.nr:15:12
   │
15 │     assert(maybe_arg.is_some(), "Function has to run last, please invert the annotation order");
   │            ------------------- Assertion failed
   │
   = Call stack:
     1. src/main.nr:19:3
     2. src/main.nr:10:5

The last lines show the callstack, and from it you can see where it was called from.

Or do you mean something like this? #5842

[asterite]

I'll also note that if a user doesn't know to bring up the error list (I had to google it) then they may not know there is even more context to see to begin with.

Should each call stack frame be reported as a separate error then?

[jfecher]

Should each call stack frame be reported as a separate error then?

Possibly? I'm not sure if it'd be confusing or excessive for large call stacks.

[asterite]

The problem is that if you have multiple macro errors, if they are reported separately you wouldn't know how to related all of them. I think showing them as related to the main error is better.