Reincorporate Printf4J as AwkPrintf with full AWK printf semantics - #543
Reincorporate Printf4J as AwkPrintf with full AWK printf semantics#543bertysentry wants to merge 14 commits into
Conversation
Replace the external Printf4J dependency (which emulated glibc's printf()) with a new io.jawk.jrt.AwkPrintf class implementing POSIX AWK / gawk printf semantics, verified against gawk 5: - %s converts numbers with AWK's number-to-string rules: integral values print without a fractional part (fixes the "x[1.0]" symptom), and non-integral values honor the script's current CONVFMT, which is now threaded through new AwkSink.printfWithConvFmt() / sprintfWithConvFmt() methods (backward-compatible defaults). - %c prints the character of a numeric code point (including numeric strings and fields) or the first character of a string value. - Dynamic star precision (%.*f), negative star width/precision, gawk positional specifiers (%2$s), and the ' grouping flag are supported. - Out-of-range integers follow gawk: unsigned 64-bit wrapping for %u/%o/%x/%X, full decimal expansion for %d/%i beyond 64 bits, %g fallback for %u/%o/%x/%X beyond 64 bits. - NaN and infinities print as nan/inf/-inf everywhere. - %e/%f/%g round halfway cases to even via BigDecimal, matching the C library used by gawk; %g strips trailing zeros before padding. - printf with too few arguments is a fatal error, like gawk; unknown conversions (including %n and invalid length modifiers) print verbatim without consuming an argument; a single h/l/L modifier is accepted and ignored. Also stop saturating integral values beyond 2^63-1 to Long.MAX_VALUE in constant folding, int(), and number-to-string conversion: print 2^100 now prints the full decimal expansion like gawk. The complete Printf4J unit test suite is incorporated in AwkPrintfTest, including the tests that were disabled or commented out there, with expectations adapted to gawk-verified AWK semantics; PrintfTest adds script-level coverage, and the previously-skipped POSIX star width/precision conformance test is enabled. Fixes #528 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 213890119c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| import static org.junit.Assert.assertEquals; | ||
| import static org.junit.Assert.assertThrows; | ||
| import static io.jawk.jrt.AwkPrintf.sprintf; |
There was a problem hiding this comment.
This newly added suite invokes AwkPrintf.sprintf and JUnit assertions directly throughout, bypassing the repository-mandated AwkTestSupport builders and their standardized Jawk setup and assertion flow. Convert these cases to the required helper-based structure rather than importing sprintf directly.
AGENTS.md reference: AGENTS.md:L23-L26
Useful? React with 👍 / 👎.
| appendPadded( | ||
| floatBody('g', new Flags(false, false, false, false, false, false), -1, d), | ||
| flags.leftJustify, | ||
| false, | ||
| width); |
There was a problem hiding this comment.
When %u, %o, %x, or %X receives a value outside the unsigned 64-bit range, this fallback formats abs(d) with a fresh flag set and default precision. Consequently negative values lose their sign (printf "%u", -(2^100) produces 1.26765e+30 instead of -1.26765e+30), and formats such as %#.3x ignore their precision and alternate form. The %g fallback needs to preserve the applicable sign, flags, precision, and width behavior.
Useful? React with 👍 / 👎.
| if (exponent >= -4 && exponent < precision) { | ||
| String s = decimalString(rounded.setScale(precision - 1 - exponent, RoundingMode.UNNECESSARY)); | ||
| return alternate ? s : stripTrailingFractionZeros(s); |
There was a problem hiding this comment.
For %#g/%#G with effective precision 1, setScale(0) produces a string without a decimal separator and the alternate branch returns it unchanged. Thus sprintf("%#.1g", 1) returns 1, while gawk's alternate form returns 1.; the scientific branch has the same problem (1.e+04 becomes 1e+04). Ensure alternate form inserts the locale decimal separator when no fractional digits remain.
Useful? React with 👍 / 👎.
| boolean zeroMagnitude = isZeroMagnitude(magnitude); | ||
| int actualPrecision = precision; | ||
| if (flags.alternate && zeroMagnitude && precision == 0 && conversion != 'u') { |
There was a problem hiding this comment.
The zero suppression logic only considers the converted magnitude, but gawk retains 0 for unsigned conversions when a nonzero fraction truncates to zero, and also for alternate %u. For example, sprintf("%.0x", 0.1) and sprintf("%#.0u", 0) should both return 0, whereas this path ultimately emits an empty string. Track the original value and alternate-form rules when selecting the effective precision.
Useful? React with 👍 / 👎.
| Flags flags = new Flags(leftJustify, plusSign, spaceSign, zeroPad, alternate, grouping); | ||
| Object arg = argPosition > 0 ? argAt(argPosition) : nextArg(); | ||
| render(conversion, flags, width, precision, arg); |
There was a problem hiding this comment.
When a format mixes positional and sequential arguments, this independently chooses argAt or nextArg without detecting the mix. For example, %2$s %s with a, b silently produces b a, while gawk treats mixing n$ and ordinary specifiers as a fatal formatting error. Track the argument-selection mode and reject a format that switches modes.
Useful? React with 👍 / 👎.
- Route the AwkPrintfTest suite through new AwkTestSupport helper
methods (assertSprintf, assertSprintfThrows, assertToAwkString), per
the repository testing guidelines.
- Out-of-range %u/%o/%x/%X values now fall back to %g with the original
sign, flags, precision, and width, like gawk: printf "%u", -(2^100)
prints -1.26765e+30, and %.10x / %#.3x / %020u honor their precision
and flags (gawk-verified).
- The '#' flag keeps the decimal point on %g/%e results without
fractional digits: %#.1g of 1 prints "1." and %#.1g of 12345 prints
"1.e+04", like gawk.
- Unsigned conversions with an explicit zero precision print "0" when a
nonzero value truncates to zero (%.0x of 0.1) or when '#' is given
(%#.0u of 0), while an exact zero still prints nothing, matching
gawk; %d keeps printing nothing in both cases.
- Mixing positional (n$) and sequential specifiers in one format string
is now a fatal error, like gawk ("must use `count$' on all formats or
none").
All new expectations verified against gawk 5.0/5.1.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9a0004adb7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (precision >= 0 && s.length() > precision) { | ||
| s = s.substring(0, precision); |
There was a problem hiding this comment.
%s precision by Unicode code points
When a string contains a supplementary Unicode character, String.length() and substring() count its two UTF-16 surrogates separately. Consequently sprintf("%.1s", "😀x") produces an unpaired surrogate (typically encoded as ?) instead of 😀, whereas gawk treats the precision as one character. Use code-point-aware length and truncation so precision cannot split a character.
Useful? React with 👍 / 👎.
| if (flags.alternate && !zeroMagnitude) { | ||
| if (conversion == 'x') { | ||
| prefix = "0x"; | ||
| } else if (conversion == 'X') { | ||
| prefix = "0X"; | ||
| } |
There was a problem hiding this comment.
When # is used with %x or %X and a nonzero fractional input truncates to zero, this condition suppresses the prefix because it only examines the converted magnitude. The new actualPrecision branch is fresh evidence that the earlier zero-suppression fix remains incomplete: sprintf("%#.0x", 0.1) now returns 0, while gawk returns 0x0; alternate-form selection must also account for the original nonzero value.
Useful? React with 👍 / 👎.
| if (flags.alternate | ||
| && conversion == 'o' | ||
| && !magnitude.startsWith("0") | ||
| && (precision < 0 || precision <= magnitude.length())) { |
There was a problem hiding this comment.
For an explicit precision, this condition assumes precision padding already satisfies alternate octal form, but gawk adds the alternate leading zero in addition to the requested precision. For example, sprintf("%#.5o", 1) currently returns 00001, while gawk returns 000001; the alternate digit should not be skipped merely because the precision exceeds the magnitude length.
Useful? React with 👍 / 👎.
| if (digitsEnd > i && digitsEnd < length && format.charAt(digitsEnd) == '$') { | ||
| argPosition = parseInt(format, i, digitsEnd); | ||
| i = digitsEnd + 1; |
There was a problem hiding this comment.
When the digits before $ parse to zero, argPosition == 0 is later treated as the sentinel for sequential selection. Thus %0$s and %00$s silently consume the next argument, while gawk rejects them because positional indices must be greater than zero. Validate the parsed position here rather than allowing zero to fall back to sequential mode.
Useful? React with 👍 / 👎.
- %s precision counts characters (code points), so it can never split a
surrogate pair: sprintf("%.1s", "😀x") returns the emoji, like gawk in
a multibyte locale.
- The '#' prefix for %x/%X/%o depends on the original value rather than
the truncated magnitude: %#.0x of 0.1 prints "0x0" and %#x of 0.5
prints "0x0", like gawk.
- gawk's alternate octal form always adds its leading zero on nonzero
values, in addition to any precision padding: %#.5o of 1 prints
"000001" and %#.3o of 8 prints "0010" (gawk-verified; this also
covers %#.0o of 0.2 printing "00").
- A zero positional argument index (%0$s) is now fatal, like gawk
("argument index with `$' must be > 0"), both for conversions and for
dynamic *n$ width/precision references.
All expectations verified against gawk 5.0/5.1.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d75fde6cd3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| int starArgEnd = starPositionEnd(i); | ||
| long dynamicWidth; | ||
| if (starArgEnd > i) { | ||
| dynamicWidth = (long) JRT.toDouble(argAt(parseInt(format, i, starArgEnd - 1))); |
There was a problem hiding this comment.
Fresh evidence after the mixed-mode fix is that this argAt call marks the entire format positional even when only a width operand uses n$. Gawk permits positional star operands with sequential conversions: sprintf("%*2$s|%s", "a", 5) produces " a|5", but this implementation calls argAt(2) for the width and then rejects the first conversion's nextArg() as mixed mode. Track the conversion-argument mode separately from explicitly positioned width and precision operands.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| private void appendPadded(String body, boolean leftJustify, boolean zeroPad, int width) { | ||
| if (width <= body.length()) { |
There was a problem hiding this comment.
When %s or %c produces a supplementary Unicode character, body.length() counts its surrogate pair as two columns. In a multibyte locale, sprintf("%3s", "😀") should contain two leading spaces like gawk, but this path emits only one; left justification and numeric-code-point %c have the same problem. Calculate padding from the code-point count, as the precision logic already does.
Useful? React with 👍 / 👎.
- The positional/sequential mixing check now applies only to how
conversions select their value argument: gawk allows an explicitly
positioned star operand alongside sequential conversions, so
sprintf("%*2$s|%s", "a", 5) now produces " a|5" (gawk-verified).
- Field widths count characters (code points), so a supplementary
character fills one column: sprintf("%3s", "😀") pads with two
spaces, like gawk's %s in a multibyte locale. gawk's %c padding
counts bytes (a C-locale artifact); Jawk pads %c by characters too,
documented in compatibility.md.vm.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 30df61f06a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| dynamicWidth = (long) JRT.toDouble(argAt(parseInt(format, i, starArgEnd - 1))); | ||
| i = starArgEnd; | ||
| } else { | ||
| dynamicWidth = (long) JRT.toDouble(nextArg()); |
There was a problem hiding this comment.
Fresh evidence after the mixed-mode fix: an unpositioned * still consumes a sequential argument without recording that mode. Consequently sprintf("%2$*d", 5, 12) returns " 12", while gawk rejects the format for mixing a sequential width operand with a positional conversion; the same issue affects unpositioned star precision operands.
Useful? React with 👍 / 👎.
| int end = i < length ? i + 1 : length; | ||
| out.append(format, start, end); | ||
| return end; |
There was a problem hiding this comment.
Fresh evidence after the mixed-mode fix: this early return bypasses recordArgumentMode, even when the unknown specifier contains an explicit position. Thus sprintf("%2$q|%d", 5, 12) produces %2$q|5, whereas gawk treats the positioned unknown specifier followed by a sequential conversion as a fatal mixed-mode format.
Useful? React with 👍 / 👎.
| String mantissa = decimalString( | ||
| rounded.movePointLeft(exponent).setScale(precision - 1, RoundingMode.UNNECESSARY)); | ||
| mantissa = alternate ? forceDecimalSeparator(mantissa) : stripTrailingFractionZeros(mantissa); |
There was a problem hiding this comment.
%g rollover
When rounding crosses a power-of-ten boundary, this fixed scale can retain a significant zero that gawk/glibc omit even under alternate form. For example, %#.2g with 99.99 returns 1.0e+02 here, while gawk 5.2.1 and libc both return 1.e+02; rollover cases need to adjust the mantissa precision after the exponent changes.
Useful? React with 👍 / 👎.
| out.append(sign); | ||
| out.append(zeros(width - full.length())); | ||
| out.append(magnitude); |
There was a problem hiding this comment.
%a zero padding
For zero-padded hexadecimal floats, padding is inserted before the entire magnitude, including its 0x prefix. As a result, %020a with 1234.5 emits 00000000000x1.34ap10; even with the documented Java-style %a notation, Java formatting produces 0x00000000001.34ap10, and gawk likewise keeps 0x at the start. Handle the hexadecimal prefix like an integer prefix when applying zero padding.
Useful? React with 👍 / 👎.
- Sequential star operands now pin the format to sequential mode, so a positional conversion with an unpositioned star width or precision (%2$*d) is a mixed-mode fatal error, like gawk. - An explicitly positioned unknown specifier (%2$q) pins the format to positional mode even though it prints verbatim, so following it with a sequential conversion is fatal, like gawk. - The hexadecimal prefix of %a/%A stays ahead of zero padding: %020a of 1234.5 prints 0x00000000001.34ap10 instead of inserting zeros before the 0x prefix. All gawk-verified. The %#.2g rollover comment is answered on the PR instead: glibc's "1.e+02" for %#.2g of 99.99 is inconsistent with its own "1.0e+04" for %#.2g of 9999, so Jawk keeps the C-standard result (which mingw-gawk also produces), per the project's compatibility policy of not reproducing C-library accidents. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Thanks — three of the four are fixed in bfb0cdd (sequential star operands pin sequential mode, positioned unknown specifiers pin positional mode, and the On the Both values round to exactly one significant digit times a power of ten, yet glibc keeps the mantissa zero in one case and drops it in the other — with @codex please review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bfb0cdd2e5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // The exact binary value of the double is intended: it makes rounding match gawk's C library. | ||
| String s = decimalString(new BigDecimal(abs).setScale(p, RoundingMode.HALF_EVEN)); // NOPMD | ||
| if (flags.alternate && p == 0) { | ||
| s = s + "."; |
There was a problem hiding this comment.
When the formatter uses a locale whose decimal separator is not ., the alternate form with zero precision still appends a hard-coded period. For example, AwkPrintf.sprintf(Locale.FRANCE, "%.6g", "%#.0f", 1) returns 1. instead of localized 1,; the analogous %#.0e path also inserts .e rather than ,e. Use DecimalFormatSymbols here as the other floating-point paths already do.
Useful? React with 👍 / 👎.
The '#' flag with zero precision appended a hard-coded '.' for %f and %e; it now inserts the locale's decimal separator, consistent with the other floating-point paths: %#.0f of 1 with Locale.FRANCE prints "1," and %#.0e of 12345 prints "1,e+04". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75294faa52
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| int p = precision < 0 ? 6 : precision == 0 ? 1 : precision; | ||
| String s = generalFloat(abs, p, flags.alternate); | ||
| return conversion == 'G' ? s.toUpperCase(Locale.ROOT) : s; |
There was a problem hiding this comment.
When the ' flag is used and %g selects fixed-point notation, this branch never applies groupDigits, unlike the %f path. For example, AwkPrintf.sprintf(Locale.US, "%.6g", "%'g", 12345) returns 12345 instead of the locale-grouped 12,345, so the newly supported grouping flag is silently ignored for %g/%G.
Useful? React with 👍 / 👎.
| if (flags.grouping) { | ||
| digits = groupDigits(digits); | ||
| } |
There was a problem hiding this comment.
Because grouping is applied unconditionally to every integer magnitude, the GNU apostrophe flag also inserts decimal grouping separators into octal and hexadecimal output, where gawk does not apply it. With the US locale, sprintf("%'x", 10000000000) currently produces 254,0be400 and sprintf("%'o", 999) produces 1,747; these should remain 2540be400 and 1747 respectively.
Useful? React with 👍 / 👎.
| if (format.charAt(i + 1) == '%') { | ||
| out.append('%'); | ||
| i += 2; | ||
| continue; |
There was a problem hiding this comment.
Only an immediately adjacent %% is recognized as a percent conversion; after flags or a width, the final % is treated as unknown and the whole specifier is emitted verbatim. Consequently sprintf("[%5%]") returns [%5%], whereas gawk accepts the percent conversion and returns [%], affecting dynamically generated formats that attach modifiers to a literal percent.
Useful? React with 👍 / 👎.
- The ' grouping flag applies to decimal conversions only: %'x and %'o no longer insert grouping separators, like gawk. - %g in fixed notation now honors the grouping flag (%'g of 12345 prints "12,345"), while exponential %g and %e stay ungrouped, like gawk. - A percent conversion reached through flags, width, or precision prints a plain '%' with the modifiers ignored: %5% prints "%", like gawk, instead of being treated as an unknown specifier. All gawk-verified (en_US locale for the grouping cases). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
The extra appendInteger parameter exceeded the 7-parameter checkstyle limit; octal and hexadecimal conversions now clear the grouping flag via Flags.withoutGrouping() instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4cef883ee3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (i < length && format.charAt(i) == '%') { | ||
| // A percent conversion reached through flags, width, or | ||
| // precision prints a plain '%' and ignores them all, like | ||
| // gawk ("%5%" prints "%"). | ||
| out.append('%'); | ||
| return i + 1; |
There was a problem hiding this comment.
When a percent conversion has an explicit argument index, this early return skips recordArgumentMode(true). Consequently sprintf("%1$%|%s", "a") returns %|a, while gawk 5.2.1 rejects it for mixing positional and sequential formats. Record the explicit position before returning, as the unknown-conversion branch already does.
Useful? React with 👍 / 👎.
| private String generalFloat(double abs, int precision, boolean alternate, boolean grouping) { | ||
| if (abs == 0) { | ||
| return alternate ? "0." + zeros(precision - 1) : "0"; |
There was a problem hiding this comment.
When %#g formats zero under a locale with a non-dot decimal separator, this hard-coded string bypasses decimalString and forceDecimalSeparator. For example, AwkPrintf.sprintf(Locale.FRANCE, "%.6g", "%#g", 0) returns 0.00000 instead of the localized 0,00000, although nonzero %g values and the other float conversions use the requested locale.
Useful? React with 👍 / 👎.
| private static final String CONVERSION_CHARS = "diouxXeEfFgGaAcs"; | ||
|
|
||
| /** Length modifier characters accepted (and ignored) like gawk. */ | ||
| private static final String LENGTH_MODIFIERS = "hlL"; |
There was a problem hiding this comment.
In default gawk-compatible mode, gawk 5.2.1 accepts single j, z, and t length modifiers and ignores them just like h and l. Restricting the recognized set to hlL therefore makes sprintf("%zd|%d", 1, 2) produce %zd|1 instead of 1|2, because the first conversion is emitted verbatim and does not consume its argument.
Useful? React with 👍 / 👎.
- A positional percent conversion (%1$%) pins the format to positional
mode before printing its '%', so following it with a sequential
conversion is fatal, like gawk.
- %#g of zero uses the locale decimal separator ("0,00000" with
Locale.FRANCE) instead of a hard-coded period.
- Length modifiers now match current gawk (verified against gawk
master's printf.c): h, j, l, L, t, and z are each accepted at most
once and ignored; distinct modifiers may stack (%lhd), while a
repeated modifier (%lld, %hhd) still invalidates the specifier. This
restores the original Printf4J expectations for %zu/%zd/%zi.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 922d2de587
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| */ | ||
| public static String sprintf(final Locale locale, final String convfmt, final String format, final Object... args) { | ||
| Locale actualLocale = locale == null ? Locale.US : locale; | ||
| String actualConvfmt = convfmt == null || convfmt.isEmpty() ? DEFAULT_CONVFMT : convfmt; |
There was a problem hiding this comment.
When a script explicitly assigns CONVFMT="", this fallback silently replaces it with %.6g, so BEGIN { CONVFMT=""; printf "[%s]", 1.5 } produces [1.5] instead of the empty conversion [] produced previously and by gawk. The analogous empty-format fallback in numberToAwkString also changes OFMT="" for non-integral print operands; only a null format should select the default, while an explicitly empty current format must remain empty.
Useful? React with 👍 / 👎.
| if (argPosition > 0) { | ||
| recordArgumentMode(true); |
There was a problem hiding this comment.
When a percent conversion has an explicit position beyond the supplied argument count, this branch records positional mode but never validates the index. Consequently sprintf("%2$%", 1) returns %, while gawk rejects it because argument 2 was not supplied; the positioned unknown-conversion branch has the same problem (sprintf("%2$q", 1) returns %2$q). Validate argPosition against the argument array even though these conversions do not consume the referenced value.
Useful? React with 👍 / 👎.
- An explicitly empty CONVFMT or OFMT stays empty, like gawk: only a null (absent) format selects the %.6g default, so CONVFMT="" makes %s convert non-integral numbers to the empty string. - Positional argument indexes are validated even for conversions that consume no argument: %2$% and %2$q with one argument are fatal, with gawk's message (argument index N greater than total number of supplied arguments). Both gawk-verified. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7e2a7afee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| int starArgEnd = starPositionEnd(i); | ||
| long dynamicWidth; | ||
| if (starArgEnd > i) { | ||
| dynamicWidth = (long) JRT.toDouble(argAt(parseInt(format, i, starArgEnd - 1))); |
There was a problem hiding this comment.
In gawk 5.2.1, a zero positional index is valid specifically for a star operand: sprintf("%*0$d|%d", 7, 42) produces 7|42, treating the width as zero without consuming an argument, and .*0$ behaves analogously for precision. This path instead passes position zero to argAt, which throws, so these gawk-compatible formats become fatal; special-case zero for positioned width and precision while continuing to reject a conversion such as %0$d.
Useful? React with 👍 / 👎.
A zero positional index on a star operand (%*0$d, %.*0$f) means the value zero without consuming an argument, while a conversion with a zero index (%0$d) remains fatal. gawk-verified. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b21491a0ee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| public String sprintfWithConvFmt(String convfmt, String format, Object... values) { | ||
| Object[] safeValues = values == null ? new Object[0] : values; | ||
| return Printf4J.sprintf(locale, format, safeValues); | ||
| return AwkPrintf.sprintf(locale, convfmt, format, safeValues); |
There was a problem hiding this comment.
When an existing custom AwkSink overrides the previously supported sprintf(String, Object...) method but not this newly introduced method, AWK sprintf() calls now bypass that override because AVM invokes sprintfWithConvFmt, whose default implementation formats directly with AwkPrintf. This silently removes the sink's customization—and can make its sprintf and printf behavior inconsistent—so the new bridge must preserve legacy sprintf overrides while still allowing built-in sinks to honor CONVFMT.
Useful? React with 👍 / 👎.
The default sprintfWithConvFmt now detects (once per sink class, via reflection) whether the historical sprintf(String, Object...) customization point is overridden and routes through it, so pre-7.1 custom sinks keep their formatting behavior for both printf and sprintf. CONVFMT cannot reach the legacy signature, so such sinks convert %s operands with the default CONVFMT; overriding sprintfWithConvFmt receives the live value. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab35dd3f7f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (overridesLegacySprintf()) { | ||
| return sprintf(format, safeValues); |
There was a problem hiding this comment.
When a legacy AwkSink override decorates the default result by calling super.sprintf(format, values), the new reflection bridge calls that override again: super.sprintf dispatches to sprintfWithConvFmt, this branch still detects the override, and line 353 re-enters it until a StackOverflowError. This is fresh evidence from the compatibility bridge added after the earlier override issue; existing overrides that previously delegated to super must be able to reach the base formatter without being redispatched.
Useful? React with 👍 / 👎.
The base sprintf(String, Object...) now invokes the formatting engine directly instead of delegating to sprintfWithConvFmt, so a legacy override that decorates super.sprintf(...) reaches the base formatter without being redispatched into itself (previously a StackOverflowError). Covered by a decorating-sink unit test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a50dc43416
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (digitsEnd > i && digitsEnd < length && format.charAt(digitsEnd) == '$') { | ||
| argPosition = parseInt(format, i, digitsEnd); | ||
| if (argPosition <= 0) { | ||
| throw new AwkRuntimeException("argument index with `$' must be > 0 in `" + format + "'"); | ||
| } | ||
| i = digitsEnd + 1; |
There was a problem hiding this comment.
When Jawk runs with --posix, this parser still accepts %n$ unconditionally because neither AwkPrintf nor its call sites receive the execution mode. Thus jawk --posix 'BEGIN { printf "%2$s", "a", "b" }' prints b, while GNU awk 5.2.1 in POSIX mode rejects $ in formats; the newly supported gawk extension therefore leaks into Jawk's strict mode.
Useful? React with 👍 / 👎.
| int width = -1; | ||
| if (i < length && format.charAt(i) == '*') { | ||
| i++; | ||
| int starArgEnd = starPositionEnd(i); |
There was a problem hiding this comment.
When digits follow * without a terminating $, starPositionEnd reports no position and this branch treats the star as sequential, consumes an argument, and later emits the malformed specifier verbatim. Consequently %*2d and %.*2f produce literal text instead of the fatal “no $ supplied for positional field width or precision” error from GNU awk 5.2.1; detect this digit-without-$ case before consuming the star operand.
Useful? React with 👍 / 👎.
- Strict --posix mode now rejects gawk positional argument references in printf/sprintf formats with gawk's message (`$' is not permitted in awk formats): AVM validates formats via the new AwkPrintf.usesPositionalArguments() before formatting. - Digits after a star operand without a terminating $ (%*2d, %.*2f) are now fatal like gawk (no `$' supplied for positional field width or precision) instead of printing the specifier verbatim after consuming an argument. Both gawk-verified. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Fixes #528.
What
Replaces the external Printf4J dependency (which emulated glibc's
printf()) with a newio.jawk.jrt.AwkPrintfclass that implements POSIX AWKprintf/sprintfsemantics as implemented by gawk. Every behavior listed below was verified empirically against gawk 5.0/5.1.AWK semantics now implemented
%suses AWK's number-to-string rules: integral values print without a fractional part —printf "x[%s]", iafteri++now printsx[1], notx[1.0](the symptom in Reincorporate Printf4J into Jawk to fully implement AWK's printf #528) — and non-integral values honor the script's currentCONVFMT.%cprints the character of a numeric code point (numbers, numeric strings, and numeric fields), or the first character of a string value.*width and precision (%*.*f,%.*s), including negative values (negative width left-justifies, negative precision means "no precision"); the previously-skipped POSIX 9.4 conformance test is enabled.%2$s %1$s) and the'grouping flag (%'d→1,234,567).%u/%o/%x/%X(%xof -1 →ffffffffffffffff); beyond 64 bits,%d/%iprint the full decimal expansion and%u/%o/%x/%Xfall back to%gnotation.nan/inf/-inf(previously Java'sNaN/Infinity).%e/%f/%gviaBigDecimal, matching the C library used by gawk (printf "%.0f", 2.5→2);%gstrips trailing zeros before padding.%q,%b,%n— which Printf4J used to turn into a newline — and invalid length modifiers likell/hh); a singleh/l/Lmodifier is accepted and ignored, exactly like gawk.Related fix: 64-bit saturation
Integral values beyond 2^63-1 were saturated to
Long.MAX_VALUEby compile-time constant folding,int(), and number-to-string conversion, soprint 2^100printed9223372036854775807. These sites now keep such values as doubles (JRT.toScalarNumber()/JRT.truncateToScalar()), andprint 2^100prints1267650600228229401496703205376like gawk. This was necessary for the out-of-rangeprintfconversions to be observable end-to-end.API
AwkSinkgainsprintfWithConvFmt(...)andsprintfWithConvFmt(...)so the runtime can pass the script's currentCONVFMTper call (stateless, thread-safe). The default implementations keep existing custom sinks working unchanged; new method names (rather than overloads) avoid silently rebinding existing call sites.AwkSink.formatOutputValue(...)now delegates toAwkPrintf.toAwkString(...), which also fixesOFMT/CONVFMTconversion: formats like%.2fare honored verbatim (no more unconditional trailing-zero stripping — stripping is now part of proper%ghandling), and exactLongvalues no longer round-trip throughdouble.org.metricshub:printf4jdependency is removed from the POM.Tests
AwkPrintfTestincorporates the complete Printf4J unit test suite — including all@Disabledtests and commented-out assertions, as requested in Reincorporate Printf4J into Jawk to fully implement AWK's printf #528. Where Printf4J's glibc-oriented expectations differ from AWK semantics, the expected values were adapted and each adaptation is annotated with the gawk-verified behavior. Adds AWK-specific coverage beyond the original suite (CONVFMT, positional specifiers, out-of-range conversions, non-finite values, argument-count errors).PrintfTestadds script-level tests throughAwkTestSupport(CONVFMT propagation incl. redirected output,%con fields, fatal error on missing arguments, huge-value printing, half-even rounding).behavior-changes.md(Unreleased),compatibility.md.vm(printf semantics + Java-platform edge cases),index.md.vm,java-output.md(new sink parameters).mvn clean verify sitepasses: all unit tests green, checkstyle/pmd/spotbugs clean.🤖 Generated with Claude Code