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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

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.

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
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