Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: make last(empty) yield no output values #3179

Merged
merged 1 commit into from
Nov 13, 2024
Merged

Conversation

itchyny
Copy link
Contributor

@itchyny itchyny commented Sep 18, 2024

This patch fixes last/1 to produce no output value when the argument yields nothing. For example, [last(empty)] and [last(range(0))] now produce [] instead of [null]. I fixed the issue using byte-coded definition, to avoid unnecessary value boxing. This closes #1869 and closes #3177.

src/builtin.c Outdated
@@ -1879,6 +1879,32 @@ BINOPS
#undef LIBM_DD
#undef LIBM_DA

// This is a hack to make last(empty) yield no output values without using boxing.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe should say "last(g) where g does not output any value" so that it does not sound like it's a special case for exactly argument "empty"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fixed the comment.

BLOCK(gen_op_bound(LOADVN, is_empty_var),
gen_op_target(JUMP_F, if_empty),
if_empty,
gen_op_bound(LOADVN, last_var)));
Copy link
Member

@wader wader Sep 23, 2024

Choose a reason for hiding this comment

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

Trying to understand what kind of "pesudo"-jq this translates, will

last(g)

be something like (vm is not limited by scoping rules?)

( true as $is_empty
| ( g as $last
  | false as $is_empty
  | empty
  )
// if $is_empty then empty else $last end
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Closer to write with reduce;

def last(g): reduce g as $item (
  { last: null, is_empty: true };
  .last = $item | .is_empty = false
) | if .is_empty then empty else .last end;

@itchyny itchyny added this to the 1.8 release milestone Oct 28, 2024
@itchyny
Copy link
Contributor Author

itchyny commented Nov 13, 2024

Thank you.

@itchyny itchyny merged commit a7b2253 into master Nov 13, 2024
30 checks passed
@itchyny itchyny deleted the fix-last-empty branch November 13, 2024 11:35
@01mf02
Copy link

01mf02 commented Nov 14, 2024

This is a very welcome change and brings jq and jaq closer together. Thank you!

01mf02 added a commit to 01mf02/jq that referenced this pull request Nov 14, 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.

4 participants