WIP :: Feature :: Overloading :: [ORPA; Tiebreakers] #19277
Draft
+3,012
−176
Conversation
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
- Create OverloadResolutionRules.fs with TiebreakRule record type - Extract all 12 existing rules from better() function into DSL format - Add placeholder for new 'MoreConcrete' rule at priority 13 - Include evaluateTiebreakRules function for rule evaluation - Add import of OverloadResolutionRules module to ConstraintSolver.fs - Update FSharp.Compiler.Service.fsproj with new files Rules documented with priority, name, and description: 1. NoTDC - Prefer no type-directed conversions 2. LessTDC - Prefer less type-directed conversion 3. NullableTDC - Prefer nullable TDC only 4. NoWarnings - Prefer no 'less generic' warnings 5. NoParamArray - Prefer no param array usage 6. PreciseParamArray - Prefer more precise param array type 7. NoOutArgs - Prefer no out args 8. NoOptionalArgs - Prefer no optional args 9. UnnamedArgs - Compare unnamed args by subsumption 10. PreferNonExtension - Prefer intrinsic over extension 11. ExtensionPriority - Prefer recently opened extension 12. PreferNonGeneric - Prefer non-generic methods 13. MoreConcrete (placeholder) - Most concrete tiebreaker from RFC 14. NullableOptionalInterop - F# 5.0 all args comparison 15. PropertyOverride - Prefer more derived property type
Add the core type concreteness comparison algorithm as specified in section-algorithm.md: - compareTypeConcreteness: Compares types under the 'more concrete' partial ordering - aggregateComparisons: Implements dominance rule for pairwise comparisons - countTyparConstraints: Helper to count constraints on type parameters Handles all 8 TType cases: - TType_var: Compares by constraint count - TType_app: Compares type arguments pairwise when constructors match - TType_tuple: Compares elements pairwise - TType_fun: Compares domain and range - TType_anon: Compares anonymous record fields - TType_measure: Measures are equal or incomparable - TType_forall: Compares body with aligned bound variables - Default: Different structural forms are incomparable Implements subtask 4 of the RFC FS-XXXX: Most Concrete Tiebreaker.
- Add new tiebreaker rule after 'prefer non-generic methods' (rule 12) - Only activates when BOTH candidates have non-empty CalledTyArgs - Uses compareTypeConcreteness to compare type arguments - Applies dominance rule: better in at least one position, not worse in any - Positioned before F# 5.0 NullableOptionalInterop rule - Update DSL placeholder documentation to reflect implementation
- Create tests/FSharp.Compiler.ComponentTests/Conformance/Tiebreakers/ directory - Add TiebreakerTests.fs with module structure and helper functions - Include placeholder test and RFC example tests - Configure for Debug build and net10 TFM - All 3 tests pass
- Example 1: Option<'t> vs Option<int> - resolves to more concrete - Example 2: 't vs Option<'t> - expects ambiguity (structural comparison not yet implemented) - Example 3: Option<Option<'t>> vs Option<Option<int>> - resolves to nested int - Example 4: list<Option<Result<'t,exn>>> vs list<Option<Result<int,exn>>> - deep nesting works All 7 tiebreaker tests pass with dotnet test --filter Tiebreakers -c Debug
- Add test for Result<int,string> selecting fully concrete overload (Example 5) - Add test for incomparable types (Result<int,'e> vs Result<'t,string>) producing FS0041 (Example 6) - Add tests verifying partial order behavior with helpful error messages - Document current limitation: partial concreteness comparison between fully generic and partially concrete types remains ambiguous - Add additional tests for tuple-like scenarios and three-way comparisons All 14 tiebreaker tests pass.
Implement tests for the primary motivating use cases:
- Example 7: ValueTask constructor scenario (Task<'T> vs 'T disambiguation)
- Example 8: CE Source overloads (FsToolkit AsyncResult pattern)
- Example 9: CE Bind with Task types (TaskBuilder pattern)
Tests document both working cases (Task<'a> vs 't with wrapped types)
and cases that remain ambiguous pending structural comparison implementation
('T vs Task<'T> when type shapes differ).
Additional real-world pattern tests:
- Source with Result types vs generic
- Nested Task<Result<...>> types
All 22 tiebreaker tests pass.
- Example 10: Verify existing Rule 8 (prefer no optional) takes priority over the 'more concrete' tiebreaker - Example 11: When both overloads have optional params, concreteness breaks the tie (Option<int> vs Option<'t>) - Example 12: ParamArray with generic element types - concreteness resolves Option<int>[] vs Option<'t>[] Additional coverage: - Multiple optional params test - Nested generics with optional params - ParamArray with Result element types - Combined optional and ParamArray scenarios - ParamArray vs explicit array ambiguity documentation All 33 Tiebreaker tests pass.
Tests covering: - Intrinsic methods ALWAYS preferred over extensions (Rule 8) - Less concrete intrinsic still wins over more concrete extension - Same-module extensions resolved by concreteness - SRTP resolution following same rules - C# style extensions in F# - Extension priority precedence over concreteness - Incomparable concreteness remains ambiguous - FsToolkit pattern documentation
Implements subtask 12: byref and Span tests from section-byref-span.md Tests added: - Span<byte> vs Span<'T> selecting concrete element type - ReadOnlySpan element type comparison (concrete vs generic) - T > inref<T> adhoc rule verification - Span with nested generics (Option<int> vs Option<'T>) - inref with nested generics (Result<int,exn> vs Result<'T,exn>) - Adhoc rule priority over concreteness test All 53 tiebreaker tests pass.
Implements subtask 13: Adds tests for constraint and TDC interaction: - Example 15 constrained type variable tests (documents FS0438 limitation) - TDC priority tests (no TDC > less TDC > concreteness) - Func adhoc rule interaction tests - Nullable adhoc rule interaction tests (T > Nullable<T>) Tests verify TDC rules have higher priority than concreteness tiebreaker, and adhoc rules (Func, inref, Nullable) apply before concreteness. All 65 tiebreaker tests pass.
Adds 27 new test scenarios beyond RFC examples, covering: - SRTPs with generic vs concrete (3 tests) - Byref/inref/outref combinations (4 tests) - Anonymous record types (3 tests) - Units of measure (4 tests) - F#-specific types: Async, MailboxProcessor, Lazy, Choice, ValueOption, ValueTask (8 tests) - Computation expressions: seq, list, async (3 tests) - Discriminated unions: Result, custom Tree (2 tests) All 91 tiebreaker tests pass.
…ameter types Sprint 1: The core algorithm fix Changes: - Compare formal (uninstantiated) parameter types using FormalMethodInst instead of comparing CalledTyArgs (which are already instantiated after type inference) - This enables proper comparison for cases like: - 't vs Option<'t> (Option wins - more concrete structure) - Task<'T> vs 'T (Task wins for Task<int> argument) - Async<Result<'ok,'e>> vs Async<'t> (Result wins for Result argument) - Result<int,'e> vs Result<'ok,'e> (partial concreteness - int wins) The fix correctly resolves overloads where one method's parameter type has more concrete structure than another, even when both methods are generic. Updated 7 tests from shouldFail to shouldSucceed: - Example 2: 't vs Option<'t> - Example 5: Partial concreteness (int ok, string error) - Example 7: ValueTask Task<T> vs T - Example 8: CE Source FsToolkit pattern - Real-world pattern: Source with Result types - FsToolkit pattern: same module extensions All 95 tiebreaker tests pass.
…gured Verified DoD: - Build succeeds with 0 errors - All 91 tiebreaker tests pass - Example 2,5,7,8: use shouldSucceed (correctly) - Example 6: uses shouldFail for incomparable case (correctly) No code changes needed - Sprint 1 already configured expectations properly.
- Add FSComp.txt entries for FS3575 (tcMoreConcreteTiebreakerUsed) and FS3576 (tcGenericOverloadBypassed) - Register both warnings as off by default in CompilerDiagnostics.fs - Add wasConcretenessTiebreaker helper in ConstraintSolver.fs to detect when concreteness rule decided - Emit warning when concreteness tiebreaker is used and --warnon:3575 is enabled - Add tests verifying warning is emitted when enabled and not emitted by default
- Add MoreConcreteTiebreaker to LanguageFeature enum (F# 10.0) - Gate concreteness tiebreaker logic in better() with SupportsFeature check - Gate wasConcretenessTiebreaker helper similarly - Add feature string to FSComp.txt
Sprint 2 audit: Verified all 14 implementable RFC examples from section-examples.md have corresponding tests in TiebreakerTests.fs. Coverage mapping: - Examples 1-14: All tested with explicit test names and line numbers - Example 15: Confirmed deferred (FS0438 language limitation) 93/93 tiebreaker tests passing.
Sprint 3: Documentation-only changes to track deferred future work: 1. TiebreakerTests.fs: Enhanced comment block for Example 15 test explaining F# language limitation (FS0438 prevents constraint-only overloading) 2. ConstraintSolver.fs: Added TODO comment at aggregateComparisons function for future enhanced FS0041 error message that explains why types are incomparable 3. VISION.md: Updated deferred items with code location cross-references No functional changes - all 93 tiebreaker tests pass.
- Remove .ralph/ folder (workflow artifacts not needed in final PR) - Add docs/TIEBREAKERS_DESIGN.md with comprehensive feature documentation - Release notes have PR number placeholders (to be updated when PR is created)
…utionRules Sprint 1 deliverables: - Add aggregateComparisons helper for dominance-based comparison - Add compareTypeConcreteness function for type concreteness ordering - Export both functions in signature file - OverloadResolutionContext already has all needed fields (g, amap, m, ndeep) These functions are now available for use by the rule engine in future sprints. The implementation matches the existing algorithm in ConstraintSolver.fs but is now accessible from OverloadResolutionRules.fs.
- Replace ~140 lines of duplicated if-then-else chains in ConstraintSolver.fs with calls to evaluateTiebreakRules() and wasDecidedByRule() from OverloadResolutionRules module - Fix moreConcreteRule to actually perform the comparison (was placeholder returning 0) - Add wasDecidedByRule helper to check if a specific rule was the deciding factor - Remove unused local helper functions from ConstraintSolver.fs - All 93 tiebreaker tests pass This eliminates the code duplication identified in VISION.md between better() and wasConcretenessTiebreaker().
This function computes position-by-position comparison results when two types are incomparable under the concreteness ordering. It returns Some with a list of (position, ty1Arg, ty2Arg, comparison) tuples when types have mixed results (incomparable), or None when one type dominates or they are equal. This is Sprint 1 of the enhanced FS0041 error message implementation.
- Add IncomparableConcretenessInfo type and explainIncomparableMethodConcreteness function - Extend PossibleCandidates to carry incomparable concreteness details - Format enhanced message in CompilerDiagnostics.fs showing which type args favor each method - Add csIncomparableConcreteness message resource to FSComp.txt - Update test to verify enhanced message content When overload resolution fails due to incomparable type concreteness, the error message now explains: 'Neither candidate is strictly more concrete than the other: - Compare is more concrete at position 1 - Compare is more concrete at position 2'
All DoD items verified: - Build succeeds with 0 errors - Enhanced FS0041 message shows per-position concreteness details - FSComp.txt string resource csIncomparableConcreteness added - All 93 TiebreakerTests pass The functionality was already implemented in earlier sprints.
DoD verified: - Build succeeds with 0 errors - Test at TiebreakerTests.fs:252-268 verifies enhanced FS0041 message - Test checks for 'Neither candidate is strictly more concrete' - Test checks for position-specific concreteness explanation - All 93 TiebreakerTests pass
… 15) - Add countTypeParamConstraints helper to count effective constraints (CoercesTo, IsNonNullableStruct, IsReferenceType, MayResolveMember, etc.) - Update compareTypeConcreteness to compare constraint counts when both types are type variables (RFC section-algorithm.md lines 136-146) - Remove 'deferred' comments as constraint comparison is now implemented - Update Example 15 test to expect success (constrained overload wins) All 97 tiebreaker tests pass.
Add 8 new tests in 'LangVersion Latest Tests' section that verify: - Existing rules work under langversion=latest (2 tests) - Non-generic overload preferred over generic - Non-extension method preferred over extension - MoreConcrete tiebreaker disabled under langversion=latest (2 tests) - Fully generic vs wrapped generic remains ambiguous (FS0041) - Array generic vs bare generic remains ambiguous (FS0041) - ORP attribute silently ignored under langversion=latest (3 tests) - Higher priority does not win - Negative priority has no effect - Priority does not override concreteness - Default langversion behaves same as explicit latest (1 test) All 132 tiebreaker tests pass.
Contributor
✅ No release notes required |
# Conflicts: # docs/release-notes/.Language/preview.md # src/Compiler/Checking/ConstraintSolver.fs # src/Compiler/FSComp.txt
…into feature/tiebreakers
… with FactForNETCOREAPP
…feedback CODE-QUALITY: - Replace duplicated dominance logic in compareArgLists and unnamedArgsRule with aggregateComparisons - Use tryTcrefOfAppTy instead of tcrefOfAppTy in filterByOverloadResolutionPriority - Use RequiredFeature field on nullableOptionalInteropRule instead of inline lang version check PERF: - Hoist getAllTiebreakRules to module-level value (avoid per-call allocation) - Allocate OverloadResolutionContext once in GetMostApplicableOverload - Add findDecidingRule to combine comparison + rule identification in one pass - Replace wasDecidedByRule with findDecidingRule in wasConcretenessTiebreaker - Single-pass aggregateComparisons using List.fold instead of two List.exists - Combine double attribute decoding in filterByOverloadResolutionPriority - Remove redundant filterByOverloadResolutionPriority call in GetMostApplicableOverload TEST-CODE-QUALITY: - Remove unused shouldCompile and shouldFailWithAmbiguity helpers - Use CSharpFromPath for csharpPriorityLib instead of inline copy - Extract shared concretenessWarningSource for 3575/3576 warning tests - Parameterize 11 wrapper type tests into a single Theory test
…DE-QUALITY feedback CODE-QUALITY: - evaluateTiebreakRules now delegates to findDecidingRule (eliminates duplicate loop) - Remove dead code: wasDecidedByRule (zero callers after fixup iteration 1) - Remove unnecessary API surface: getAllTiebreakRules (zero external callers) PERF: - Cache deciding rules from pairwise comparisons in Dictionary; concreteness warning check uses cache lookup instead of re-evaluating all 15 rules - Reuse existing OverloadResolutionContext (ctx) in incomparableConcretenessInfo instead of allocating a duplicate record TEST-CODE-QUALITY: - Parameterize 16 remaining structurally identical tests into concreteWrapperTestCases (12 Fact tests) and concreteWrapperNetCoreTestCases (4 FactForNETCOREAPP tests via TheoryForNETCOREAPP) - Net reduction: ~319 lines
- Remove evaluateTiebreakRules (zero callers, delegates to findDecidingRule) - Remove explainIncomparableConcreteness and collectTypeArgComparisons (zero callers) - Remove aggregateComparisons and compareTypeConcreteness from .fsi (internal-only use)
…urface, fix test quality and coverage CODE-QUALITY: - Remove unused FS3590 (tcOverloadResolutionPriorityUsed) from FSComp.txt, CompilerDiagnostics.fs, and 14 xlf translations - Hide TiebreakRule record from .fsi (only TiebreakRuleId is needed externally) - Remove unused Features import from .fsi TEST-CODE-QUALITY: - Remove duplicate 'Warning 3576 - Off by default' test (identical to 3575 off-by-default test) - Remove semantic duplicate 'Adhoc rule - T preferred over Nullable T' (same scenario as TDC priority test) - Parameterize 3 'Extension methods in same module' tests into Theory with MemberData - Strip redundant comments from test file TEST-COVERAGE: - Add 'MoreConcrete - Both generic, function type parameter' test (exercises Case 5: TType_fun) - Add 'MoreConcrete - Both generic, tuple type parameter' test (exercises Case 4: TType_tuple) - Add 'MoreConcrete - Both generic, Option of list vs Option of generic' test - Add 'SRTP skip - Both generic with SRTP produces ambiguity' negative test
…meterize Optional/ParamArray tests NO-LEFTOVERS: Remove agent-generated docs (TIEBREAKERS_DESIGN.md, TIEBREAKERS_DIAGRAM.md, FS-XXXX-most-concrete-tiebreaker.md) and redundant inline comments from ConstraintSolver.fs, OverloadResolutionRules.fs, and test files. TEST-CODE-QUALITY: Remove duplicate 'RFC Example' test (identical to 'TDC priority - Concreteness applies only when TDC is equal') and duplicate 'LangVersion Latest' test (identical to 'Non-generic overload is preferred over generic'). Parameterize 4 'Example 11 - Both Have Optional' tests and 3 'Example 12 - ParamArray' tests into Theory with MemberData, following established concreteWrapperTestCases pattern. All 109 tests pass (108 tiebreaker + 1 ORP), 0 failed, 0 skipped.
…rning 3576 test, remove placeholder test
…feedback PERF fixes: - Use struct tuples in findDecidingRule and TiebreakRule.Compare to avoid 3 heap allocations per pairwise comparison (hot path) - Precompute warning list lengths once per method via cache in GetMostApplicableOverload instead of per-pair List.length - Inline 2-element aggregation in TType_fun case of compareTypeConcreteness to avoid list allocation TEST-CODE-QUALITY fixes: - Merge duplicate Example 6 tests (identical source, first was subset) - Combine Example 5 partial concreteness tests into Theory with InlineData - Extract shared csharpPriorityLib to SharedTestHelpers.fs used by both TiebreakerTests and OverloadResolutionPriorityTests NO-LEFTOVERS fixes: - Remove stale RFC FS-XXXX references from release notes, comments, and test module doc
…ncreteRule, remove redundant doc comments PERF: Add paramDataCache and srtpCache to OverloadResolutionContext to eliminate redundant GetParamDatas+List.concat calls and SRTP traversals across pairwise comparisons in moreConcreteRule (rule 13). NO-LEFTOVERS: Remove 6 doc comments that restate function/module names.
…S feedback PERF: - Replace List.map2+aggregateComparisons with aggregateMap2 (direct fold with early exit on incomparability) in compareTypeConcreteness, compareArgLists, and moreConcreteRule — eliminates intermediate list allocations on the overload resolution hot path - Remove list concatenation via @ in unnamedArgsRule; aggregate obj-arg and unnamed-arg comparisons separately then combine results - Add early exit in filterByOverloadResolutionPriority: check for any non-zero priority before allocating tuple list (99% fast path) - Guard concretenessWarns List.choose behind check if any MoreConcrete rule was actually used as deciding factor TEST-COVERAGE: - Convert 3 key MoreConcrete tests (Example 1, Example 5, Three-way) from typecheck|>shouldSucceed to compileAndRun with runtime return-value assertions verifying the correct overload is selected NO-LEFTOVERS: - Commit previously uncommitted test deduplication (Theory/MemberData in ORP tests) and redundant XML doc comment removal in CSharpPriorityLib
… runtime assertions
…roup dominance CODE-QUALITY: The refactored unnamedArgsRule was reducing obj-arg and unnamed-arg comparisons into separate sub-group results before combining, which lost cross-group incomparability information. Restored the original semantics: all individual arg comparisons are collected into a single flat list, then dominance (List.forall/exists) is applied over the entire list. NO-LEFTOVERS: Commit the pending CSharpPriorityLib.cs doc comment cleanup.
T-Gro
force-pushed
the
feature/tiebreakers
branch
from
8be8dbb to
10addb1
Compare
- Add aggregateComparisons helper for single-pass dominance over int lists - Use aggregateComparisons in unnamedArgsRule instead of two-pass List.forall/List.exists - Fix double GetOverloadResolutionPriority() call in filterByOverloadResolutionPriority by computing priorities once in enrichment pass, then checking for non-zero
- Add moreConcreteTestCases Theory (5 cases) exercising the new MoreConcrete tiebreaker with both-generic overloads - Add function type concreteness test (TType_fun code path) - Add tuple type concreteness test (TType_tuple code path) All 112 tiebreaker tests pass.
…ncreteTestCases Theory - Remove Example 2 (duplicate of moreConcreteTestCases 'T vs Option<'T>) - Remove MoreConcrete Option-of-list test (duplicate of moreConcreteTestCases entry) - Remove standalone function-type and tuple-type [<Fact>] tests - Absorb 4 standalone tests into moreConcreteTestCases Theory: function-type concrete range, tuple-type concrete element, function-type concrete domain, tuple-type concrete first element
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.
No description provided.