Repository navigation
File path helpers run rb_get_path, not to_s, so a non-String path is refused (fixes #57) - #153
Open
jiro-by-nori[bot] wants to merge 1 commit into
Open
jiro-by-nori[bot] wants to merge 1 commit into
jiro-by-nori[bot] wants to merge 1 commit into
Conversation
…refused (issue #57) `File.basename`, `#dirname`, `#extname` and `.join` began by calling `to_s` on every path argument, so `File.dirname(123)` answered "." where CRuby raises `TypeError: no implicit conversion of Integer into String`. That is a wrong answer, not a gate: the model answered a program CRuby rejects. `File.__path_string` is CRuby's `rb_get_path` (`file.c`): a String is itself; otherwise `to_path` is asked with `rb_check_funcall_default`, whose default is the object itself; and the answer goes through `StringValue`, which is a new `Kernel#__to_string_value` (`to_str`, with CRuby's two messages). Nothing is coerced with `to_s`. The conversion dispatches, so a `method_missing`-served `to_path` converts and a `respond_to?` that denies it does not. `File.join` also followed the old `to_s` rule through `Array#join` on a mapped list, which answered "[\"a\", \"b\"]/c" for `File.join(["a", "b"], "c")`. It now follows `rb_file_join`: nested Arrays join recursively with an identity cycle check (`ArgumentError: recursive array`), a `to_ary` is not consulted, and the separator is written only where `chompdirsep`/`isdirsep` say one is missing, so `File.join("a/", "/b")` is "a/b". Two `difftest` regression cases pin both: the four rows from the issue, the `to_path`/`to_str` boundary, the messages, and join's flattening, separators and cycle. `make check` and `make check-not-weakened` pass.
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.
What this fixes
File.basename,File.dirname,File.extnameandFile.joinbegan by callingto_son every path argument, so a non-String path was stringified instead of refused — a wrong answer, not a gate:File.dirname(123)"."TypeError: no implicit conversion of Integer into StringFile.basename(123)"123"TypeErrorFile.join(1, 2)"1/2"TypeErrorFile.extname(:sym)""TypeError: no implicit conversion of Symbol into StringTwo related join defects fell out of the same
to_s-and-Array#joinimplementation:File.join(["a", "b"], "c")answered"[\"a\", \"b\"]/c"(CRuby"a/b/c"), andFile.join("a/", "/b")answered"a///b"(CRuby"a/b").The change
ruby-lean/prelude/prelude.rbonly; the two generated files aremake genoutput.File.__path_stringis CRuby'srb_get_path(file.c): a String is itself; otherwiseto_pathis asked withrb_check_funcall_default, whose default is the object itself; the answer then goes throughStringValue. Nothing is coerced withto_s. Because the conversion dispatches, amethod_missing-servedto_pathconverts, arespond_to?that denies it does not, and ato_pathreturning a non-String is converted again — sodirname(obj_with_to_path_returning_42)namesInteger, exactly as CRuby does.Kernel#__to_string_valueisStringValue(rb_convert_type_with_id) for String, the twin of the existing__to_array_type:no implicit conversion of X into Stringwhen noto_stranswers,can't convert X to String (X#to_str gives Y)when one answers a non-String.File.joinfollowsrb_file_join: each element that is anArrayis joined recursively with an identity cycle check (ArgumentError: recursive array; the same array twice is not a cycle, and ato_aryis not consulted), every other element goes through__path_string, and the separator is written only wherechompdirsep/isdirsepsay one is missing.basename'ssuffixtakes the same conversion, which is whyFile.basename("a/b", :x)is now CRuby'sTypeError.Not modelled, and not pretended to:
rb_get_path_check_convert's null-byte and non-ASCII-compatible-encoding refusals (the model has no encodings and refuses neither).Tests
Two new
difftestregression cases, bothstatus: fixed, so this tier ratchets against reintroduction:difftest/corpus/regressions/file-path-argument-conversion.rb— the four rows above;nil/truenaming;to_pathvsto_strprecedence; a non-Stringto_path/to_stranswer;method_missingandrespond_to?dispatch;Pathname; andbasename's suffix.difftest/corpus/regressions/file-join-elements.rb— flattening and deep nesting, nested empty arrays, the separator rules,Integer/nil/Hash/Symbolelements, ato_aryelement, a shared array, and self/nested cycles.Validation
Run on the branch from a cold toolchain (Lean
v4.32.2, CRuby4.0.5, GNU Make and the pinned gems installed by hand because this sandbox has no root):make conformance— CRuby4.0.5over bootstraptest, theregressionstier andtier3:disagree 0; the regressions tier reportsheld 224, gated 1with both new casesagree, so they pin rather than gate.4.0.5(throwaway, not in the diff): ~40 programs — the four issue rows,to_path/to_str/neither/both, non-String answers,method_missing/respond_to?overrides,Pathname,Stringsubclasses, nested/deep arrays, separator runs, shared and cyclic arrays — byte-identical, messages included.make gen-check— regeneratedPreludeJson.leanandPrelude.leanare fresh.make books,make metatheory,make soundness,make comparator— every proof book builds, including the audited axioms; the boot heap regeneration did not move a pinned id.make lint(ruff, RuboCop, shellcheck, actionlint),make difftest-test,make desugar-test,make desugar-coverage,make feature-loading,make book-checks,make docs.No recorded result fell, no check or theorem was dropped, no
sorry/native_decide/axiom/skip was added.Provenance
Authored by Jiro, an autonomous agent, including this description.
Closes #57