fix: reject a same-kind duplicate method across impl blocks, and remove std's own - #806
Merged
Conversation
…ve std's own two impl blocks giving the same declaration the same method name used to be accepted: the second registration overwrote the first in method_type_map, and which body answered depended on module order. a program calling such a method got whichever implementation happened to register last, silently. the checker now reports E263 at the second declaration. the duplicate check is keyed the way dispatch is keyed: by the owner TID where the impl target resolves to a concrete declaration, and by a module-scoped name for a generic base — a generic base has no tid and its bare name is NOT globally unique (std.iter's MapIter and a user module's MapIter both register "MapIter.next"; the first cut of this rule collided them, and the self-hosted regression gate caught it). only a SAME-KIND collision errors. an interface impl re-declaring a method the inherent impl also has is std/io's deliberate conformance idiom (impl StringReader: fn read alongside impl Reader for StringReader: fn read, identical bodies) and stays legal in either order — the registration records which kinds have declared each method and errors only when the incoming kind is already present. cross-module impls adding NEW method names to a visible type stay legal too. the falsification sweep over the whole tree then found that std itself carried the exact hazard the rule targets, twice: - std/fs re-declared nine FileStream methods that std/io (the struct's home module) also declares — read, read_all, read_bytes, read_all_bytes, write, write_all, write_bytes, write_all_bytes, close — with behaviorally identical bodies through different plumbing. which set answered depended on module registration order. the duplicate impl block and its four now-dead helpers are removed; equivalence was verified pairwise before deletion (same file_read_bytes/file_write_bytes handle calls, same gather-then-decode read_all, same write-all resume loop, same file_close) - std/os/process did the same for ProcessStdin.write/write_all and ProcessStdout/ProcessStderr read/read_all. removed likewise, with the same pairwise equivalence check ## what was tested - new tests/invalid/duplicate_impl_method pins E263 (two inherent impl blocks, same struct, same method); check-invalid 51/51 - tree-wide E263 sweep over std, self-host, examples, tools and bench: zero hits after the std cleanup — the io conformance pairs, the fs and process extensions, and every generic adapter register clean - test_lazy_map_iter (user MapIter beside std.iter's MapIter) passes — it failed under the first, name-keyed cut of the generic dedup and is the reason that path is module-scoped - run-regressions-only 365/365; fs and process examples run (process_ops, fs_ops, file_ops, test_process_command) after the std cleanup - bootstrap seed regenerated; make bootstrap-verify reports the fixed point, including the self-hosted regression and example passes - fmt --check and lint clean on every edited file
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
1 participant
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
Two impl blocks giving the same declaration the same method name used to be accepted: the second registration silently overwrote the first, and which body answered depended on module order. docs/limitations.md warned users to "give methods on a shared type distinct names" — while std itself carried the hazard twice.
the change
Checker. A same-kind duplicate is now E263 at the second declaration. The duplicate check is keyed the way dispatch is keyed: by the owner TID where the impl target resolves concretely, and by a module-scoped name for a generic base — a generic base has no tid and its bare name is not globally unique (std.iter's
MapIterand a user module'sMapIterboth registerMapIter.next; the first cut of this rule collided them and the self-hosted regression gate caught it).Only a same-kind collision errors. An interface impl re-declaring a method the inherent impl also has is std/io's deliberate conformance idiom (
impl StringReader: fn readalongsideimpl Reader for StringReader: fn read, identical bodies) and stays legal in either order. Cross-module impls adding new method names to a visible type stay legal too.std cleanup. The tree-wide falsification sweep found the exact hazard live in std, twice:
std/fsre-declared nineFileStreammethods that std/io (the struct's home module) also declares — identical behavior through different plumbing, winner decided by registration order. Deleted, with pairwise equivalence verified before deletion (same handle calls, same gather-then-decoderead_all, same write-all resume loop, same close).std/os/processdid the same forProcessStdin.write/write_allandProcessStdout/ProcessStderrread/read_all. Deleted likewise.Docs: limitations entry rewritten (rejected now, with the conformance-idiom exception spelled out), E263 added to docs/errors.md.
what was tested
tests/invalid/duplicate_impl_methodpins E263; check-invalid 51/51test_lazy_map_iter(userMapIterbeside std.iter's) passes — it failed under the first name-keyed cut and is why the generic path is module-scopedrun-regressions-only365/365; process_ops, fs_ops, file_ops and test_process_command run correctly after the std cleanupmake bootstrap-verifyreports the fixed point, including the self-hosted regression and example passesfmt --checkand lint clean on every edited file