Skip to content

Fix IR for try#13811

Merged
IanManske merged 2 commits intonushell:mainfrom
IanManske:fix-ir-try
Sep 10, 2024
Merged

Fix IR for try#13811
IanManske merged 2 commits intonushell:mainfrom
IanManske:fix-ir-try

Conversation

@IanManske
Copy link
Copy Markdown
Member

@IanManske IanManske commented Sep 8, 2024 •
edited
Loading

Description

Fixes a bug in the IR for try to match that of the regular evaluator (continuing from #13515):

# without IR:
try { ^false } catch { 'caught' } # == 'caught'

# with IR:
try { ^false } catch { 'caught' } # error, non-zero exit code

In this PR, both now evaluate to caught. For the implementation, I had to add another instruction, and feel free to suggest better alternatives. In the future, it might be possible to get rid of this extra instruction.

User-Facing Changes

Bug fix, try { ^false } catch { 'caught' } now works in IR.

@IanManske IanManske marked this pull request as ready for review September 8, 2024 17:10
@devyn
Copy link
Copy Markdown
Contributor

devyn commented Sep 9, 2024

Seems reasonable to me - this doesn't break returning a value on success from try {} though right? (Since Capture mode should cause the value to be passed through, I assume it's fine)

@IanManske
Copy link
Copy Markdown
Member Author

IanManske commented Sep 10, 2024

Yeah, pipe and capture should make the value or steam simply pass through unaffected. Couldn't find anything wrong after trying a bunch of different things, let's give it a shot.

@IanManske IanManske merged commit 493850b into nushell:main Sep 10, 2024
@IanManske IanManske added notes:fixes Include the release notes summary in the "Bug fixes" section deprecated:pr-errors Deprecated: use A:error-handling, A:error-unhelpful, or A:error-silent-fail instead labels Sep 10, 2024
@hustcer hustcer added this to the v0.98.0 milestone Sep 10, 2024
@IanManske IanManske deleted the fix-ir-try branch March 9, 2025 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@devyn devyn devyn approved these changes

Assignees

No one assigned

Labels

deprecated:pr-errors Deprecated: use A:error-handling, A:error-unhelpful, or A:error-silent-fail instead notes:fixes Include the release notes summary in the "Bug fixes" section

Projects

None yet

Milestone

v0.98.0

Development

Successfully merging this pull request may close these issues.

3 participants

@IanManske @devyn @hustcer