fix(typeselector): drop a generic candidate the field cannot close - #174
Merged
Merged
Conversation
The candidate scan matched the two generic *definitions* and ignored their arguments, so `GenericToString<TFrom> : IConverter<TFrom, string>` was offered for an `IConverter<float, float>` field — the definitions agree, yet no TFrom makes the closed type fit. Inference then failed (correctly), the caller fell back to the open definition, and the row became a dead end: selecting it opened the argument page, which refused every choice made on it. The whole numeric converter family in Aspid.MVVM shows the row. CanCloseToFieldType now compares the arguments too, position by position, and honours declared variance while doing it — assignability does, so with `IConverter<in TFrom, out TTo>` an `IConverter<object, string>` candidate is still listed for an `IConverter<string, string>` field. A purely structural comparison would have traded this defect for its mirror image, a usable candidate missing from the list. Where variance stops is part of the rule: the CLR only applies it across an implicit reference conversion, so a position the field closed over a value type admits exactly one argument, just like an invariant one. Those pinned positions are matched first and their bindings recorded; the variant positions are judged afterwards, against what the pinned ones already forced. That ordering is what also removes `SequenceConverters<T> : IConverter<T, T>` from an `IConverter<float, string>` field — the same dead row, found while verifying this fix — without depending on which position happens to be declared first. A position that still admits a family of arguments is left alone: proving that none of them converts would mean sweeping the domain, which is what the argument page does anyway, validating each choice through TryConstruct. Verified on the live 6000.4.0f1 Editors. The FastTools EditMode suite is green (352/352, seven new cases; the four negative ones each fail against the previous behaviour). Against MVVM's real `IConverter`, the candidate lists now read: `<float,float>` → GenericFuncConverter, SequenceConverters<Single>; `<float,string>` → GenericFuncConverter, GenericToString<Single>; `<float,object>` → GenericFuncConverter, GenericToString (open, variance); `<string,string>` unchanged. A brute-force cross-check over seven field shapes — every candidate closed over a pool of arguments and tested with IsAssignableFrom — reports no over-rejection. Co-Authored-By: Claude Opus 5 (1M context) <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.
Summary
The candidate scan matched the two generic definitions and ignored their arguments, so a candidate that fixes an argument itself was offered for a field it can never fit:
IConverter<float, float>fieldGenericToString<TFrom>listedGenericToString<TFrom> : IConverter<TFrom, string>is anIConverter<,>, yet noTFrommakes the closed type assignable. Inference then failed (correctly), the caller fell back to the open definition, and the row became a dead end. The whole numeric converter family in Aspid.MVVM showed it.CanCloseToFieldTypenow compares the arguments too, position by position, rather than matching definitions alone.GenericBaseDefinitionsbecame dead and is removed.IConverter<in TFrom, out TTo>anIConverter<object, string>candidate is still listed for anIConverter<string, string>field. A purely structural comparison would have traded this defect for its mirror image — a usable candidate missing from the list, which is not hypothetical: anIConverter<float, object>field legitimately acceptsGenericToString<float>.IsAssignableFromalone does not express this —typeof(object).IsAssignableFrom(typeof(int))istrueby boxing — hence an explicit guard.SequenceConverters<T> : IConverter<T, T>from anIConverter<float, string>field — the same dead row, found while verifying this fix — without depending on which position happens to be declared first.Pair<TKey, TValue>under anIKeyed<string>field keeps its open definition and still opens the argument page. Proving no argument converts would mean sweeping the domain — work the page already does, validating each choice throughTryConstruct.Verification
Live
6000.4.0f1Editor, against Aspid.MVVM's realIConverterhierarchy:IConverter<float, float>GenericFuncConverter<Single, Single>,SequenceConverters<Single>—GenericToStringgone (the reported defect)IConverter<float, string>GenericFuncConverter<Single, String>,GenericToString<Single>—SequenceConverters<open>gone (side finding)IConverter<float, object>GenericFuncConverter<Single, Object>,GenericToString<open>— variance kept the usable rowIConverter<string, string>IsAssignableFrom)Nine cases are added to
GenericTypeResolverTests, covering the repro, the inverse (candidate fixes an argument the field agrees with), the determined and partially-determined candidates, variance on a reference type, two value-type rejections, and a definition implemented twice.Notes for review
run_testscan be driven from this CLI, contrary to the note in fix(typeselector): list a determined generic candidate closed #173:run_tests --mode EditMode --async_tests trueplus pollingtest_statusworks. Only the synchronous mode hits the Pipeline server's 30 s cut-off.SequenceConverters<T>under anIConverter<object, string>field), the pre-filter still passes it — the brute-force sweep reports offered=3 against closable=2. Ruling it out means enumerating the domain, which is the argument page's job; the behaviour predates this change and is documented in the<remarks>.SerializeReferences.mdEN/RU.🤖 Generated with Claude Code