Skip to content

Comments

fixes lifetime issue in OperationResult::owner()#784

Merged
raviqqe merged 2 commits intomlir-rs:mainfrom
tim-hoffman:main
Jan 7, 2026
Merged

fixes lifetime issue in OperationResult::owner()#784
raviqqe merged 2 commits intomlir-rs:mainfrom
tim-hoffman:main

Conversation

@tim-hoffman
Copy link
Contributor

@tim-hoffman tim-hoffman commented Jan 7, 2026

The lifetime of the return value of OperationResult::owner() was incorrectly tied to the lifetime of &self due to not being explicitly specified. Additionally, the test case for this function did not reveal the error because it was calling BlockArgument::owner() instead!

Copilot AI review requested due to automatic review settings January 7, 2026 03:23
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a lifetime issue in the OperationResult::owner() method by properly preserving the 'a lifetime parameter that represents the parent operation's lifetime. Previously, the lifetime was being erased with '_, which lost important lifetime information.

Key Changes

  • Updated the impl block to preserve the 'a lifetime parameter instead of using '_
  • Changed the return type of owner() to properly reflect the parent operation's lifetime
  • Rewrote the test to validate OperationResult::owner() behavior instead of using BlockArgument

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


/// Returns an owner operation.
pub fn owner(&self) -> OperationRef<'c, '_> {
pub fn owner(&self) -> OperationRef<'c, 'a> {
Copy link
Member

Choose a reason for hiding this comment

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

😨

@raviqqe
Copy link
Member

raviqqe commented Jan 7, 2026

Thank you for the changes!

@raviqqe raviqqe merged commit c33b2ba into mlir-rs:main Jan 7, 2026
22 checks passed
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.

2 participants