Extension members solve SRTP constraints (continuation)#19602
Extension members solve SRTP constraints (continuation)#19602gusty wants to merge 147 commits intodotnet:mainfrom
Conversation
- Add LanguageFeature.ExtensionConstraintSolutions mapped to preview version - Gate FS1215 (tcMemberOperatorDefinitionInExtrinsic) behind the feature flag - With --langversion:preview, extension operator definitions no longer warn - Add FSComp.txt entry and xlf translations for the feature description
…with traitCtxt field - Add ITraitAccessorDomain marker interface to TypedTree - Add ITraitContext interface with SelectExtensionMethods and AccessRights - Extend TTrait with 8th field: traitCtxt: ITraitContext option - Add TraitContext member and traitCtxtNone helper - Make AccessorDomain implement ITraitAccessorDomain - Update all TTrait pattern matches across the compiler (8 files) - Pickling: traitCtxt is not serialized; deserialized as None - No behavioral change: all trait contexts are None Sprint 2 of RFC FS-1043 (extension method SRTP resolution)
…functions - Add traitCtxt: ITraitContext option parameter to CopyTyparConstraints, FixupNewTypars, FreshenAndFixupTypars, FreshenTypeInst, FreshMethInst, FreshenMethInfo, FreshenTypars, CopyAndFixupTypars, FreshenPossibleForallTy, LightweightTcValForUsingInBuildMethodCall, FreshenTyconRef, FreshenTyconRef2, FreshenAbstractSlot, FreshenObjectArgType - CopyTyparConstraints stamps traitCtxt onto MayResolveMember constraints when the original has None - TcEnv implements ITraitContext with SelectExtensionMethods and AccessRights - Add SelectExtensionMethInfosForTrait to NameResolution.fs - Thread traitCtxtNone through all call sites (no behavioral change)
…ll call sites with TcEnv in scope - TTrait construction sites in TcPseudoMemberSpec and TcImplicitOpItemThen now use env.TraitContext instead of None - All 28 freshening call sites in CheckExpressions.fs use env.TraitContext (or envinner.TraitContext where the variable is named envinner) - CheckDeclarations.fs call sites use env/envForTycon/tcEnv.TraitContext - CheckExpressionsOps.fs CompilePatternForMatch uses env.TraitContext - CopyTyparConstraints traitCtxt propagation is no longer dead code
- GetRelevantMethodsForTrait: collect extension methods from trait context when ExtensionConstraintSolutions feature is enabled - SolveMemberConstraint: compute traitAD from trait context for accessibility - Use traitAD in CalledMeth, ResolveOverloading, TryFindIntrinsicNamedItemOfType - Use traitCtxt (not traitCtxtNone) in FreshenMethInfo calls - Built-in rules use intrinsicMinfos in match pattern; when clauses use full minfos so extension methods properly defer built-in resolution - traitsAEquiv already ignores traitCtxt field (verified)
… tests - Change typecheck to compileAndRun in 'Extension operator on string resolves with langversion preview' test to validate runtime behavior - Add test: FS1215 warning fires for extension operator without langversion preview - Add test: FS1215 warning does not fire with langversion preview Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ring optimization When FSharp.Core inline operators are expanded, the resulting trait calls have no ITraitContext (it's not serialized in metadata). This prevented extension methods defined in the current compilation unit from being found during optimization/codegen, causing Error 193 type mismatches. Changes: - ConstraintSolver: Add guard preventing built-in mul/div rule from firing for concrete non-numeric types when ExtensionConstraintSolutions is enabled - ConstraintSolver: Add CreateImplFileTraitContext that walks the expression tree bindings (not CcuThunk module type) to find extension member Val objects with correct stamps matching IlxGen's bound vals - Optimizer: Add traitCtxt field to cenv, supply fallback trait context in OptimizeTraitCall when trait's original context is None Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Test B5: Multiple extension operators with different signatures resolve correctly - Test B6: Intrinsic operator takes priority over extension with same name and signature Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add 5 new tests covering: - Extension operator resolves via SRTP alongside IWSAM types (B7) - Extension wins over IWSAM interface impl for same operator (B7) - Extension operator not visible without opening defining module (B9) - Inline SRTP resolves using consumer's scope for extensions (B9) - Internal record field resolves via SRTP within same compilation unit (B10) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Test 1: True optional extension on System.String (*) across assemblies. Documents that cross-assembly extension resolution fails (Error 193) because TTrait.traitCtxt deserializes as None from pickled metadata. - Test 2: Intrinsic augmentation Widget (+) across assemblies. Confirms intrinsic operators work cross-assembly (for contrast). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove three tests that were near-identical duplicates exercising the same 'define record + extension (+) + inline add' pattern: - 'Inline SRTP function uses extension method on custom type' (duplicate of 1910) - 'Extension operator resolves via SRTP alongside IWSAM types' (duplicate, misleading name) - 'Extension operator on custom type typechecks in separate module' (subset of Intrinsic method priority test) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add defensive type-test assertions replacing unsafe obj downcasts in CheckBasics.fs (InfoReader), ConstraintSolver.fs (AccessorDomain, MethInfo) - Hoist ExtensionConstraintSolutions feature flag check to local binding in SolveMemberConstraint and GetRelevantMethodsForTrait - Extract SelectExtMethInfosForType shared helper in NameResolution.fs and refactor SelectExtensionMethInfosForTrait to use it Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… resolution Define AllowOverloadOnReturnTypeAttribute in FSharp.Core and wire it into the compiler's overload resolution and uniqueness checking. When any applicable overload carries this attribute, the return type is considered during overload resolution, generalizing the existing op_Explicit/op_Implicit mechanism to arbitrary methods. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…eftover files Update FSharp.Core surface area baseline to include the new AllowOverloadOnReturnTypeAttribute type. Remove leftover code review artifact files that were accidentally committed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…InfosForType Extract SelectIndexedExtMethInfosForType helper for indexed-only extension member lookup. Refactor ExtensionMethInfosOfTypeInScope to use SelectExtMethInfosForType for the root type (both indexed and unindexed members) and SelectIndexedExtMethInfosForType for base types in the hierarchy (indexed only), removing the duplicated tryTcrefOfAppTy / eIndexedExtensionMembers.Find / SelectMethInfosFromExtMembers pattern. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…sForType Eliminate code duplication by having SelectExtMethInfosForType delegate its indexed lookup to SelectIndexedExtMethInfosForType instead of duplicating the tryTcrefOfAppTy/Find/SelectMethInfosFromExtMembers pattern. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ions Skip SRTP constraint canonicalization for inline bindings when the ExtensionConstraintSolutions feature is enabled (--langversion:preview). This implements RFC FS-1043 claim dotnet#6: weak resolution should not force inline code to collapse generic SRTP constraints to concrete types. Changes: - CheckExpressions.fs: Skip CanonicalizePartialInferenceProblem for inline bindings in TcLetBinding, TcObjectExprBinding, and TcIncrementalLetRecGeneralization when ExtensionConstraintSolutions is on. - IWSAMsAndSRTPsTests.fs: Add 6 tests covering inline DateTime generics, non-inline unaffected, backward compat with langversion 8.0, and built-in numeric operators. Non-inline code is completely unaffected. Gated behind langversion:preview. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Enhance CreateImplFileTraitContext to search referenced CCU module types for extension member Vals, not just local impl file bindings. This enables cross-assembly SRTP constraint resolution during optimization/codegen when extension operators are defined in a referenced assembly. Changes: - ConstraintSolver.CreateImplFileTraitContext: accept CcuThunk list, walk referenced CCU module types to collect extension members - Optimizer.IncrementalOptimizationEnv: add referencedCcus field populated by BindCcu - Optimizer.OptimizeImplFile: pass referencedCcus to CreateImplFileTraitContext - Tests: change cross-assembly test from shouldFail to compileAndRun/shouldSucceed, remove TODO comment, add transitive A→B→C test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove Skip from 'AllowOverloadOnReturnType resolves overloads by return type' test now that AllowOverloadOnReturnTypeAttribute is available in FSharp.Core. Add test for ambiguity error when no type annotation is present. Add test for mixed attributed/non-attributed overloads. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add 4 tests covering SRTP weak resolution behavior with langversion preview: - FSharpPlus-style InvokeMap/InvokeApply pattern compiles with preview - Type annotation workaround for InvokeMap pattern compiles - Non-inline code canonicalization unaffected by ExtensionConstraintSolutions - Inline numeric operators with multiple overloads stay generic Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…nType assertions - Remove duplicate 'Non-inline DateTime addition still resolves concretely' test (identical to 'Non-inline code canonicalization is unaffected') - Fix 'AllowOverloadOnReturnType resolves overloads by return type' to assert shouldFail (attribute not yet functional for return-type disambiguation) - Fix 'AllowOverloadOnReturnType mixed' to assert shouldFail (attribute type not yet defined in FSharp.Core) All 227 IWSAM/SRTP tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add 4 tests covering instance extension methods with SRTP constraints: - Instance extension method resolves via SRTP - Instance extension method with parameter resolves via SRTP - Instance extension does not satisfy static SRTP constraint - Intrinsic instance method takes priority over instance extension Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace 3 failwith calls with error(InternalError(...)) in ITraitContext downcast code paths to produce proper compiler diagnostics instead of opaque exceptions: - CheckBasics.fs: SelectExtensionMethods InfoReader cast - ConstraintSolver.fs: SolveMemberConstraint AccessorDomain cast - ConstraintSolver.fs: GetRelevantMethodsForTrait MethInfo cast Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add 4 new tests covering accessibility-domain scenarios for SRTP: - Internal type extension in same assembly resolves via SRTP - SRTP constraints from different accessibility domains flow together - Internal record field resolves via SRTP within same assembly - Cross-assembly internal extension is not visible via SRTP Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Rename _collectionSettings back to collectionSettings and add early-exit check after rootResults to skip expensive type hierarchy lookup when only one result is needed (AtMostOneResult). This restores the perf optimization that was lost during RFC FS-1043 refactoring. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Document extension members for operators and SRTP constraints in preview release notes - Add SRTP guide covering extension constraints, priority rules, scope capture, weak resolution changes, and workarounds - Feature flag: ExtensionConstraintSolutions (--langversion:preview) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove false 'known limitation' about cross-assembly resolution from release notes and srtp-guide.md (cross-assembly resolution works) - Clarify AllowOverloadOnReturnType: attribute allows defining overloads, but full call-site disambiguation is not yet implemented - Add cross-assembly resolution as an explicit feature bullet in release notes - Fix stale test comment about AllowOverloadOnReturnType not being in FSharp.Core Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add AllowOverloadOnReturnTypeAttribute to all surface area baselines (debug ns2.0, debug ns2.1, release ns2.0 were missing the entry) - Add withErrorCode to all 7 shouldFail tests without error verification: - Extension operator not visible: error 1 - AllowOverloadOnReturnType overloads by return type: error 41 - Overloads without AllowOverloadOnReturnType: error 41 - AllowOverloadOnReturnType with no annotation: error 41 - Instance extension does not satisfy static SRTP: error 1 - Cross-assembly internal extension not visible: error 43 - Fix mixed overloads test: overloads with different param types (string vs int) resolve without ambiguity — changed to shouldSucceed - Remove AllowOverloadOnReturnType attribute from shouldFail tests since the attribute is not available in the test runtime's FSharp.Core - Parameterize DateTime addition/subtraction tests into a single Theory - Extract stringRepeatExtLib helper to eliminate duplicated string extension operator library definitions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…nition - Merge 'AllowOverloadOnReturnType resolves overloads by return type' and 'Overloads without AllowOverloadOnReturnType produce ambiguity error' into single 'Overloads differing only by return type produce ambiguity error' test (identical source and assertions) - Extract stringRepeatExtDef [<Literal>] constant for inline string repeat extension definition, reducing 5 inline duplicates to 1 - Update stringRepeatExtLib to reuse stringRepeatExtDef Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Tests that a C#-style extension method Append<T>(this T[] a, T[] b) resolved via SRTP inside <@ @> produces a correct quotation tree that evaluates successfully at runtime via reflection. Verifies the minst fix-up for unsolved typars and quotation encoding of generic method type parameters work correctly together. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Test that an F# extrinsic extension operator on System.String defined in a separate library can be used inside <@ @> quotations and evaluated via LeafExpressionConverter.EvaluateQuotation. Covers FSMethSln path in QuotationTranslator for cross-assembly resolution. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Tests generic type arg instantiation in quotation tree when using F# extension operator (|>>) on Option<'T> via inline SRTP inside <@ @>. Covers cross-assembly method resolution and runtime evaluation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Test cross-assembly user-defined generic type with extension operator in quotations. Verifies type arg instantiation in quotation tree for non-FSharp.Core generic types with both int and string instantiations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Verifies that extension operators on discriminated union types with generic type parameters (Tree<'T>) correctly resolve and evaluate inside quotations via SRTP constraint resolution. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Test verifies [<ReflectedDefinition>] correctly stores quotation trees when a non-inline function calls an inline SRTP function using an extension operator (+) on a custom Widget type. Validates: - Direct execution produces correct result - ReflectedDefinition is retrievable via Expr.TryGetReflectedDefinition - Quotation evaluation produces correct result Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add T6 (StringRep with <*> extension operator) and T7 (generic Wrapper with ++ extension operator) to verify binary compatibility of extension operators across compiler versions via concrete wrappers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add test verifying that 'open type StringOps' makes static operators on StringOps available for SRTP constraint resolution. Compiler changes: - NameResolution: add eOpenedTypeOperators field to NameResolutionEnv, populated by AddStaticContentOfTypeToNameEnv for operator MethInfos, searched by SelectExtensionMethInfosForTrait - ConstraintSolver: add staticOperatorsByName collection to CreateImplFileTraitContext for optimization-phase SRTP resolution (operators from open type scopes lose traitCtxt after pickling) - Filter out operators on support types from staticOperatorsByName to avoid changing resolution priority (intrinsics already found) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add single-file and cross-assembly tests verifying that F#-authored C#-style extension methods using [<Extension>] attribute are correctly resolved via SRTP constraint resolution. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… via SRTP Verify that extension members defined in an internal module of a referenced assembly are not accessible through SRTP constraint resolution, both with and without optimization enabled. This guards against CreateImplFileTraitContext (which uses AccessibleFromEverywhere) leaking internal extensions across assembly boundaries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Negative test: operator on type without it produces FS0001 at compile time - Positive test: adding extension operator makes SRTP solvable, runs correctly Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
a502c13 to
fde29bf
Compare
…ensions # Conflicts: # tests/projects/CompilerCompat/CompilerCompatApp/Program.fs # tests/projects/CompilerCompat/CompilerCompatLib/Library.fs
fde29bf to
781729e
Compare
Avoid allocating HashSet<ExtensionMember> and a list comprehension builder
when there are no candidate extension members to consider. Hot under
extension-method / SRTP-heavy code.
Measurement on an fsharpplus-style stress harness
(9600 SRTP sites, 4x call-repetition factor, 5-run median):
deepest-compiler-frame SelectMethInfosFromExtMembers:
before: 1033 ms
after: 714 ms (-31%)
TC self time:
before: 1.33 s (stdev 0.28)
after: 1.29 s (stdev 0.03)
The stdev collapse reflects reduced GC pressure from the per-call
HashSet allocations. No behaviour change: PropertyCollector.Close()
already returns [] on empty input, and both HashSet/ctor allocations
are pure side-effect-free constructions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| { Some $3, $2.LongIdent, true, $1, None } | ||
|
|
||
| /* Tuple type extension: type ('T1 * 'T2) with ... */ | ||
| | opt_access LPAREN appTypeCanBeNullable STAR appTypeCanBeNullable rparen |
There was a problem hiding this comment.
Could you please add recovery rules and SyntaxTree tests covering various unfinished examples? I'd start with something like this:
type ('T1)type ('T1 *)type ('T1 * 'T2)type ('T1) withtype ('T1 *) with
| /* 3+ tuple type extension: type ('T1 * 'T2 * 'T3) with ... */ | ||
| | opt_access LPAREN appTypeCanBeNullable STAR appTypeCanBeNullable STAR tupleOrQuotTypeElements rparen |
There was a problem hiding this comment.
Can this rule be unified with the previous one?
| /* Struct tuple type extension: type struct ('T1 * 'T2) with ... */ | ||
| | opt_access STRUCT LPAREN appTypeCanBeNullable STAR appTypeCanBeNullable rparen |
There was a problem hiding this comment.
Can STRUCT be extracted into an optional rule, like it's done for opt_access?
| typeDefs |> List.choose (fun typeDef -> | ||
| match typeDef with | ||
| | SynTypeDefn(typeInfo = SynComponentInfo(longId = []; synType = Some(SynType.Tuple(isStruct, path, tupleRange))) as compInfo) -> | ||
| // Transform tuple type extensions: type ('T1 * 'T2) with ... -> type System.Tuple<'T1,'T2> with ... |
There was a problem hiding this comment.
I suspect the transformed nodes may leak to the symbols that are reported to IDEs. Can this be done without rewriting the AST? If that's not possible, we need to make the ranges of the synthetic nodes synthetic.
@gusty Could you please add FCS tests that show that symbols returned by GetAllUsesOfAllSymbols match exactly what was written in the original source?
| accessibility: SynAccess option * | ||
| range: range | ||
| range: range * | ||
| synType: SynType option |
There was a problem hiding this comment.
Maybe we should replace longId with synType instead of making it possible to have both. longId can be represented as SynType. That would also allow to remove the synthetic nodes that are created for tuples in this PR.
…to pr-19602 # Conflicts: # src/Compiler/Checking/NameResolution.fs
❗ Release notes requiredCaution No release notes found for the changed paths (see table below). Please make sure to add an entry with an informative description of the change as well as link to this pull request, issue and language suggestion if applicable. Release notes for this repository are based on Keep A Changelog format. The following format is recommended for this repository:
If you believe that release notes are not necessary for this PR, please add NO_RELEASE_NOTES label to the pull request. You can open this PR in browser to add release notes: open in github.dev
Warning No PR link found in some release notes, please consider adding it.
|
ad7fda3 to
36f9914
Compare
Implements RFC FS-1043. Gated behind
--langversion:preview.See docs/RFC_Changes.md for implementation-specific amendments to the RFC (interop, diagnostics, compatibility, binary compat).
1. Extension members solve SRTP constraints
Extrinsic extension members (operators, named methods, properties) in scope at an inline function's definition site are now captured in the SRTP constraint and considered during overload resolution. Built-in primitive operator solutions retain priority when types match precisely.
2. Weak resolution deferred for inline code
Inline SRTP functions no longer eagerly collapse to a concrete type.
let inline f1 (x: DateTime) y = x + ystays generic (DateTime -> ^a -> ^b when ...). Breaking: signature files and monomorphic bindings may need updating.3.
[<AllowOverloadOnReturnType>]New FSharp.Core attribute enables return-type-based overload resolution for any method, extending behavior previously reserved for
op_Explicit/op_Implicit.NOTE: this is a continuation from #19396 (which is now experimental zone)