Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Print renders incorrect values when passing multiple structs #4073

Closed
ggiraldez opened this issue Jan 17, 2024 · 0 comments · Fixed by #4071
Closed

Print renders incorrect values when passing multiple structs #4073

ggiraldez opened this issue Jan 17, 2024 · 0 comments · Fixed by #4071
Labels
bug Something isn't working

Comments

@ggiraldez
Copy link
Contributor

Aim

Given the following program:

struct Foo {
  x: u32,
  y: u32,
}

fn main() {
  let foo = Foo { x: 1, y: 2 };

  dep::std::println(f"{foo} == {foo}");
}

Expected Behavior

Should print

Foo { x: 1, y: 2 } == Foo { x: 1, y: 2 }

Bug

But instead prints

Foo { x: 1, y: 2 } == Foo { x: 2, y: 1 }

To Reproduce

Run the program.

Installation Method

None

Nargo Version

No response

Additional Context

This happens because the code to extract the values to print using a formatted string fail to take into account that the structs are expanded into a tuple occupying multiple fields. When decoding the second value, the first value retrieved corresponds to the overall second value, which in the example given is the second field of the first value struct.

Would you like to submit a PR for this Issue?

No

Support Needs

No response

@ggiraldez ggiraldez added the bug Something isn't working label Jan 17, 2024
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir Jan 17, 2024
github-merge-queue bot pushed a commit that referenced this issue Jan 18, 2024
# 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](https://gist.github.com/ggiraldez/e3709def6c26e7585d12002fc8a0a216)
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:
- [X] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[Exceptional Case]** Documentation to be submitted in a separate
PR.

# PR Checklist\*

- [X] I have tested the changes locally.
- [X] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: synthia <[email protected]>
@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Noir Jan 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant