Skip to content

Conversation

@aborgna-q
Copy link
Collaborator

@aborgna-q aborgna-q commented Dec 17, 2024

Updates the Display implementation of poly functions, functions, types, type arguments, and type params so its easier to read.

Comparison below. I added the newlines manually.

Before:

forall Any _0.map(|i|i.to_string()).unwrap_or("-".to_string()).
    [array([
        Variable { v: TypeArgVariable { idx: 1, cached_decl: BoundedNat { bound: UpperBound(None) } } },
        Type { ty: TypeBase(Sum(General { rows: [TypeRowBase { types: [] }, TypeRowBase { types: [TypeBase(Variable(0, Any), Any)] }] }), Any) }
    ])]
 -> [[]][[
    array([
            Variable { v: TypeArgVariable { idx: 1, cached_decl: BoundedNat { bound: UpperBound(None) } } },
            Type { ty: TypeBase(Sum(General { rows: [TypeRowBase { types: [] }, TypeRowBase { types: [TypeBase(Variable(0, Any), Any)] }] }), Any) }
        ]),
        int([BoundedNat { n: 6 }])
    ]]

(yes, that is some hardcoded rust source)

After:

 (var#0 : Type) (var#1 : Nat). [array(var#1, Type([]+[var#0]))]
                             -> [[array(var#1, Type([]+[var#0])), int(6)]]

That is, a generic function on a type var and an unbounded natural that takes an array of (nat, option type) and returns a tuple with the array and an int.

It may be better if the tuple display used () instead of [], but that can be discussed later.

Update after review suggestions:

 (#0 : Type) (#1 : Nat). [array(#1, []+[#0])] -> [[array(#1, []+[#0]), int(6)]]

@aborgna-q aborgna-q requested a review from a team as a code owner December 17, 2024 14:23
@aborgna-q aborgna-q requested review from acl-cqc and mark-koch and removed request for acl-cqc December 17, 2024 14:23
@aborgna-q
Copy link
Collaborator Author

I could also remove the var part of the variables, and just call them #42 (and/or use some other reserved character)

@codecov
Copy link

codecov bot commented Dec 17, 2024

Codecov Report

Attention: Patch coverage is 27.27273% with 16 lines in your changes missing coverage. Please review.

Project coverage is 86.58%. Comparing base (8a3d98e) to head (89dbae0).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
hugr-core/src/types/poly_func.rs 22.22% 7 Missing ⚠️
hugr-core/src/types/custom.rs 33.33% 0 Missing and 4 partials ⚠️
hugr-core/src/types/signature.rs 25.00% 1 Missing and 2 partials ⚠️
hugr-core/src/types/type_param.rs 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1802      +/-   ##
==========================================
- Coverage   86.59%   86.58%   -0.02%     
==========================================
  Files         188      188              
  Lines       34227    34234       +7     
  Branches    31098    31105       +7     
==========================================
  Hits        29640    29640              
- Misses       2900     2906       +6     
- Partials     1687     1688       +1     
Flag Coverage Δ
python 92.58% <ø> (ø)
rust 85.97% <27.27%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@aborgna-q aborgna-q force-pushed the ab/nicer-type-printing branch from f694de3 to 782506d Compare December 17, 2024 14:45
Copy link
Contributor

@mark-koch mark-koch left a comment

Choose a reason for hiding this comment

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

Very nice, this is a lot more readable!

/// A type variable, defined by an index into a list of type parameters.
//
// We cache the TypeBound here (checked in validation)
#[display("var#{_0}")]
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer to get rid of the var part

#[serde(tag = "tya")]
pub enum TypeArg {
/// Where the (Type/Op)Def declares that an argument is a [TypeParam::Type]
#[display("Type({ty})")]
Copy link
Contributor

Choose a reason for hiding this comment

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

Or just {ty}?

@aborgna-q aborgna-q enabled auto-merge December 17, 2024 14:57
@aborgna-q aborgna-q added this pull request to the merge queue Dec 17, 2024
Merged via the queue into main with commit b310fac Dec 17, 2024
23 of 24 checks passed
@aborgna-q aborgna-q deleted the ab/nicer-type-printing branch December 17, 2024 15:03
@hugrbot hugrbot mentioned this pull request Dec 17, 2024
github-merge-queue bot pushed a commit that referenced this pull request Dec 18, 2024
## 🤖 New release
* `hugr`: 0.14.0 -> 0.14.1 (✓ API compatible changes)
* `hugr-core`: 0.14.0 -> 0.14.1 (✓ API compatible changes)
* `hugr-model`: 0.15.0 -> 0.16.0 (⚠️ API breaking changes)
* `hugr-llvm`: 0.14.0 -> 0.14.1 (✓ API compatible changes)
* `hugr-passes`: 0.14.0 -> 0.14.1 (✓ API compatible changes)
* `hugr-cli`: 0.14.0 -> 0.14.1 (✓ API compatible changes)

### ⚠️ `hugr-model` breaking changes

```
--- failure constructible_struct_adds_field: externally-constructible struct adds field ---

Description:
A pub struct constructible with a struct literal has a new pub field. Existing struct literals must be updated to include the new field.
        ref: https://doc.rust-lang.org/reference/expressions/struct-expr.html
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.37.0/src/lints/constructible_struct_adds_field.ron

Failed in:
  field Region.scope in /tmp/.tmpN5f4vM/hugr/hugr-model/src/v0/mod.rs:414
  field LinkId.1 in /tmp/.tmpN5f4vM/hugr/hugr-model/src/v0/mod.rs:162

--- failure enum_missing: pub enum removed or renamed ---

Description:
A publicly-visible enum cannot be imported by its prior path. A `pub use` may have been removed, or the enum itself may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.37.0/src/lints/enum_missing.ron

Failed in:
  enum hugr_model::v0::GlobalRef, previously in file /tmp/.tmp2gotdW/hugr-model/src/v0/mod.rs:456
  enum hugr_model::v0::LocalRef, previously in file /tmp/.tmp2gotdW/hugr-model/src/v0/mod.rs:476
  enum hugr_model::v0::LinkRef, previously in file /tmp/.tmp2gotdW/hugr-model/src/v0/mod.rs:494

--- failure enum_struct_variant_field_added: pub enum struct variant field added ---

Description:
An enum's exhaustive struct variant has a new field, which has to be included when constructing or matching on this variant.
        ref: https://doc.rust-lang.org/reference/attributes/type_system.html#the-non_exhaustive-attribute
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.37.0/src/lints/enum_struct_variant_field_added.ron

Failed in:
  field symbol of variant Term::Apply in /tmp/.tmpN5f4vM/hugr/hugr-model/src/v0/mod.rs:544
  field symbol of variant Term::ApplyFull in /tmp/.tmpN5f4vM/hugr/hugr-model/src/v0/mod.rs:557

--- failure enum_struct_variant_field_missing: pub enum struct variant's field removed or renamed ---

Description:
A publicly-visible enum has a struct variant whose field is no longer available under its prior name. It may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.37.0/src/lints/enum_struct_variant_field_missing.ron

Failed in:
  field global of variant Term::Apply, previously in file /tmp/.tmp2gotdW/hugr-model/src/v0/mod.rs:544
  field global of variant Term::ApplyFull, previously in file /tmp/.tmp2gotdW/hugr-model/src/v0/mod.rs:557

--- failure enum_variant_added: enum variant added on exhaustive enum ---

Description:
A publicly-visible enum without #[non_exhaustive] has a new variant.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#enum-variant-new
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.37.0/src/lints/enum_variant_added.ron

Failed in:
  variant ModelError:InvalidVar in /tmp/.tmpN5f4vM/hugr/hugr-model/src/v0/mod.rs:723
  variant ModelError:InvalidSymbol in /tmp/.tmpN5f4vM/hugr/hugr-model/src/v0/mod.rs:726
  variant Operation:Import in /tmp/.tmpN5f4vM/hugr/hugr-model/src/v0/mod.rs:374

--- failure enum_variant_missing: pub enum variant removed or renamed ---

Description:
A publicly-visible enum has at least one variant that is no longer available under its prior name. It may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.37.0/src/lints/enum_variant_missing.ron

Failed in:
  variant ModelError::InvalidLocal, previously in file /tmp/.tmp2gotdW/hugr-model/src/v0/mod.rs:723
  variant ModelError::InvalidGlobal, previously in file /tmp/.tmp2gotdW/hugr-model/src/v0/mod.rs:727

--- failure inherent_method_missing: pub method removed or renamed ---

Description:
A publicly-visible method or associated fn is no longer available under its prior name. It may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.37.0/src/lints/inherent_method_missing.ron

Failed in:
  LinkId::new, previously in file /tmp/.tmp2gotdW/hugr-model/src/v0/mod.rs:140
  LinkId::index, previously in file /tmp/.tmp2gotdW/hugr-model/src/v0/mod.rs:140
  LinkId::unwrap_slice, previously in file /tmp/.tmp2gotdW/hugr-model/src/v0/mod.rs:140
  LinkId::wrap_slice, previously in file /tmp/.tmp2gotdW/hugr-model/src/v0/mod.rs:140

--- failure struct_repr_transparent_removed: struct repr(transparent) removed ---

Description:
repr(transparent) was removed from a struct whose layout was part of the public ABI. This can cause its memory layout to change, breaking FFI use cases.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#repr-transparent-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.37.0/src/lints/struct_repr_transparent_removed.ron

Failed in:
  struct LinkId in /tmp/.tmpN5f4vM/hugr/hugr-model/src/v0/mod.rs:162
```

<details><summary><i><b>Changelog</b></i></summary><p>

## `hugr`
<blockquote>

##
[0.14.1](hugr-v0.14.0...hugr-v0.14.1)
- 2024-12-18

### Bug Fixes

- Constant folding now tolerates root nodes without input/output nodes
(#1799)
- `Call` ops not tracking their parameter extensions (#1805)

### New Features

- add MonomorphizePass and deprecate monomorphize (#1809)
- Lower LoadNat to LLVM (#1801)
- Cleanup `Display` of types and arguments (#1802)
- add ArrayValue to python, rust and lowering (#1773)
- Scoping rules and utilities for symbols, links and variables (#1754)
</blockquote>

## `hugr-core`
<blockquote>

##
[0.14.1](hugr-core-v0.14.0...hugr-core-v0.14.1)
- 2024-12-18

### Bug Fixes

- `Call` ops not tracking their parameter extensions (#1805)

### New Features

- Lower LoadNat to LLVM (#1801)
- Cleanup `Display` of types and arguments (#1802)
- add ArrayValue to python, rust and lowering (#1773)
- Scoping rules and utilities for symbols, links and variables (#1754)
</blockquote>

## `hugr-model`
<blockquote>

##
[0.16.0](hugr-model-v0.15.0...hugr-model-v0.16.0)
- 2024-12-18

### New Features

- Scoping rules and utilities for symbols, links and variables (#1754)
</blockquote>

## `hugr-llvm`
<blockquote>

##
[0.14.1](hugr-llvm-v0.14.0...hugr-llvm-v0.14.1)
- 2024-12-18

### Bug Fixes

- Add LLVM lowering for `logic.Not` (#1812)

### New Features

- Lower LoadNat to LLVM (#1801)
- add ArrayValue to python, rust and lowering (#1773)
</blockquote>

## `hugr-passes`
<blockquote>

##
[0.14.1](hugr-passes-v0.14.0...hugr-passes-v0.14.1)
- 2024-12-18

### Bug Fixes

- Constant folding now tolerates root nodes without input/output nodes
(#1799)

### New Features

- Cleanup `Display` of types and arguments (#1802)
- add MonomorphizePass and deprecate monomorphize (#1809)
</blockquote>

## `hugr-cli`
<blockquote>

##
[0.14.1](hugr-cli-v0.14.0...hugr-cli-v0.14.1)
- 2024-12-18

### New Features

- Print `hugr-cli`'s correct version when using '--version' (#1790)
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/release-plz/release-plz/).
@hugrbot hugrbot mentioned this pull request Dec 19, 2024
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