[osh] Test `${a[@]:-}`, `${a[*]:-}`, etc. based on the string result #2215

akinomyoga · 2025-01-02T16:36:17Z

This follows the behavioral change from Bash 4.4 to 5.0.

osh/word_eval.py

andychu

OK thank you, it looks like this improves bash compatibility so it's good

Although now I am sort of interested in "${@:-}" and so forth because it's a POSIX construct

I wonder if you can also improve test case 18, because it looks like we are missing coverage there. I think it may hit these same code paths

Or add another test case below 18

I am a bit surprised by the difference between

set -- ""
set -- "" ""

and also zsh is different

#### $@ and - and +
echo argv=${@-minus}
echo argv=${@+plus}
echo argv=${@:-minus}
echo argv=${@:+plus}

andychu · 2025-01-02T18:23:26Z

osh/word_eval.py

+                                is_falsey = False
+                                break
+                    else:
+                        is_falsey = len(strs) == 0 or (len(strs) == 1 and


Hm this seems worth documenting in doc/ref/chap-word-lang.md under op-test ?

I didn't know this !

This is actually a combination of $* / $@, double quoting, and IFS. I just calculated the width of the resulting length without actually calculating the string, which may look non-trivial. I'll add documentation and also a code comment here.

Hmm, I realized that OSH has another divergence from Bash:

$ bash -c 'IFS=; set -- "1x2" "3x4"; echo $*' 1x2 3x4 $ bin/osh -c 'IFS=; set -- "1x2" "3x4"; echo $*' 1x23x4

This is actually specified by POSIX, so all shells except OSH is consistent:

$ busybox sh -c 'IFS=; set -- "1x2" "3x4"; echo $*' 1x2 3x4 $ dash -c 'IFS=; set -- "1x2" "3x4"; echo $*' 1x2 3x4 $ ksh -c 'IFS=; set -- "1x2" "3x4"; echo $*' 1x2 3x4 $ mksh -c 'IFS=; set -- "1x2" "3x4"; echo $*' 1x2 3x4 $ zsh -c 'IFS=; set -- "1x2" "3x4"; echo $*' 1x2 3x4 $ yash -c 'IFS=; set -- "1x2" "3x4"; echo $*' 1x2 3x4

@andychu Is there a way to tell whether the currently evaluated variable substitution is inside double quotations or not? SimpleVarSub and BracedVarSub don't seem to contain the necessary context information.

I added 228ea80.

OK interesting, yeah that looks like a bug

Regarding IFS=, we have to rewrite word splitting ... long story, but you can see there are multiple bugs reported over the years in test/spec.sh word-split

This is one of the biggest remaining areas of divergence

I am not sure if this is the same bug

For $*, is the problem simply that we should ignore IFS?? We should always use a space rather than the join char? Not sure

$ dash -c 'IFS=x; set -- a b; echo $*' a b $ dash -c 'IFS=; set -- a b; echo $*' a b

Basically I am not sure that we need to see if it's double quoted or not, for this case?

But I know that is an issue with word splitting in general. We have a complex set of data structures like Piece that know about quoting

Basically I am not sure that we need to see if it's double quoted or not, for this case?

I think so (edit: I added a TODO and a failing test in commit 7ea2d8b). POSIX says (list formatting is added by me):

$*

Expands to the positional parameters, starting from one, initially producing one field for each positional parameter that is set.

When the expansion occurs in a context where field splitting will be performed, any empty fields may be discarded and each of the non-empty fields shall be further split as described in 2.6.5 Field Splitting.

When the expansion occurs in a context where field splitting will not be performed, the initial fields shall be joined to form a single field with the value of each parameter separated by the first character of the IFS variable if IFS contains at least one character, or separated by a if IFS is unset, or with no separation if IFS is set to a null string.

So, IFS is used only in the context where field splitting (word splitting) is inactive. OSH now uses IFS regardless of the context, which cause the deviation.

But I know that is an issue with word splitting in general. We have a complex set of data structures like Piece that know about quoting

I think we can include the context information in SimpleVarSub and BracedVarSub at the parser or lexer stage (instead of analyzing the AST on the evaluation time); i.e., $* in echo "$*" and echo $* would be treated as different constructs in the parser.

I think a simple solution may be to convert $* to $@ when $* is used in a context where field splitting is active. It seems $* and $@ are functionally equivalent in the context where field splitting is performed.

andychu · 2025-01-02T18:25:42Z

spec/var-op-test.test.sh

+# a   : 'no-colon' 'with-colon'
+# a[0]: 'no-colon' 'with-colon'
+# a[@]: 'no-colon' 'with-colon'
+# a[*]: 'no-colon' 'with-colon'


I think it could be useful to split this test up like the other one, since bash agrees with OSH on the first part

Although it is a bit annoying because you have to share test-hyphen

I suppose that could be source spec/testdata/var-op-test-helper.sh or something

Or I guess it is OK to copy and paste too, since the function is short

~~361ac8c~~ 82d75cc

akinomyoga · 2025-01-03T00:59:10Z

I wonder if you can also improve test case 18, because it looks like we are missing coverage there. I think it may hit these same code paths

Or add another test case below 18

I added a set of tests in commit 655d14d.

This follows the behavioral change from Bash 4.4 to 5.0.

andychu

OK thanks, this looks good ... I guess the main point is that is an array with 1 value of empty string is considered empty

Thanks for adding the docs

I am going to adjust them a bit, and also adjust the test failures

osh/word_eval.py

andychu · 2025-01-03T16:51:12Z

spec/var-op-test.test.sh

+argv=
+argv=plus
+## END
+## BUG mksh/osh STDOUT:


I usually let OSH fail and adjust allowed_failures

i.e. we don't usually have BUG osh, which is yellow - we make it RED

This is basically a matter of convention

I actually wondered if it would be possible to add another type of confirmed failure (e.g. "TODO") that produces red characters (or characters of another color) in the table. With the current strategy,

When the number of allowed failures is increased by some changes, we cannot identify which failure was the increased one. We need to check out an older commit, build it, and compare the test results manually. Or one needs to open the github-jobs website, looking up the corresponding page, and compare the results.

Even when the problem is fixed in the process of implementing another feature, the failure can be left to be FAIL if the result does not exactly match the expected result (which probably describes the Bash behavior).

Hm well technically ALL the ~100 failures now are TODO ! :-) Some of them will be classified as "OK osh", but never "BUG osh" I think ... our bugs are just things we haven't done yet

And notice there are ZERO FAIL on bash/dash/mksh/zsh ... so FAIL is really reserved for OSH

and BUG is reserved for the other shells

At least that is the convention I follow, but we can talk about it more if it causes a problem

I'm not sure if I could call it "a problem", but at least the following situation can be improved:

When the number of allowed failures is increased by some changes, we cannot identify which failure was the increased one. We need to check out an older commit, build it, and compare the test results manually. Or one needs to open the github-jobs website, looking up the corresponding page, and compare the results.

Even when the problem is fixed in the process of implementing another feature, the failure can be left to be FAIL if the result does not exactly match the expected result (which probably describes the Bash behavior).

doc/ref/chap-word-lang.md

andychu · 2025-01-03T16:55:22Z

doc/ref/chap-word-lang.md

+When the variable substituion is inside double quotations, the `[*]` form joins
+the elements by the first character of `IFS`:
+
+    $ printf '<%s>\n' "${a[*]}"


I think these cases can more concisely explained by a table, with 4 entries or 8 entries, like:

$@
"$@"
$*
"$*"

andychu · 2025-01-03T17:51:55Z

OK thanks

I also fixed the ${*:-} bug on top of this - 31ce9f6

Thanks for finding it and writing the tests - that is the hard part!

akinomyoga · 2025-01-04T07:16:49Z

Thank you!

akinomyoga commented Jan 2, 2025

View reviewed changes

osh/word_eval.py Outdated Show resolved Hide resolved

akinomyoga force-pushed the array-op-test branch from 9145e4f to fba1079 Compare January 2, 2025 16:51

andychu reviewed Jan 2, 2025

View reviewed changes

akinomyoga force-pushed the array-op-test branch from 361ac8c to 655d14d Compare January 3, 2025 00:57

akinomyoga force-pushed the array-op-test branch from 8ccf487 to 14014d6 Compare January 3, 2025 01:56

akinomyoga added 6 commits January 3, 2025 10:59

[osh] Test ${a[@]:-}, ${a[*]:-}, etc. based on the string result

b73e819

This follows the behavioral change from Bash 4.4 to 5.0.

[spec/array] Disallow array#42 failure for OSH as it is now supported

ea123c2

[doc/ref/cha-word-lang] Describe ${a[*]} and ${a[@]}

31627ae

[spec/var-op-test] Split test cases var-op-test#{28,29}

fd2810e

[spec/var-op-test] Extend tests for ${@:-}

8c23463

[osh] Comment on unquoted ${a[*]:-empty} with empty IFS

7ea2d8b

akinomyoga force-pushed the array-op-test branch from 14014d6 to 7ea2d8b Compare January 3, 2025 01:59

akinomyoga changed the base branch from master to soil-staging January 3, 2025 14:08

andychu reviewed Jan 3, 2025

View reviewed changes

Andy C added 3 commits January 3, 2025 12:01

[spec/var-op-test] Let OSH fail so we remember to fix it

7c7a663

[doc/ref] Polish help topics a bit

1b96798

More polish

d140554

andychu merged commit c326e7c into soil-staging Jan 3, 2025
18 checks passed

akinomyoga deleted the array-op-test branch January 3, 2025 22:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[osh] Test `${a[@]:-}`, `${a[*]:-}`, etc. based on the string result #2215

[osh] Test `${a[@]:-}`, `${a[*]:-}`, etc. based on the string result #2215

akinomyoga commented Jan 2, 2025

andychu left a comment

andychu Jan 2, 2025

akinomyoga Jan 2, 2025

akinomyoga Jan 2, 2025 •

edited

Loading

akinomyoga Jan 3, 2025

andychu Jan 3, 2025

andychu Jan 3, 2025

akinomyoga Jan 3, 2025 •

edited

Loading

akinomyoga Jan 3, 2025

andychu Jan 2, 2025

akinomyoga Jan 3, 2025 •

edited

Loading

akinomyoga commented Jan 3, 2025

andychu left a comment

andychu Jan 3, 2025

akinomyoga Jan 4, 2025

andychu Jan 4, 2025

akinomyoga Jan 4, 2025

andychu Jan 3, 2025

andychu commented Jan 3, 2025

akinomyoga commented Jan 4, 2025

[osh] Test ${a[@]:-}, ${a[*]:-}, etc. based on the string result #2215

[osh] Test ${a[@]:-}, ${a[*]:-}, etc. based on the string result #2215

Conversation

akinomyoga commented Jan 2, 2025

andychu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akinomyoga Jan 2, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akinomyoga Jan 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akinomyoga Jan 3, 2025 • edited Loading

Choose a reason for hiding this comment

akinomyoga commented Jan 3, 2025

andychu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andychu commented Jan 3, 2025

akinomyoga commented Jan 4, 2025

[osh] Test `${a[@]:-}`, `${a[*]:-}`, etc. based on the string result #2215

[osh] Test `${a[@]:-}`, `${a[*]:-}`, etc. based on the string result #2215

akinomyoga Jan 2, 2025 •

edited

Loading

akinomyoga Jan 3, 2025 •

edited

Loading

akinomyoga Jan 3, 2025 •

edited

Loading