Skip to content

feat!: always print Type::Constant kinds#6080

Closed
michaeljklein wants to merge 2 commits intomasterfrom
michaeljklein/print-numeric-kinds
Closed

feat!: always print Type::Constant kinds#6080
michaeljklein wants to merge 2 commits intomasterfrom
michaeljklein/print-numeric-kinds

Conversation

@michaeljklein
Copy link
Contributor

Description

Problem*

Instead of printing:

Expected type [RootParityInput; 4)], found type [RootParityInput; 4)]

We print:

Expected type [RootParityInput; (4: u32)], found type [RootParityInput; (4: Field)]

Summary*

Additional Context

Documentation*

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [For Experimental Features] 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.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 18, 2024

Changes to Brillig bytecode sizes

Generated at commit: e56f2c4cfb5c169a15358302d9d4dd0bf4cac948, compared to commit: 426f2955cbe4f086581d05eea7d06c47e0491195

There are no changes in circuit sizes

Copy link
Contributor

@jfecher jfecher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should almost never print constant kinds, this is the same formatting which broke #6077.

We need to either only print these when they differ or use a different error type entirely for this case. We should see what Rust does here.

@jfecher
Copy link
Contributor

jfecher commented Sep 18, 2024

For:

struct Foo<const N: usize> {
    x: [u8; N],
}

fn main() {}

fn bar<const M: u32>() -> Foo<M> {
    todo!()
}

Rust gives:

error: the constant `M` is not of type `usize`
 --> t.rs:8:27
  |
8 | fn bar<const M: u32>() -> Foo<M> {
  |                           ^^^^^^ expected `usize`, found `u32`
  |
note: required by a bound in `Foo`
 --> t.rs:2:12
  |
2 | struct Foo<const N: usize> {
  |            ^^^^^^^^^^^^^^ required by this bound in `Foo`

@michaeljklein
Copy link
Contributor Author

The original issue appears to be resolved by #6083

auto-merge was automatically disabled September 18, 2024 21:01

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants