Fixup #1 (iteration 2): Address remaining CODE-QUALITY, PERF, TEST-CO… · dotnet/fsharp@d8172a9 · GitHub
Skip to content

Commit d8172a9

Browse files
T-GroT-Gro
committed
Fixup #1 (iteration 2): Address remaining CODE-QUALITY, PERF, TEST-CODE-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
1 parent 4c1662d commit d8172a9

4 files changed

Lines changed: 80 additions & 399 deletions

File tree

‎src/Compiler/Checking/ConstraintSolver.fs‎

Lines changed: 11 additions & 14 deletions

‎src/Compiler/Checking/OverloadResolutionRules.fs‎

Lines changed: 8 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -647,37 +647,14 @@ let private allTiebreakRules: TiebreakRule list =
647647
propertyOverrideRule
648648
]
649649

650-
/// Get all tiebreaker rules in priority order (ascending by TiebreakRuleId value).
651-
let getAllTiebreakRules () : TiebreakRule list = allTiebreakRules
652-
653650
/// Helper to check if a rule's required feature is supported
654651
let private isRuleEnabled (context: OverloadResolutionContext) (rule: TiebreakRule) =
655652
match rule.RequiredFeature with
656653
| None -> true
657654
| Some feature -> context.g.langVersion.SupportsFeature(feature)
658655

659-
/// Evaluate all tiebreaker rules to determine which method is better.
660-
/// Returns >0 if candidate is better, <0 if other is better, 0 if they are equal.
661-
let evaluateTiebreakRules
662-
(context: OverloadResolutionContext)
663-
(candidate: CalledMeth<Expr> * TypeDirectedConversionUsed * int)
664-
(other: CalledMeth<Expr> * TypeDirectedConversionUsed * int)
665-
: int =
666-
667-
let rec loop rules =
668-
match rules with
669-
| [] -> 0
670-
| rule :: rest ->
671-
if isRuleEnabled context rule then
672-
let c = rule.Compare context candidate other
673-
if c <> 0 then c else loop rest
674-
else
675-
loop rest
676-
677-
loop allTiebreakRules
678-
679656
/// Evaluate all tiebreaker rules and return both the result and the deciding rule.
680-
/// Returns (result, Some ruleId) if a rule decided, or (0, None) if all rules returned 0.
657+
/// Returns (result, ValueSome ruleId) if a rule decided, or (0, ValueNone) if all rules returned 0.
681658
let findDecidingRule
682659
(context: OverloadResolutionContext)
683660
(candidate: CalledMeth<Expr> * TypeDirectedConversionUsed * int)
@@ -696,28 +673,14 @@ let findDecidingRule
696673

697674
loop allTiebreakRules
698675

699-
/// Check if a specific rule was the deciding factor between two methods.
700-
/// Returns true if all rules BEFORE the specified rule returned 0, and the specified rule returned > 0.
701-
let wasDecidedByRule
702-
(ruleId: TiebreakRuleId)
676+
/// Evaluate all tiebreaker rules to determine which method is better.
677+
/// Returns >0 if candidate is better, <0 if other is better, 0 if they are equal.
678+
let evaluateTiebreakRules
703679
(context: OverloadResolutionContext)
704-
(winner: CalledMeth<Expr> * TypeDirectedConversionUsed * int)
705-
(loser: CalledMeth<Expr> * TypeDirectedConversionUsed * int)
706-
: bool =
707-
let rec loop rules =
708-
match rules with
709-
| [] -> false
710-
| rule :: rest ->
711-
if isRuleEnabled context rule then
712-
let c = rule.Compare context winner loser
713-
714-
if rule.Id = ruleId then c > 0 // The specified rule decided in favor of winner
715-
elif c <> 0 then false // An earlier rule decided, so the specified rule wasn't the decider
716-
else loop rest
717-
else
718-
loop rest
719-
720-
loop allTiebreakRules
680+
(candidate: CalledMeth<Expr> * TypeDirectedConversionUsed * int)
681+
(other: CalledMeth<Expr> * TypeDirectedConversionUsed * int)
682+
: int =
683+
fst (findDecidingRule context candidate other)
721684

722685
// -------------------------------------------------------------------------
723686
// OverloadResolutionPriority Pre-Filter (RFC: .NET 9 attribute)

‎src/Compiler/Checking/OverloadResolutionRules.fsi‎

Lines changed: 0 additions & 12 deletions

0 commit comments

Comments
 (0)