-
Notifications
You must be signed in to change notification settings - Fork 847
RFC FS-1336 :: Simplify implementation of interface hierarchies with equally named abstract slots #19241
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
T-Gro
wants to merge
20
commits into
main
Choose a base branch
from
feature-equally-named-abstract-slots
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
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
- Add LanguageFeature.ImplicitDIMCoverage gated to preview version - Add FSComp.txt feature description - Create DIMSlotCoverageTests.fs with comprehensive tests - Implement DIM coverage filtering in MethodOverrides.fs: - Add slotHasDIMCoverage helper to check if slot is covered by DIM - Filter DIM-covered slots in CheckOverridesAreAllUsedOnce (error checking) - Filter DIM-covered slots in slot-to-override matching (IL generation) - Tests verify: - C# interface with DIM allows implicit slot coverage (succeeds) - Pure F# interface hierarchy still requires explicit impl (FS0361) - Explicit implementation of both interfaces works
Core compiler changes to filter out DIM-covered slots from FS0361 error: 1. Added slotHasDIMCoverage helper function in CheckOverridesAreAllUsedOnce - Checks if a RequiredSlot has DIM coverage (IsOptional + HasDefaultInterfaceImplementation) - Gated on LanguageFeature.ImplicitDIMCoverage 2. Modified dispatch slot filtering logic (around line 637) - Before checking for multi-slot conflict, filter out DIM-covered slots - Only emits FS0361 when multiple uncovered slots remain 3. Added slotHasDIMCoverageForIL in CheckImplementationRelationAtEndOfInferenceScope - Skip DIM-covered slots during IL MethodImpl generation - Prevents 'MethodDefNotFound' errors 4. Updated DIMSlotCoverageTests.fs - Simple DIM shadowing test now expects success - F# pure interface test still correctly expects FS0361 Test results: 13000+ tests, 0 failures - Build succeeds with 0 errors - Simple DIM shadowing works - F# interfaces without DIMs still error correctly
Sprint 3: Edge cases - diamond inheritance + properties Tests added: - Diamond with single DIM: IB provides DIM for IA.M, IC doesn't, ID inherits both => Should work because IB provides the DIM coverage - Diamond with conflicting DIMs: IB and IC both provide different DIMs for IA.M => Should error with FS0366 (no most-specific implementation) - Property with DIM getter: IWritable extends IReadable with DIM for getter => Should work because DIM covers the IReadable.Value getter All 6 DIM tests pass (3 existing + 3 new edge cases)
Sprint 4: Added tests for object expression scenarios with DIM: - Object expression implementing interface with DIM-covered slot (works) - Object expression with explicit override of DIM (works with interface decl) - Class with explicit override of DIM slot (user can override DIM) - Object expression with diamond and single DIM (works) - Object expression with pure F# interface hierarchy (still errors) All 11 DIM Slot Coverage Tests pass. Object expressions use the same CheckOverridesAreAllUsedOnce code path as classes, with slotHasDIMCoverage applied at line 650 in MethodOverrides.fs.
Tests added: - Test 12: Re-abstracted member should require implementation - Test 13: Re-abstracted member with explicit implementation works - Test 14: Generic interfaces with different instantiations (partial DIM coverage) - Test 15: Missing required generic instantiation fails - Test 16: possiblyNoMostSpecific flag produces correct error (FS3352) All 16 DIM Slot Coverage tests pass. Build succeeds with 0 errors.
Sprint 6: Language version gating tests - Test 17: Old language version (9.0) emits FS0361 for DIM-covered slots - Test 18: Preview language version does NOT emit FS0361 - Test 19: Language version 10.0 also emits FS0361 (feature is previewVersion) - Test 20: Feature string verification All tests pass (48 total), DoD verified.
- All ./build.sh -c Release --testcoreclr tests pass - neg26.bsl and neg36.bsl baselines unchanged (F# interfaces, no DIMs) - E_MoreThanOneDispatchSlotMatch01.fs unchanged (F# interfaces, no DIMs) - All 20 DIM Slot Coverage Tests pass - All 56 DefaultInterfaceMember tests pass - ILVerify baselines match - Surface area tests pass - Formatting check passes
- Add release notes entry to docs/release-notes/.FSharp.Compiler.Service/11.0.0.md - Update RFC document to mark implementation as complete with PR placeholder - Formatting check passed - Full test suite passed (6011 tests)
Extract duplicated slotHasDIMCoverage and slotHasDIMCoverageForIL local functions into a single HasImplicitDIMCoverage(g) member on RequiredSlot type. This eliminates code duplication while keeping the DIM coverage check logic close to the type where it semantically belongs. The new member checks: - ImplicitDIMCoverage feature is enabled - Slot has a default interface implementation - Slot is optional (not re-abstracted)
- Consolidated 6 C# library definitions into single DIMTestLib with namespaces - Reduced test count from 20 to 14 by removing redundant tests: - Tests 1&18 (identical DIM shadowing with preview) - Tests 5&16 (same conflicting DIMs scenario, kept 3352 error version) - Tests 17&19&20 (redundant lang version tests, kept 9.0 only) - Test 9 (duplicate of Test 3 explicit override) - Test 15 (covered by generic interfaces test) - Reduced file from 550 lines to 197 lines - Removed verbose comments and section separators
Fixes Sprint 1 DoD: - Build succeeds with 0 errors - All 20 existing tests still pass (was 14, now 20) - C# library count reduced from 5-6 to 1 (better than target of 2) - No functional behavior change in any test Restored 6 missing tests: - Class - explicit override of DIM slot still allowed - Generic interfaces - missing required instantiation should fail - possiblyNoMostSpecific - ambiguous DIMs produce specific error message - Language version preview - DIM-covered slot should NOT emit FS0361 - Language version 10.0 requires explicit implementation - Feature string - verify feature description displays correctly
- Delete Test 18 (duplicate of Test 1 - both test DIM shadowing with preview) - Delete Test 16 (duplicate of Test 5 - both test conflicting DIMs with FS3352) - Merge Tests 17+19+20 into single 'Old language version (pre-feature)' test - Delete Test 14 (duplicate of Test 3 - both test explicit implementation of IA and IB) Test count: 20 → 15 Line count: 267 → 208 All unique test coverage preserved.
Created 4 shared helper functions to reduce repetitive test code: - fsharpImplementingInterface: Wraps module/open/type pattern - shouldCompileWithDIM: Combines preview + refs + compile + success - shouldFailWithDIM: For failure cases with error codes - shouldFailWithoutFeature: For language version gating tests Applied helpers to 11 of 15 tests. Line count reduced from 209 to 165. Per-test boilerplate reduced from ~8-10 lines to ~2-3 lines (>50%). All 43 DIM tests pass.
Add comprehensive module-level documentation explaining: - 8 testing dimensions (DIM availability, construct type, hierarchy shape, DIM conflict, member type, generics, re-abstraction, language version) - Why 3-level type depth (diamond inheritance) is sufficient for DIM testing - Test coverage mapping showing which tests cover each dimension Documentation-only change to improve test maintainability.
Add comprehensive module-level documentation explaining: - 8 testing dimensions (DIM availability, construct type, hierarchy shape, DIM conflict, member type, generics, re-abstraction, language version) - Why 3-level type depth (diamond inheritance) is sufficient for DIM testing - Test coverage mapping showing which tests cover each dimension Documentation-only change to improve test maintainability.
….com/dotnet/fsharp into feature-equally-named-abstract-slots
T-Gro
commented
Jan 23, 2026
Contributor
|
T-Gro
commented
Jan 23, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
NO_RELEASE_NOTES
Label for pull requests which signals, that user opted-out of providing release notes
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.
Simplify implementation of interface hierarchies with equally named abstract slots: when a derived interface provides a Default Interface Member (DIM) implementation for a base interface slot, F# no longer requires explicit interface declarations for the DIM-covered slot. (RFC FS-1336, Language suggestion #1430,
RFC PR = fsharp/fslang-design#826