feat: Support printing more types

[ggiraldez]

Description

Problem*

As part of our debugger implementation (see #3015) we need to extend the supported types that can be printed. We are using noirc_printable_type to keep values of variables at runtime for debug inspection.

Also resolves #4073

Summary*

This PR adds support for converting these new types:

• Tuples
• Slices: lift the restriction to print them and handle arrays with dynamic size (flagging them with a None length in the type)
• Functions: printed as an opaque <<function>> tag for now
• Mutable references: printed as an opaque <<mutable ref>> tag for now
• Unit: this is actually required to fully support function type conversion, for non-closured function references (ie. the environment is () in that case)

The PR also fixes a preexisting bug when printing multiple values using formatted strings with the first ones being structs. Since structs are expanded into tuples which take up more than one field, the printing code would fail to skip over all the fields of the struct.

Additional Context

I've been using this program to test this functionality. If it makes sense, I can add it as an integration test to test_programs/execution_success.

The program produces this output:

(0x01, 0x02, 0x03)
0xbbbb # a = (0x01, 0x02, 0x03) # 0xeeee
(0x01, 0x02, 0x03) == (0x01, 0x02, 0x03)

((0x01, 0x02, 0x03), 0x04, 0x05, 0x06)
0xbbbb # b = ((0x01, 0x02, 0x03), 0x04, 0x05, 0x06) # 0xeeee
((0x01, 0x02, 0x03), 0x04, 0x05, 0x06) == ((0x01, 0x02, 0x03), 0x04, 0x05, 0x06)

<<mutable ref>>
0xbbbb # c = <<mutable ref>> # 0xeeee

<<function>>
0xbbbb # d = <<function>> # 0xeeee

<<function>>
0xbbbb # f = <<function>> # 0xeeee

[0x01, 0x02, 0x03]
0xbbbb # g = [0x01, 0x02, 0x03] # 0xeeee
[0x01, 0x02, 0x03] == [0x01, 0x02, 0x03]

Foo { x: 555, y: 666 } == Foo { x: 555, y: 666 }

Vec { slice: [0x01, 0x02] }
0xbbbb # h = Vec { slice: [0x01, 0x02] } # 0xeeee

[0x01, 0x02]
0xbbbb # j = [0x01, 0x02] # 0xeeee
[0x01, 0x02] == [0x01, 0x02]

[printable_types] Circuit witness successfully solved

Documentation*

Check one:

• No documentation needed.
• Documentation included in this PR.
• [Exceptional Case] Documentation to be submitted in a separate PR.

PR Checklist*

• I have tested the changes locally.
• I have formatted the changes with Prettier and/or cargo fmt on default settings.

[vezenovm]

Thanks for the PR I was actually just adding Tuple types for printing and you beat me to it :).

Could you add that test under test_programs/execution_success. We also have a debug_logs test where you can add those cases.

[vezenovm]

The PR also fixes a preexisting bug when printing multiple values using formatted strings with the first ones being structs. Since structs are expanded into tuples which take up more than one field, the printing code would fail to skip over all the fields of the struct.

Is there an issue for this bug? Or did you find it on your own?

[ggiraldez]

Thanks for the PR I was actually just adding Tuple types for printing and you beat me to it :).

Could you add that test under test_programs/execution_success. We also have a debug_logs test where you can add those cases.

Oh, right! I'll try adding the new cases to debug_logs. Thanks!

[ggiraldez]

The PR also fixes a preexisting bug when printing multiple values using formatted strings with the first ones being structs. Since structs are expanded into tuples which take up more than one field, the printing code would fail to skip over all the fields of the struct.

Is there an issue for this bug? Or did you find it on your own?

I just found it while inspecting the printing code. I don't think there's an issue for it. I can create it, no problem.

[vezenovm]

What happens in the case where a type doesn't work w/ ACIR output such as slices or Vectors? Does it just panic?

[vezenovm]

Oh, right! I'll try adding the new cases to debug_logs. Thanks!

It would be great if you could post the output here as well.

[ggiraldez]

What happens in the case where a type doesn't work w/ ACIR output such as slices or Vectors? Does it just panic?

It doesn't compile. Seems related to the restriction on slices for Brillig entry points:

The application panicked (crashed).
Message:  not implemented: Unsupported slices as parameter
Location: compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs:67

[vezenovm]

It doesn't compile. Seems related to the restriction on slices for Brillig entry points:

Ah yeah this is currently unsupported as we need to know the exact number of parameters we are passing to a Brillig entry point. We could add support for slices as parameters to Brillig entry points but this is a larger change. Tbh right now I lean towards not supporting slices in the debugger if this is necessary for your work.

Being able to print in Brillig then getting a panic of "Unsupported slices as parameter" when printing in ACIR is going to be confusing to users.

[ggiraldez]

It doesn't compile. Seems related to the restriction on slices for Brillig entry points:

Ah yeah this is currently unsupported as we need to know the exact number of parameters we are passing to a Brillig entry point. We could add support for slices as parameters to Brillig entry points but this is a larger change. Tbh right now I lean towards not supporting slices in the debugger if this is necessary for your work.

Being able to print in Brillig then getting a panic of "Unsupported slices as parameter" when printing in ACIR is going to be confusing to users.

I understand. We will have a similar problem with mutable references though. In that case the compiler fails with:

The application panicked (crashed).
Message:  internal error: entered unreachable code: Expected all allocate instructions to be removed before acir_gen
Location: compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs:579

[vezenovm]

I understand. We will have a similar problem with mutable references though. In that case the compiler fails with:

Hmm I see. Then we should add a type check of some sort that will return an error if we are attempting to call println w/ mutable references or slices from ACIR.

We probably could do this check during SSA gen as a FunctionContext will store the runtime type of the function for which we are generating SSA.

[ggiraldez]

I understand. We will have a similar problem with mutable references though. In that case the compiler fails with:

Hmm I see. Then we should add a type check of some sort that will return an error if we are attempting to call println w/ mutable references or slices from ACIR.

We probably could do this check during SSA gen as a FunctionContext will store the runtime type of the function for which we are generating SSA.

If we want to limit the types for printing, then append_printable_type_info_inner looks like the best place to put the checks. This will affect both Brillig and ACIR output modes, and should remove the source of confusion.

AFAIK that code is only used when handling print/println from the stdlib. This would still allow the debugger instrumentation code to pass slices and mutable references to the oracle functions. And since we're (at least initially) planning to inject the instrumentation code only when compiling in Brillig mode, the debugger would still work for most cases.

As requested above, I filed the bug re: passing structs and multiple arguments and added the new printable types to the debug_logs sample. I put the problematic cases in a separate function, so we can disable/remove them easily if necessary.

[vezenovm]

If we want to limit the types for printing, then append_printable_type_info_inner looks like the best place to put the checks. This will affect both Brillig and ACIR output modes, and should remove the source of confusion.

AFAIK that code is only used when handling print/println from the stdlib. This would still allow the debugger instrumentation code to pass slices and mutable references to the oracle functions. And since we're (at least initially) planning to inject the instrumentation code only when compiling in Brillig mode, the debugger would still work for most cases.

As requested above, I filed the bug re: passing structs and multiple arguments and added the new printable types to the debug_logs sample. I put the problematic cases in a separate function, so we can disable/remove them easily if necessary.

Yeah currently that code is only used when handling print/println from the stdlib. I'm working on enabling assert messages to be able to print more than just strings literals so this will most likely change in the future, but continue as you have been as that work is still in flux. If you are able to add the checks and still allow the debugger instrumentation code then I am good with that. I just don't want users to see Unsupported slices as parameter rather than something like Slices are unsupported to be printed from constrained code.

Will give the code another look!

[vezenovm]

LGTM, thanks for simplifying the fmtstr execution with that bug fix as well.