refactor!: rework extensions interface #119
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #119 +/- ##
==========================================
+ Coverage 83.42% 85.22% +1.80%
==========================================
Files 27 31 +4
Lines 4164 4184 +20
Branches 4164 4184 +20
==========================================
+ Hits 3474 3566 +92
+ Misses 368 297 -71
+ Partials 322 321 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
aborgna-q
approved these changes
Oct 9, 2024
This was
linked to
issues
Oct 10, 2024
Co-authored-by: Agustín Borgna <121866228+aborgna-q@users.noreply.github.com>
|
Thanks Agustin. I'm going to leave this unmerged for at least today to give others a chance to suggest improvements |
Merged
github-merge-queue bot
pushed a commit
that referenced
this pull request
Oct 21, 2024
## 🤖 New release * `hugr-llvm`: 0.5.1 -> 0.6.0 (⚠️ API breaking changes) ###⚠️ `hugr-llvm` breaking changes ``` --- failure auto_trait_impl_removed: auto trait no longer implemented --- Description: A public type has stopped implementing one or more auto traits. This can break downstream code that depends on the traits being implemented. ref: https://doc.rust-lang.org/reference/special-types-and-traits.html#auto-traits impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.36.0/src/lints/auto_trait_impl_removed.ron Failed in: type TypeConverter is no longer UnwindSafe, in /tmp/.tmpGjujM0/hugr-llvm/src/types.rs:59 type TypeConverter is no longer RefUnwindSafe, in /tmp/.tmpGjujM0/hugr-llvm/src/types.rs:59 --- failure derive_trait_impl_removed: built-in derived trait no longer implemented --- Description: A public type has stopped deriving one or more traits. This can break downstream code that depends on those types implementing those traits. ref: https://doc.rust-lang.org/reference/attributes/derive.html#derive impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.36.0/src/lints/derive_trait_impl_removed.ron Failed in: type TypeConverter no longer derives Copy, in /tmp/.tmpGjujM0/hugr-llvm/src/types.rs:59 type TypeConverter no longer derives Clone, in /tmp/.tmpGjujM0/hugr-llvm/src/types.rs:59 type TypeConverter no longer derives Debug, in /tmp/.tmpGjujM0/hugr-llvm/src/types.rs:59 --- failure function_missing: pub fn removed or renamed --- Description: A publicly-visible function cannot be imported by its prior path. A `pub use` may have been removed, or the function 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.36.0/src/lints/function_missing.ron Failed in: function hugr_llvm::custom::prelude::array::emit_array_op, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom/prelude/array.rs:51 function hugr_llvm::custom::int::add_int_extensions, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom/int.rs:226 function hugr_llvm::custom::rotation::add_rotation_extensions, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom/rotation.rs:196 function hugr_llvm::custom::prelude::add_prelude_extensions, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom/prelude.rs:295 function hugr_llvm::custom::float::add_float_extensions, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom/float.rs:186 function hugr_llvm::custom::conversions::add_conversions_extension, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom/conversions.rs:270 function hugr_llvm::custom::logic::add_logic_extensions, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom/logic.rs:106 function hugr_llvm::custom::prelude::add_default_prelude_extensions, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom/prelude.rs:304 --- 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.36.0/src/lints/inherent_method_missing.ron Failed in: TypeConverter::new, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/types.rs:107 TypeConverter::iw_context, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/types.rs:112 CodegenExtsMap::add_float_extensions, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom/float.rs:192 CodegenExtsMap::add_int_extensions, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom/int.rs:234 CodegenExtsMap::add_logic_extensions, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom/logic.rs:113 CodegenExtsMap::add_default_prelude_extensions, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom/prelude.rs:311 CodegenExtsMap::add_prelude_extensions, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom/prelude.rs:317 CodegenExtsMap::add_rotation_extensions, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom/rotation.rs:201 CodegenExtsMap::new, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom.rs:88 CodegenExtsMap::add_cge, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom.rs:97 CodegenExtsMap::get, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom.rs:110 CodegenExtsMap::llvm_type, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom.rs:120 CodegenExtsMap::emit, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom.rs:130 CodegenExtsMap::load_constant, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom.rs:145 --- failure module_missing: pub module removed or renamed --- Description: A publicly-visible module cannot be imported by its prior path. A `pub use` may have been removed, or the module may have been renamed, removed, or made non-public. ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.36.0/src/lints/module_missing.ron Failed in: mod hugr_llvm::custom::conversions, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom/conversions.rs:1 mod hugr_llvm::custom::logic, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom/logic.rs:1 mod hugr_llvm::custom::int, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom/int.rs:1 mod hugr_llvm::custom::prelude, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom/prelude.rs:1 mod hugr_llvm::fat, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/fat.rs:1 mod hugr_llvm::custom::float, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom/float.rs:1 mod hugr_llvm::custom::rotation, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom/rotation.rs:1 mod hugr_llvm::custom::prelude::array, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom/prelude/array.rs:1 --- failure struct_missing: pub struct removed or renamed --- Description: A publicly-visible struct cannot be imported by its prior path. A `pub use` may have been removed, or the struct 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.36.0/src/lints/struct_missing.ron Failed in: struct hugr_llvm::custom::logic::LogicCodegenExtension, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom/logic.rs:78 struct hugr_llvm::fat::FatNode, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/fat.rs:28 struct hugr_llvm::custom::int::IntOpsCodegenExtension, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom/int.rs:120 struct hugr_llvm::custom::prelude::PreludeCodegenExtension, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom/prelude.rs:174 struct hugr_llvm::custom::float::FloatTypesCodegenExtension, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom/float.rs:27 struct hugr_llvm::emit::NullEmitLlvm, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/emit.rs:52 struct hugr_llvm::custom::conversions::ConversionsCodegenExtension, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom/conversions.rs:244 struct hugr_llvm::custom::prelude::DefaultPreludeCodegen, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom/prelude.rs:169 struct hugr_llvm::custom::int::IntTypesCodegenExtension, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom/int.rs:150 struct hugr_llvm::custom::rotation::RotationCodegenExtension, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom/rotation.rs:30 --- failure trait_method_added: pub trait method added --- Description: A non-sealed public trait added a new method without a default implementation, which breaks downstream implementations of the trait ref: https://doc.rust-lang.org/cargo/reference/semver.html#trait-new-item-no-default impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.36.0/src/lints/trait_method_added.ron Failed in: trait method hugr_llvm::custom::CodegenExtension::add_extension in file /tmp/.tmpGjujM0/hugr-llvm/src/custom.rs:41 --- failure trait_method_missing: pub trait method removed or renamed --- Description: A trait method is no longer callable, and may have been renamed or removed entirely. ref: https://doc.rust-lang.org/cargo/reference/semver.html#major-any-change-to-trait-item-signatures impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.36.0/src/lints/trait_method_missing.ron Failed in: method extension of trait CodegenExtension, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom.rs:38 method supported_consts of trait CodegenExtension, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom.rs:44 method llvm_type of trait CodegenExtension, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom.rs:50 method emitter of trait CodegenExtension, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom.rs:58 method load_constant of trait CodegenExtension, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom.rs:68 --- failure trait_missing: pub trait removed or renamed --- Description: A publicly-visible trait cannot be imported by its prior path. A `pub use` may have been removed, or the trait 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.36.0/src/lints/trait_missing.ron Failed in: trait hugr_llvm::emit::EmitOp, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/emit.rs:45 trait hugr_llvm::custom::prelude::PreludeCodegen, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/custom/prelude.rs:52 trait hugr_llvm::fat::FatExt, previously in file /tmp/.tmpu0X9jI/hugr-llvm/src/fat.rs:313 --- failure trait_no_longer_object_safe: trait no longer object safe --- Description: Trait is no longer object safe, which breaks `dyn Trait` usage. ref: https://doc.rust-lang.org/stable/reference/items/traits.html#object-safety impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.36.0/src/lints/trait_no_longer_object_safe.ron Failed in: trait CodegenExtension in file /tmp/.tmpGjujM0/hugr-llvm/src/custom.rs:38 ``` <details><summary><i><b>Changelog</b></i></summary><p> <blockquote> ## [0.6.0](v0.5.1...v0.6.0) - 2024-10-21 ### Bug Fixes - Conversion operations having poison results ([#131](#131)) ### New Features - [**breaking**] Allow extension callbacks to have non-`'static` lifetimes ([#128](#128)) - [**breaking**] Support `tket2.rotation.from_halfturns_unchecked` ([#133](#133)) ### Refactor - [**breaking**] remove trait emit op ([#104](#104)) - [**breaking**] rework extensions interface ([#119](#119)) - [**breaking**] move packaged extensions from `crate::custom` to `crate::extension` ([#126](#126)) </blockquote> </p></details> --- This PR was generated with [release-plz](https://github.com/MarcoIeni/release-plz/).
github-merge-queue bot
pushed a commit
that referenced
this pull request
Nov 13, 2024
Follow up to #119
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
CodegenExtensionis replaced with a new trait of the same name, that instead of defining extension behaviour provides a convenient interface for registering a set of extensions. Thepreludeextension gives the best example, exercising all the features.Extensions are now registered via methods on
CodegenExtsBuilder, each method takes a callback that is stored in the builder. The builder is consumed byfinishwhich returns a collection of "Callback Maps". One of the big wins here is that the "Type Callback Map" does not depend on aH: HugrViewparameter.The "root"
&'c Contextnow lives inEmitModuleContext, as well as inTypingSession.TypeConverteris now the "Type Callback Map". This is implemented using the newsrc/utils/type_map.rs, which we intend to leverage for computing "def-hooks" and "destructors" for types.We go to some effort to ensure the callbacks are not required to have
'staticlifetimes, nothing insidecrate::customassumes or requires this. However, we do in several places (e.g.TypingSession,EmitModuleContext) fix these lifetimes to'static, in lieu of adding additional lifetime parameters to these types.We have removed the
guppy.rstest case, which was ignored, rather than rework it to use the new interface.Otherwise, test cases are largely unchanged, and there are no snapshot changes.
All extensions have been changed to use the new interface, which is largely just moving code around. In some cases error messages are consolidated by the new tools providing more appropriate inputs to extension points. See for example
CodegenExtsBuilder::custom_const.At several points we mark TODO refactorings that will be carried out in the near future.
BREAKING CHANGE: extensions interface in
src/custom.rsentirely reworked.