fix: improve error message for local memory index bounds check

[crStiv]

Description:

• Enhanced error message for out-of-bounds local memory access
• Added specific details in error message:
  • Invalid index value
  • Total number of available locals
  • Valid index range
• Replaced generic validation with explicit bounds check

Example of new error message:
"invalid local memory index: 5 is out of bounds for procedure with 3 locals (valid range: 0..2)"

Fixes #1619

[plafer]

We actually already have the machinery to output very nice error messages (built on top of miette). The desired solution would output an error variant that is similar to InvalidSyscallTarget (in that it takes a SourceSpan and a SourceFile).

[crStiv]

@plafer hi Philippe! made some changes, what do you think abt them?

[plafer]

Left some suggestions on how to fix the problems.

Could you also fix the merge conflicts by merging in the next branch?

Also please make sure that the code compiles, as well as removing any compiler warnings (i.e. cargo check should not print any warning).

Thank you!

[plafer]

This currently doesn't compile, right? These methods don't exist on BasicBlockBuilder.

Let rewrite the method to:

pub fn local_to_absolute_addr(
    block_builder: &mut BasicBlockBuilder,
    index: u16,
    proc_ctx: &ProcedureContext,
) -> Result<(), AssemblyError> {
    let num_proc_locals = proc_ctx.num_locals();
    if num_proc_locals == 0 {
        return Err(AssemblyError::Other(
            Report::msg(
                "number of procedure locals was not set (or set to 0), but local values were used"
                    .to_string(),
            )
            .into(),
        ));
    }

    let max_index = num_proc_locals - 1;
    if index > max_index {
        let span = proc_ctx.span();
        return Err(AssemblyError::InvalidLocalMemoryIndex {
            span,
            source_file: proc_ctx.source_manager().get(span.source_id()).ok(),
            index,
            num_locals: num_proc_locals,
            max_index,
        });
    }

    push_felt(block_builder, -Felt::from(max_index - index));
    block_builder.push_op(FmpAdd);

    Ok(())
}

and adjust every caller to pass in the ProcedureContext instead of num_proc_locals

[plafer]

let's remove this as it's unused

[plafer]

Suggested change

	max: u16,
	max_index: u16,

[plafer]

Suggested change

	#[error("invalid local memory index: {index} is out of bounds (valid range: 0..{max})")]
	#[diagnostic(help("procedure has {num_locals} locals available"))]
	#[error("invalid local memory index: {index} is out of bounds")]
	#[diagnostic(help("procedure has {num_locals} locals available, so the valid range for the index is 0..{max_index}"))]

[crStiv]

@plafer made an update

[plafer]

@crStiv this branch still doesn't compile (i.e. make sure cargo build succeeds)