Consume the elements a query translation produces

docs/release-notes/.FSharp.Core/11.0.200.md

@@ -6,5 +6,7 @@
 
 * Correct `FSharpValue.PreComputeTuplePropertyInfo` to report that struct tuples store elements in fields, not properties, and recommend supported tuple readers instead of reporting a valid index as out of range. ([Issue #20801](https://github.com/dotnet/fsharp/issues/20801), [PR #20815](https://github.com/dotnet/fsharp/pull/20815))
 * Quotation debug text from `Expr.ToString()`, `Expr.ToString(full)`, and `%A` now prints struct tuple construction as `NewStructTuple`; reference tuples remain `NewTuple`. ([Issue #20800](https://github.com/dotnet/fsharp/issues/20800), [PR #20814](https://github.com/dotnet/fsharp/pull/20814))
+* Fix struct tuples in query expressions: selecting one no longer fails with "unreachable", and the items of one that a later operator reads are taken from its fields instead of failing with `ArgumentException: Struct tuples store their elements in fields, not properties`. ([Issue #15522](https://github.com/dotnet/fsharp/issues/15522), [PR #20894](https://github.com/dotnet/fsharp/pull/20894))
+* Fix LINQ providers failing on, or evaluating on the client, queries that end by consuming the tuples or records a query produces: `exists`, `all`, `sumBy`, `averageBy`, `minBy`, `maxBy` and `count` after two `for`s, a `join` or a tuple or record `select` now work on the query's own elements instead of on tuples and records rebuilt at the end of the query. ([PR #20901](https://github.com/dotnet/fsharp/pull/20901))
 
 ### Changed

src/FSharp.Core/Linq.fs

@@ -475,7 +475,14 @@ module LeafExpressionConverter =
 
         | TupleGet(arg, n) ->
              let argP = ConvExprToLinqInContext env arg
-             let rec build ty argP n =
+             let rec build (ty: Type) argP n =
+                 // A struct tuple holds its elements in fields, which PreComputeTuplePropertyInfo cannot return
+                 if ty.IsValueType then
+                     if n < 7 then
+                         Expression.Field(argP, ty.GetField("Item" + string (n + 1))) |> asExpr
+                     else
+                         build (ty.GetGenericArguments().[7]) (Expression.Field(argP, ty.GetField "Rest") |> asExpr) (n - 7)
+                 else
                  match FSharpValue.PreComputeTuplePropertyInfo(ty, n) with
                  | propInfo, None ->
                      Expression.Property(argP, propInfo) |> asExpr

src/FSharp.Core/Query.fs

@@ -1156,8 +1156,9 @@ module Query =
     /// immutable-to-mutable conversion information, convert it back to an expression involving immutable tuples or records.
     let rec ConvMutableToImmutable conv mutExpr =
         match conv with
-        | TupleConv convs ->
-            Expr.NewTuple (convs |> List.mapi (fun i conv -> ConvMutableToImmutable conv (AnonymousObjectGet (mutExpr, i))))
+        | TupleConv (isStruct, convs) ->
+            let elements = convs |> List.mapi (fun i conv -> ConvMutableToImmutable conv (AnonymousObjectGet (mutExpr, i)))
+            if isStruct then Expr.NewStructTuple elements else Expr.NewTuple elements
         | RecordConv (typ, convs) ->
             Expr.NewRecord (typ, convs |> List.mapi (fun i conv -> ConvMutableToImmutable conv (AnonymousObjectGet (mutExpr, i))))
 
@@ -1835,29 +1836,102 @@ module Query =
 
         result
 
+    /// Whether the translation turns the elements of this type into anonymous objects, and builds them again from those:
+    /// tuples and records, of a class or of a struct, groups of such elements, and sequences of any of these. Only the
+    /// type arguments of the sequences and groups are followed, so that a type which holds a sequence of itself ends
+    /// the search at once.
+    let rec IsConvertedElement (ty: Type) =
+        Microsoft.FSharp.Reflection.FSharpType.IsTuple ty
+        || isPartiallyImmutableRecord ty
+        || ((IsIEnumerableTy ty || IsIQueryableTy ty) && IsConvertedElement (ty.GetGenericArguments().[0]))
+        || (ty.IsGenericType
+            && ty.GetGenericTypeDefinition() = typedefof<IGrouping<_, _>>
+            && IsConvertedElement (ty.GetGenericArguments().[1]))
+
+    /// A tuple or record of a class, which the translation builds anew from the elements it produces each time one
+    /// is asked for, and a sequence of converted elements, which it selects anew over those elements
+    let rec IsRebuiltObject (ty: Type) =
+        if ty.IsValueType then
+            // A struct holds an object that is rebuilt when one of its parts is one
+            (Microsoft.FSharp.Reflection.FSharpType.IsTuple ty
+             && Microsoft.FSharp.Reflection.FSharpType.GetTupleElements ty |> Array.exists IsRebuiltObject)
+            || (isPartiallyImmutableRecord ty
+                && Microsoft.FSharp.Reflection.FSharpType.GetRecordFields ty
+                   |> Array.exists (fun field -> IsRebuiltObject field.PropertyType))
+        else
+            Microsoft.FSharp.Reflection.FSharpType.IsTuple ty
+            || isPartiallyImmutableRecord ty
+            || ((IsIEnumerableTy ty || IsIQueryableTy ty) && IsConvertedElement (ty.GetGenericArguments().[0]))
+
+    /// Whether the property is a part of the record or tuple it is read from, and not a member that computes something
+    /// from it: such a member gets the object itself
+    let IsPartOf (owner: Type) (property: System.Reflection.PropertyInfo) =
+        if Microsoft.FSharp.Reflection.FSharpType.IsRecord owner then
+            Microsoft.FSharp.Reflection.FSharpType.GetRecordFields owner
+            |> Array.exists (fun field -> field.Name = property.Name)
+        else
+            Microsoft.FSharp.Reflection.FSharpType.IsTuple owner
+            && property.Name.StartsWith("Item", StringComparison.Ordinal)
+
+    /// Whether the expression uses the element, or a tuple or record in it, as a value of its own, and not only for
+    /// what it holds. Where the function is rewritten over the elements the translation produces, each such use
+    /// is an object of its own, and not the one object the element is.
+    let rec UsesWholeElement (element: Var) (expr: Expr) =
+        let rec readsPartOfElement (expr: Expr) =
+            match expr with
+            | Var used -> used = element
+            | TupleGet (inner, _) -> readsPartOfElement inner
+            | PropertyGet (Some inner, property, []) when IsPartOf inner.Type property -> readsPartOfElement inner
+            | _ -> false
+
+        if readsPartOfElement expr then
+            IsRebuiltObject expr.Type
+        else
+            match expr with
+            | ExprShape.ShapeVar _ -> false
+            | ExprShape.ShapeLambda (_, body) -> UsesWholeElement element body
+            | ExprShape.ShapeCombination (_, args) -> List.exists (UsesWholeElement element) args
+
+    /// Evaluate the inner core of a query whose elements a function consumes. The function is rewritten over the
+    /// elements as the query translation produces them, so they are not converted back to immutable tuples and
+    /// records for it. A function that uses an element as a value of its own gets the elements as they are.
+    let EvalNonNestedInnerWithConsumer (queryProducingSequence: Expr) (immutConsumingVar: Var, immutConsumingExpr: Expr) =
+        if UsesWholeElement immutConsumingVar immutConsumingExpr then
+            EvalNonNestedInner CanEliminate.Yes queryProducingSequence, immutConsumingVar, MacroExpand immutConsumingExpr
+        else
+            let mutSource, _, mutConsumingVar, mutConsumingExpr =
+                TransInnerApplicativeAndCommit true queryProducingSequence (immutConsumingVar, immutConsumingExpr)
+            EvaluateQuotation (EliminateNestedQueries mutSource), mutConsumingVar, mutConsumingExpr
+
+    /// Evaluate the inner core of a query whose elements are only counted, with the type of the elements as the
+    /// query translation produces them.
+    let EvalNonNestedInnerForCount (queryProducingSequence: Expr) (immutElemTy: Type) =
+        let mutSource, conv = TransInnerAndCommit CanEliminate.Yes true queryProducingSequence
+        EvaluateQuotation (EliminateNestedQueries mutSource), ConvImmutableTypeToMutableType conv immutElemTy
+
     /// Evaluate the outer calls of a query until the inner core that actually produces a sequence of results is reached.
     let rec EvalNonNestedOuter canElim (tm: Expr) =
         match tm with
 
-        | CallMinBy (_, [srcItemTy; qTy; keyItemTy], source, Lambda(v, valSelector)) ->
-            let sourcev = EvalNonNestedInner CanEliminate.Yes source
-            CallMinBy (qTyIsIQueryable qTy, srcItemTy, keyItemTy, sourcev, keyItemTy, v, MacroExpand valSelector)
+        | CallMinBy (_, [_; qTy; keyItemTy], source, Lambda(v, valSelector)) ->
+            let sourcev, v, valSelector = EvalNonNestedInnerWithConsumer source (v, valSelector)
+            CallMinBy (qTyIsIQueryable qTy, v.Type, keyItemTy, sourcev, keyItemTy, v, valSelector)
 
-        | CallMaxBy (_, [srcItemTy; qTy; keyItemTy], source, Lambda(v, valSelector)) ->
-            let sourcev = EvalNonNestedInner CanEliminate.Yes source
-            CallMaxBy (qTyIsIQueryable qTy, srcItemTy, keyItemTy, sourcev, keyItemTy, v, MacroExpand valSelector)
+        | CallMaxBy (_, [_; qTy; keyItemTy], source, Lambda(v, valSelector)) ->
+            let sourcev, v, valSelector = EvalNonNestedInnerWithConsumer source (v, valSelector)
+            CallMaxBy (qTyIsIQueryable qTy, v.Type, keyItemTy, sourcev, keyItemTy, v, valSelector)
 
-        | CallMinByNullable (_, [srcItemTy; qTy; keyItemTy ], source, Lambda(v, valSelector)) ->
-            let sourcev = EvalNonNestedInner CanEliminate.Yes source
-            CallMinByNullable  (qTyIsIQueryable qTy, srcItemTy, keyItemTy, sourcev, MakeNullableTy keyItemTy, v, MacroExpand valSelector)
+        | CallMinByNullable (_, [_; qTy; keyItemTy ], source, Lambda(v, valSelector)) ->
+            let sourcev, v, valSelector = EvalNonNestedInnerWithConsumer source (v, valSelector)
+            CallMinByNullable  (qTyIsIQueryable qTy, v.Type, keyItemTy, sourcev, MakeNullableTy keyItemTy, v, valSelector)
 
-        | CallMaxByNullable (_, [srcItemTy; qTy; keyItemTy ], source, Lambda(v, valSelector)) ->
-            let sourcev = EvalNonNestedInner CanEliminate.Yes source
-            CallMaxByNullable  (qTyIsIQueryable qTy, srcItemTy, keyItemTy, sourcev, MakeNullableTy keyItemTy, v, MacroExpand valSelector)
+        | CallMaxByNullable (_, [_; qTy; keyItemTy ], source, Lambda(v, valSelector)) ->
+            let sourcev, v, valSelector = EvalNonNestedInnerWithConsumer source (v, valSelector)
+            CallMaxByNullable  (qTyIsIQueryable qTy, v.Type, keyItemTy, sourcev, MakeNullableTy keyItemTy, v, valSelector)
 
         | CallCount (_, [srcItemTy; qTy], source) ->
-            let sourcev = EvalNonNestedInner CanEliminate.Yes source
-            CallCount          (qTyIsIQueryable qTy, srcItemTy, sourcev)
+            let sourcev, mutSrcItemTy = EvalNonNestedInnerForCount source srcItemTy
+            CallCount          (qTyIsIQueryable qTy, mutSrcItemTy, sourcev)
 
         | CallHead (_, [srcItemTy; qTy], source) ->
             let sourcev = EvalNonNestedInner CanEliminate.Yes source
@@ -1883,29 +1957,29 @@ module Query =
             let sourcev = EvalNonNestedInner CanEliminate.Yes source
             CallSingleOrDefault(qTyIsIQueryable qTy, srcItemTy, sourcev)
 
-        | CallAverageBy (qb, [srcItemTy; qTy; resTy], source, Lambda(v, valSelector)) ->
-            let sourcev = EvalNonNestedInner CanEliminate.Yes source
-            CallAverageBy        (qb, qTyIsIQueryable qTy, srcItemTy, resTy, sourcev, resTy, v, MacroExpand valSelector)
+        | CallAverageBy (qb, [_; qTy; resTy], source, Lambda(v, valSelector)) ->
+            let sourcev, v, valSelector = EvalNonNestedInnerWithConsumer source (v, valSelector)
+            CallAverageBy        (qb, qTyIsIQueryable qTy, v.Type, resTy, sourcev, resTy, v, valSelector)
 
-        | CallAverageByNullable (qb, [srcItemTy; qTy; resTyNoNullable], source, Lambda(v, valSelector)) ->
-            let sourcev = EvalNonNestedInner CanEliminate.Yes source
-            CallAverageByNullable(qb, qTyIsIQueryable qTy, srcItemTy, resTyNoNullable, sourcev, MakeNullableTy resTyNoNullable, v, MacroExpand valSelector)
+        | CallAverageByNullable (qb, [_; qTy; resTyNoNullable], source, Lambda(v, valSelector)) ->
+            let sourcev, v, valSelector = EvalNonNestedInnerWithConsumer source (v, valSelector)
+            CallAverageByNullable(qb, qTyIsIQueryable qTy, v.Type, resTyNoNullable, sourcev, MakeNullableTy resTyNoNullable, v, valSelector)
 
-        | CallSumBy (qb, [srcItemTy; qTy; resTy], source, Lambda(v, valSelector)) ->
-            let sourcev = EvalNonNestedInner CanEliminate.Yes source
-            CallSumBy (qb, qTyIsIQueryable qTy, srcItemTy, resTy, sourcev, resTy, v, MacroExpand valSelector)
+        | CallSumBy (qb, [_; qTy; resTy], source, Lambda(v, valSelector)) ->
+            let sourcev, v, valSelector = EvalNonNestedInnerWithConsumer source (v, valSelector)
+            CallSumBy (qb, qTyIsIQueryable qTy, v.Type, resTy, sourcev, resTy, v, valSelector)
 
-        | CallSumByNullable (qb, [srcItemTy; qTy; resTyNoNullable], source, Lambda(v, valSelector)) ->
-            let sourcev = EvalNonNestedInner CanEliminate.Yes source
-            CallSumByNullable (qb, qTyIsIQueryable qTy, srcItemTy, resTyNoNullable, sourcev, MakeNullableTy resTyNoNullable, v, MacroExpand valSelector)
+        | CallSumByNullable (qb, [_; qTy; resTyNoNullable], source, Lambda(v, valSelector)) ->
+            let sourcev, v, valSelector = EvalNonNestedInnerWithConsumer source (v, valSelector)
+            CallSumByNullable (qb, qTyIsIQueryable qTy, v.Type, resTyNoNullable, sourcev, MakeNullableTy resTyNoNullable, v, valSelector)
 
-        | CallExists (_, [srcItemTy; qTy], source, Lambda(v, predicate)) ->
-            let sourcev = EvalNonNestedInner CanEliminate.Yes source
-            CallAny (qTyIsIQueryable qTy, srcItemTy, sourcev, v, MacroExpand predicate)
+        | CallExists (_, [_; qTy], source, Lambda(v, predicate)) ->
+            let sourcev, v, predicate = EvalNonNestedInnerWithConsumer source (v, predicate)
+            CallAny (qTyIsIQueryable qTy, v.Type, sourcev, v, predicate)
 
-        | CallForAll (_, [srcItemTy; qTy], source, Lambda(v, predicate)) ->
-            let sourcev = EvalNonNestedInner CanEliminate.Yes source
-            CallAll (qTyIsIQueryable qTy, srcItemTy, sourcev, v, MacroExpand predicate)
+        | CallForAll (_, [_; qTy], source, Lambda(v, predicate)) ->
+            let sourcev, v, predicate = EvalNonNestedInnerWithConsumer source (v, predicate)
+            CallAll (qTyIsIQueryable qTy, v.Type, sourcev, v, predicate)
 
         | CallFind (_, [srcItemTy; qTy], source, Lambda(v, f)) ->
             let sourcev = EvalNonNestedInner CanEliminate.Yes source

src/FSharp.Core/QueryExtensions.fs

@@ -108,14 +108,6 @@ module internal Adapters =
 
     let anonObjectTypes = tupleTypes |> Array.map snd
 
-    let tupleToAnonTypeMap =
-        let t = Dictionary<Type, Type>()
-
-        for k, v in tupleTypes do
-            t.[k] <- v
-
-        t
-
     let anonToTupleTypeMap =
         let t = Dictionary<Type, Type>()
 
@@ -173,18 +165,13 @@ module internal Adapters =
 
         walk i e e.Type
 
+    /// Reference and struct tuples of the same arity share one anonymous object type.
     let RewriteTupleType (ty: Type) conv =
-        // Tuples are generic, so lookup only for generic types
-        assert ty.IsGenericType
-        let generic = ty.GetGenericTypeDefinition()
-
-        match tupleToAnonTypeMap.TryGetValue generic with
-        | true, mutableTupleType ->
-            // Recursively transform type arguments
-            mutableTupleType.MakeGenericType(ty.GetGenericArguments() |> Array.toList |> conv |> Array.ofList)
-        | _ ->
-            assert false
-            failwith "unreachable"
+        assert (FSharpType.IsTuple ty)
+        let elementTypes = ty.GetGenericArguments()
+        let mutableTupleType = anonObjectTypes.[elementTypes.Length - 1]
+        // Recursively transform type arguments
+        mutableTupleType.MakeGenericType(elementTypes |> Array.toList |> conv |> Array.ofList)
 
     let (|RecordFieldGetSimplification|_|) (expr: Expr) =
         match expr with
@@ -206,7 +193,7 @@ module internal Adapters =
     /// that we can invert the process in final query results.
     [<NoComparison; NoEquality>]
     type ConversionDescription =
-        | TupleConv of ConversionDescription list
+        | TupleConv of isStruct: bool * ConversionDescription list
         | RecordConv of Type * ConversionDescription list
         | GroupingConv (* origKeyType: *) of Type (* origElemType: *) * Type * ConversionDescription
         | SeqConv of ConversionDescription
@@ -216,18 +203,18 @@ module internal Adapters =
     /// "yield" or "select", convert it to a type involving anonymous objects according to the conversion data.
     let rec ConvImmutableTypeToMutableType conv ty =
         match conv with
-        | TupleConv convs ->
+        | TupleConv(isStruct, convs) ->
             assert (FSharpType.IsTuple ty)
 
             match convs with
             | x1 :: x2 :: x3 :: x4 :: x5 :: x6 :: x7 :: x8 :: tail ->
-                let els = [ x1; x2; x3; x4; x5; x6; x7; TupleConv(x8 :: tail) ]
+                let els = [ x1; x2; x3; x4; x5; x6; x7; TupleConv(isStruct, x8 :: tail) ]
                 RewriteTupleType ty (List.map2 ConvImmutableTypeToMutableType els)
             | _ -> RewriteTupleType ty (List.map2 ConvImmutableTypeToMutableType convs)
         | RecordConv(_, convs) ->
             assert (isPartiallyImmutableRecord ty)
             let types = [| for f in FSharpType.GetRecordFields ty -> f.PropertyType |]
-            ConvImmutableTypeToMutableType (TupleConv convs) (FSharpType.MakeTupleType types)
+            ConvImmutableTypeToMutableType (TupleConv(false, convs)) (FSharpType.MakeTupleType types)
         | GroupingConv(_keyTy, _elemTy, conv) ->
             assert ty.IsGenericType
             assert (ty.GetGenericTypeDefinition() = typedefof<IGrouping<_, _>>)
@@ -324,7 +311,7 @@ module internal Adapters =
             let argExprsNow, argScripts =
                 exprs |> List.map (ProduceMoreMutables tipf) |> List.unzip
 
-            NewAnonymousObject argExprsNow, TupleConv argScripts
+            NewAnonymousObject argExprsNow, TupleConv(expr.Type.IsValueType, argScripts)
 
         // Replace immutable records by anonymous objects
         | Patterns.NewRecord(typ, args) when isPartiallyImmutableRecord typ ->

tests/FSharp.Core.UnitTests/FSharp.Core.UnitTests.fsproj

@@ -92,6 +92,15 @@
     <Compile Include="FSharp.Core\Microsoft.FSharp.Quotations\FSharpQuotations.fs" />
     <Compile Include="FSharp.Core\Microsoft.FSharp.Linq\NullableOperators.fs" />
     <Compile Include="FSharp.Core\Microsoft.FSharp.Linq\QueryTests.fs" />
+    <Compile Include="FSharp.Core\Microsoft.FSharp.Linq\QueryProviderShape.fs" />
+    <Compile Include="FSharp.Core\Microsoft.FSharp.Linq\QueryScenarioDomain.fs" />
+    <Compile Include="FSharp.Core\Microsoft.FSharp.Linq\QueryScenario.fs" />
+    <Compile Include="FSharp.Core\Microsoft.FSharp.Linq\QueryProjectionScenarios.fs" />
+    <Compile Include="FSharp.Core\Microsoft.FSharp.Linq\QueryOperatorScenarios.fs" />
+    <Compile Include="FSharp.Core\Microsoft.FSharp.Linq\QueryKeyScenarios.fs" />
+    <Compile Include="FSharp.Core\Microsoft.FSharp.Linq\QueryStructureScenarios.fs" />
+    <Compile Include="FSharp.Core\Microsoft.FSharp.Linq\QueryMethodSyntaxScenarios.fs" />
+    <Compile Include="FSharp.Core\Microsoft.FSharp.Linq\QueryShapeTests.fs" />
     <Compile Include="FSharp.Core\XmlDocumentationValidation.fs" />
     <Compile Include="Interop\CSharpCollectionExpressions.fs" />
     <Compile Include="StructTuples.fs" />