Repository navigation
wave 34 (encoding): transcoding errors carry their attributes, an encoding tag is resolved before it keys a codec (+37) - #679
Merged
Merged
Conversation
Encoding::UndefinedConversionError and Encoding::InvalidByteSequenceError had no #source_encoding, #destination_encoding, #error_char, #error_bytes, #readagain_bytes or #incomplete_input?, so 28 ruby/spec examples raised NoMethodError. The data was already computed correctly — Encoding::Converter#primitive_errinfo reported it byte for byte — but two things kept it off the exception. MRI's make_econv_exception (transcode.c) picks the class, formats the message and attaches the attributes in one breath, and rb_econv_check_error passes that very object to rb_exc_raise. rbgo recorded an exception in #last_error and then raised a *second* one built from the message alone, so the object the user rescued had no attributes to read. setEconvErrorAttrs now mirrors make_econv_exception, and #convert / #finish raise the recorded object itself, so the rescued exception and #last_error can never disagree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ere MRI does Three defects that all live on the String#encode path. An encoding TAG was used raw where a canonical name was needed. MagicSourceEncoding's comment says the vm's case-insensitive lookup makes `# encoding: iso-8859-9` resolve to the canonical object — true of String#encoding, which goes through the registry, but NOT of the transcoding path, which keyed xtextEncodings on the literal tag. So a file with that magic comment produced strings reporting ISO-8859-9 that no converter could be found for. MRI cannot have this bug: rb_enc_associate_index (encoding.c) resolves the spelling once and a String then carries an encoding INDEX. canonicalEncName is that resolution, in one place. String#encode raised ConverterNotFoundError where MRI needs no converter. str_transcode0 (transcode.c) short-circuits before any converter is looked up when source and destination encodings match and no decorator is in play: with an explicit `invalid:` it is rb_enc_str_scrub in that one encoding, otherwise the bytes pass through. rbgo checked converter availability first. #unicode_normalize accepted any encoding. UnicodeNormalize.normalize (lib/unicode_normalize/normalize.rb) dispatches on the encoding first: UTF-8 direct, US-ASCII already normalized, the other Unicode encodings through UTF-8, everything else Encoding::CompatibilityError. rbgo normalized the bytes of any encoding as if they were UTF-8, so ISO-8859-1 reported "invalid byte sequence in UTF-8" where MRI reports the incompatibility. Also adds the Emacs-Mule character automaton, transcribed from the `trans` table of enc/emacs_mule.c, so #scrub, #valid_encoding? and the new same-encoding #encode path agree about what a well-formed Emacs-Mule character is. validInEncoding now asks that same scanner rather than keeping a second opinion. ruby/spec: string/encode 150->152, string/encoding 24->25, string/unicode_normalize 12->14, string/unicode_normalized? 10->11 (each now at its measured MRI ceiling). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
setConvError had grown a `default: return` that no caller can reach — every call site passes one of the three failure statuses — so it was an untestable branch. setEconvErrorAttrs already returns without touching the exception for any other status, and that guard IS reachable and tested, so the duplicate one comes out. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Wave 34 (core/string, core/symbol, core/encoding). Measured +37 ruby/spec
examples: 22489 → 22526 (
core+language, serial 2-way,TIMEOUT=25,corpus
87b1631). No file moved down.FLOORis untouched.Per file: before → after → measured MRI ceiling
Every ceiling below is MRI 4.0.5 run through our own mspec shim, not the file's
example count.
core/encoding/undefined_conversion_error/source_encoding_spec.rbcore/encoding/undefined_conversion_error/source_encoding_name_spec.rbcore/encoding/undefined_conversion_error/error_char_spec.rbcore/encoding/undefined_conversion_error/destination_encoding_spec.rbcore/encoding/undefined_conversion_error/destination_encoding_name_spec.rbcore/encoding/invalid_byte_sequence_error/source_encoding_spec.rbcore/encoding/invalid_byte_sequence_error/source_encoding_name_spec.rbcore/encoding/invalid_byte_sequence_error/error_bytes_spec.rbcore/encoding/invalid_byte_sequence_error/readagain_bytes_spec.rbcore/encoding/invalid_byte_sequence_error/destination_encoding_spec.rbcore/encoding/invalid_byte_sequence_error/destination_encoding_name_spec.rbcore/encoding/invalid_byte_sequence_error/incomplete_input_spec.rbcore/string/encode_spec.rbcore/string/encoding_spec.rbcore/string/unicode_normalize_spec.rbcore/string/unicode_normalized_spec.rbAll sixteen files are now at their ceiling.
What was wrong, and the C functions read for each decision
1. The transcoding exceptions carried no attributes (+33).
Encoding::UndefinedConversionErrorandEncoding::InvalidByteSequenceErrorhadnone of
#source_encoding,#source_encoding_name,#destination_encoding,#destination_encoding_name,#error_char,#error_bytes,#readagain_bytes,#incomplete_input?. The data was already right —#primitive_errinforeported itbyte for byte — but two things kept it off the object.
Read:
make_econv_exception,rb_econv_check_error,ecerr_source_encoding,ecerr_source_encoding_name,ecerr_destination_encoding,ecerr_destination_encoding_name,ecerr_error_char,ecerr_error_bytes,ecerr_readagain_bytes,ecerr_incomplete_input(transcode.c).make_econv_exceptionpicks the class, formats the message and attaches theattributes in one breath, and
rb_econv_check_errorhands that very object torb_exc_raise. rbgo recorded one exception in#last_errorand then raised asecond one built from the message alone, so what the user rescued had nothing to
read.
setEconvErrorAttrsnow mirrorsmake_econv_exception(including@readagain_bytesbeingnil, not"", whenreadagain_lenis 0, and theset_encsrule that a name is always stored but anEncodingobject only when thename resolves), and
#convert/#finishraise the recorded object itself. Eachaccessor is
rb_attr_getof one ivar, soEncoding::InvalidByteSequenceError.new .incomplete_input?isnilrather than aNoMethodError.2. An encoding TAG was used raw where a canonical name was needed (+1).
MagicSourceEncoding's comment claimed the vm's case-insensitive lookup made# encoding: iso-8859-9resolve to the canonical object. That is true ofString#encoding, which goes through the registry — and not true of thetranscoding path, which keyed
xtextEncodingson the literal tag. So such a file'sliterals reported
ISO-8859-9yet raisedConverterNotFoundError (iso-8859-9 to UTF-8)on#encode.Read:
rb_enc_associate_index,rb_enc_find_index(encoding.c). MRI cannot havethis defect: a String carries an encoding index, so the spelling is resolved
once and every later comparison is index equality.
canonicalEncNameis thatresolution, in one place, and
encodeEncName's private copy of it is gone.3.
String#encodelooked for a converter MRI never needs (+2).Read:
str_transcode0(transcode.c). It short-circuits before any converteris looked up when source and destination encodings match and no newline/xml
decorator is in play: with an explicit
invalid:the result isrb_enc_str_scrubin that one encoding, otherwise the bytes pass through. rbgochecked converter availability first, so
"\x80".force_encoding("Emacs-Mule").encode(invalid: :replace)raised instead ofreturning
"?".That case also needed an Emacs-Mule character automaton, transcribed from the
transtable ofenc/emacs_mule.c(the grammar comment there is not enough: thethree secondary leads take different extension ranges, and
EncLen_EmacsMuleisonly an upper bound — MRI judges validity with the automaton).
validInEncodingnowasks that same scanner instead of keeping a second opinion, so
#scrub,#valid_encoding?and this#encodepath cannot disagree about what a well-formedcharacter is.
4.
#unicode_normalizeaccepted any encoding (+3).Read:
UnicodeNormalize.normalize/.normalized?inlib/unicode_normalize/normalize.rb— this one is Ruby, not C. It dispatches on theencoding first: UTF-8 direct, US-ASCII already normalized in every form, the
UNICODE_ENCODINGSlist through UTF-8 and back, everything elseEncoding::CompatibilityError. rbgo normalized the bytes of any encoding as if theywere UTF-8, so ISO-8859-1 reported
"invalid byte sequence in UTF-8"where MRIreports the incompatibility. The order matters and is now preserved: that
ArgumentErrorcan only come from the UTF-8 branch.Verified in both directions
TestEconvErrorAttributes,TestEncodeSameEncoding,TestValidInEncodingUsesTheScannerandTestUnicodeNormalizeEncodingDispatchwere each run against the unfixed product— the behaviour reverted while the new symbols stayed, so the mutation still
compiled — and each failed. The remaining tests in the file cover helpers
that only exist with the fix; they are labelled GUARDS in the file header.
origin/main's, measured with the same-coverpkg=./internal/...profile on adetached worktree at
origin/main. Nothing was orphaned. (Totals were notcompared: both read 99.9%.) One
default:branch insetConvErrorthat nocaller can reach was removed rather than left untestable —
setEconvErrorAttrsalready holds that judgement, reachably and tested.
go test ./...,go build ./...,GOOS=wasip1 GOARCH=wasm go build ./...,gofmt -l internal/ cmd/all clean.Left undone, with reasons
core/string/valid_encoding_spec.rb(1 example short of its ceiling of 8) needsCP949, Emacs-Mule, EUC-TW and ISO-2022-JP to agree with MRI in one
assertion. Emacs-Mule and ISO-2022-JP now would; CP949 and EUC-TW need their
Onigmo DFAs (
enc/cp949.c,enc/euc_tw.c) transcribed — several hundred linesof table for one example. Not worth this wave.
core/encoding/converter/last_error_spec.rbandfinish_spec.rb(1 each):ISO-8859-1 → Big5 does not raise at all in rbgo, and the stateful ISO-2022-JP
converter does not emit its closing escape from
#finish. Both areconverter_codec.gocodec coverage, not error plumbing.String#crypt(6 examples) is a genuine gap and needs real DEScrypt(3).Deliberately out of scope here.
🤖 Generated with Claude Code