Skip to content

Commit 3377652

Browse files
xperiandriclaude
andcommitted
Shrink the mixed range change and fix the range lowering it exposed
Review asked for a smaller diff without code comments. The intent now sits in names (BareRangeExpr, RangeAsSingleValue, mkRangeSplice, rewriteYieldFromRange, rejectRangeOperand, SimpleSemicolonSequenceWithRanges); one rejectRangeOperand replaces the recovery arms for yield and return of a range; the tests are four compilations instead of a fixture per scenario. Fixes found on the way: - a unativeint range with a single element was treated as empty (EmptyRange), which also dropped the element of [ 2un..2un ] and skipped for _ in 2un..2un on main - convertSimpleReturnToExpr declines BindReturn for a returned range and stays free of diagnostics, so FS3925 is reported once by the return arm - the counted loop reads a mutable range operand before the later operands run and no longer counts extra elements for a nativeint range with the smallest step - a then sequence is not split into the elements of a flat literal - query { return 1..3 } reports FS3144 only Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
1 parent d053f72 commit 3377652

8 files changed

Lines changed: 233 additions & 627 deletions

File tree

‎docs/release-notes/.FSharp.Compiler.Service/11.0.200.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
* Fix `NativePtr.stackalloc` used inside the `with` handler, filter or `finally` block of a `try` expression producing an assembly that throws `InvalidProgramException` at method load. Direct or mandatory-inline allocations are now reported at compile time as error FS3924; calls to non-inline allocation helpers, including handler-local helpers, remain legal and retain their method boundary during inlining and let-binding elimination. ([Issue #20295](https://github.com/dotnet/fsharp/issues/20295), [PR #20491](https://github.com/dotnet/fsharp/pull/20491))
1313
* Preserve SRTP code generation when extension constraint solutions are disabled. ([PR #20773](https://github.com/dotnet/fsharp/pull/20773))
1414
* `FsiEvaluationSession.EvalInteraction` and `EvalInteractionNonThrowing` evaluate every `;;`-separated interaction in the text they are given, as their documentation has always said, instead of silently discarding everything after the first. ([PR #20396](https://github.com/dotnet/fsharp/pull/20396))
15+
* Fix the counted-loop lowering of integral ranges: a range with a single `unativeint` element is no longer treated as empty, a mutable range bound is read before the later bounds are evaluated, and a `nativeint` range with the smallest step no longer yields extra elements. ([PR #20759](https://github.com/dotnet/fsharp/pull/20759))
1516
* Prevent `InvalidProgramException` when runtime async awaits a `Task<T>` upcast to `Task`, including through `ConfigureAwait`. ([Issue #20686](https://github.com/dotnet/fsharp/issues/20686), [PR #20726](https://github.com/dotnet/fsharp/pull/20726))
1617
* Struct/reference anonymous-record mismatches now name anonymous records rather than tuples. ([Issue #6110](https://github.com/dotnet/fsharp/issues/6110), [PR #20681](https://github.com/dotnet/fsharp/pull/20681))
1718
* Preserve constraint inference order when explicit generic arguments share a caller type parameter, avoiding spurious FS0001 errors with multiply implemented generic interfaces. ([PR #20491](https://github.com/dotnet/fsharp/pull/20491))

‎src/Compiler/Checking/Expressions/CheckArrayOrListComputedExpressions.fs‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ let TcArrayOrListComputedExpression (cenv: TcFileState) env (overallTy: OverallT
1616
let g = cenv.g
1717

1818
// The syntax '[ n .. m ]' and '[ n .. step .. m ]' is not really part of array or list syntax.
19-
// It could be in the future, e.g. '[ 1; 2..30; 400 ]'
2019
//
2120
// The elaborated form of '[ n .. m ]' is 'List.ofSeq (seq (op_Range n m))' and this shouldn't change
2221
match RewriteRangeExpr comp with
@@ -99,12 +98,8 @@ let TcArrayOrListComputedExpression (cenv: TcFileState) env (overallTy: OverallT
9998
| _ -> SynExpr.ArrayOrList(isArray, elems, m)
10099

101100
TcExprUndelayed cenv overallTy env tpenv replacementExpr
102-
| _ ->
103-
104-
let comp =
105-
match comp with
106-
| SimpleSemicolonSequenceWithRanges cenv valuesAndRanges -> valuesAndRanges
107-
| _ -> comp
101+
| SimpleSemicolonSequenceWithRanges cenv comp
102+
| comp ->
108103

109104
let genCollElemTy = NewInferenceType g
110105

‎src/Compiler/Checking/Expressions/CheckComputationExpressions.fs‎

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2447,27 +2447,13 @@ let rec TryTranslateComputationExpression
24472447

24482448
Some(translatedCtxt returnFromCall)
24492449

2450-
// A range is not yielded or returned as one value (FS-1031). Recover 'yield e1..e2' as 'yield! e1..e2' when the
2451-
// builder can splice; otherwise 'Yield' or 'Return' takes an argument of any type, so no type error follows.
2452-
| SynExpr.YieldOrReturn(((isYield, _) as flags), (BareRangeExpr _ as synRange), mFull, ({ YieldOrReturnKeyword = m } as trivia)) when
2453-
cenv.g.langVersion.SupportsFeature LanguageFeature.AllowMixedRangesAndValuesInSeqExpressions
2454-
->
2455-
errorR (Error(FSComp.SR.tcRangeIsNotASingleValue (), synRange.Range))
2456-
2457-
let recovery =
2458-
if isYield && hasBuilderMethod ceenv m "YieldFrom" then
2459-
mkRangeSplice synRange
2460-
else
2461-
SynExpr.YieldOrReturn(flags, arbExpr ("rangeAsSingleValue", synRange.Range), mFull, trivia)
2462-
2463-
TryTranslateComputationExpression ceenv firstTry q varSpace recovery translatedCtxt
2464-
24652450
| SynExpr.YieldOrReturn((isYield, _), synYieldOrReturnExpr, mFull, { YieldOrReturnKeyword = m }) ->
24662451
let methName = (if isYield then "Yield" else "Return")
24672452

24682453
if ceenv.isQuery && not isYield then
24692454
error (Error(FSComp.SR.tcReturnMayNotBeUsedInQueries (), m))
24702455

2456+
let synYieldOrReturnExpr = rejectRangeOperand cenv synYieldOrReturnExpr
24712457
requireBuilderMethod methName ceenv m m
24722458

24732459
let yieldOrReturnCall =
@@ -2765,6 +2751,8 @@ and TranslateComputationExpressionBind
27652751
/// The inner option indicates if a custom operation is involved inside
27662752
and convertSimpleReturnToExpr (ceenv: ComputationExpressionContext<'a>) comp varSpace innerComp =
27672753
match innerComp with
2754+
| SynExpr.YieldOrReturn((false, _), RangeAsSingleValue ceenv.cenv, _, _) -> None
2755+
27682756
| SynExpr.YieldOrReturn((false, _), returnExpr, m, _) ->
27692757
let returnExpr =
27702758
SynExpr.DebugPoint(DebugPointAtLeafExpr.Yes(false, m), false, returnExpr)

‎src/Compiler/Checking/Expressions/CheckExpressionsOps.fs‎

Lines changed: 42 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -182,30 +182,33 @@ let RewriteRangeExpr synExpr =
182182
Some otherExpr
183183
| _ -> None
184184

185-
/// A closed range 'e1..e2' or 'e1..e2..e3' that is not parenthesized, as a call to '(..)' or '(.. ..)'.
186185
[<return: Struct>]
187186
let (|BareRangeExpr|_|) synExpr =
187+
RewriteRangeExpr synExpr |> ValueOption.ofOption
188+
189+
let (|RangeAsSingleValue|_|) (cenv: TcFileState) synExpr =
188190
match synExpr with
189-
| SynExpr.IndexRange _ ->
190-
match RewriteRangeExpr synExpr with
191-
| Some rangeExpr -> ValueSome rangeExpr
192-
| None -> ValueNone
193-
| _ -> ValueNone
191+
| BareRangeExpr _ -> cenv.g.langVersion.SupportsFeature LanguageFeature.AllowMixedRangesAndValuesInSeqExpressions
192+
| _ -> false
194193

195-
/// A range in element position of a list, array, sequence or computation expression
196-
/// splices its elements, as 'yield! e1..e2' does (FS-1031).
197194
let mkRangeSplice (synRange: SynExpr) =
198195
let m = synRange.Range
199196
SynExpr.YieldOrReturnFrom((true, false), synRange, m, { YieldOrReturnFromKeyword = m })
200197

201-
/// The source of 'yield! e1..e2' is '(..) e1 e2' (FS-1031).
202198
let rewriteYieldFromRange (cenv: TcFileState) synExpr =
203199
match synExpr with
204-
| BareRangeExpr rangeExpr ->
200+
| BareRangeExpr rangeCall ->
205201
checkLanguageFeatureAndRecover cenv.g.langVersion LanguageFeature.AllowMixedRangesAndValuesInSeqExpressions synExpr.Range
206-
rangeExpr
202+
rangeCall
207203
| _ -> synExpr
208204

205+
let rejectRangeOperand (cenv: TcFileState) operand =
206+
match operand with
207+
| RangeAsSingleValue cenv ->
208+
errorR (Error(FSComp.SR.tcRangeIsNotASingleValue (), operand.Range))
209+
arbExpr ("rangeAsSingleValue", operand.Range)
210+
| _ -> operand
211+
209212
/// Check if a computation or sequence expression is syntactically free of 'yield' (though not yield!)
210213
let YieldFree (_cenv: TcFileState) expr =
211214
let rec YieldFree expr =
@@ -273,35 +276,37 @@ let rec TryGetSimpleSemicolonSequenceOfComprehension expr acc cenv acceptDepreca
273276
let (|SimpleSemicolonSequence|_|) cenv acceptDeprecated cexpr =
274277
TryGetSimpleSemicolonSequenceOfComprehension cexpr [] cenv acceptDeprecated
275278

276-
/// A flat list or array of values with at least one range, such as '[ 0; 1..3; 10 ]' (FS-1031),
277-
/// as '[ yield 0; 1..3; yield 10 ]': every value stays an element, as in a list literal.
278279
[<return: Struct>]
279280
let (|SimpleSemicolonSequenceWithRanges|_|) cenv cexpr =
280-
let yieldElement (e: SynExpr) =
281-
SynExpr.YieldOrReturn((true, false), e, e.Range, { YieldOrReturnKeyword = e.Range })
282-
283-
let rec collectReversed expr acc hasRange =
281+
let isRange = RewriteRangeExpr >> Option.isSome
282+
283+
let yieldValue expr =
284+
SynExpr.YieldOrReturn((true, false), expr, expr.Range, SynExprYieldOrReturnTrivia.Zero)
285+
286+
let sequential head tail =
287+
SynExpr.Sequential(
288+
DebugPointAtSequential.SuppressNeither,
289+
true,
290+
head,
291+
tail,
292+
unionRanges head.Range tail.Range,
293+
SynExprSequentialTrivia.Zero
294+
)
295+
296+
let rec elements acc expr =
284297
match expr with
285-
| SynExpr.Sequential(isTrueSeq = true; expr1 = BareRangeExpr _ as e1; expr2 = e2) -> collectReversed e2 (e1 :: acc) true
286-
| SynExpr.Sequential(isTrueSeq = true; expr1 = e1; expr2 = e2) when IsSimpleSemicolonSequenceElement e1 cenv false ->
287-
collectReversed e2 (yieldElement e1 :: acc) hasRange
288-
| BareRangeExpr _ -> ValueSome(expr :: acc)
289-
| _ when hasRange && IsSimpleSemicolonSequenceElement expr cenv false -> ValueSome(yieldElement expr :: acc)
290-
| _ -> ValueNone
291-
292-
match collectReversed cexpr [] false with
293-
| ValueSome(last :: before) ->
294-
let mkSequential (tail: SynExpr) (e: SynExpr) =
295-
SynExpr.Sequential(
296-
DebugPointAtSequential.SuppressNeither,
297-
true,
298-
e,
299-
tail,
300-
unionRanges e.Range tail.Range,
301-
SynExprSequentialTrivia.Zero
302-
)
303-
304-
ValueSome(List.fold mkSequential last before)
298+
| SynExpr.Sequential(isTrueSeq = true; expr1 = head; expr2 = tail) -> elements (head :: acc) tail
299+
| _ -> List.rev (expr :: acc)
300+
301+
match elements [] cexpr with
302+
| elems when
303+
List.exists isRange elems
304+
&& List.forall (fun e -> isRange e || IsSimpleSemicolonSequenceElement e cenv false) elems
305+
->
306+
elems
307+
|> List.map (fun e -> if isRange e then e else yieldValue e)
308+
|> List.reduceBack sequential
309+
|> ValueSome
305310
| _ -> ValueNone
306311

307312
let elimFastIntegerForLoop (spFor, spTo, id, start: SynExpr, dir, finish: SynExpr, innerExpr, m: range) =

‎src/Compiler/Checking/Expressions/CheckSequenceExpressions.fs‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ open FSharp.Compiler.CheckBasics
77
open FSharp.Compiler.CheckExpressions
88
open FSharp.Compiler.CheckExpressionsOps
99
open FSharp.Compiler.ConstraintSolver
10-
open FSharp.Compiler.Features
1110
open FSharp.Compiler.NameResolution
1211
open FSharp.Compiler.PatternMatchCompilation
1312
open FSharp.Compiler.Syntax
@@ -405,12 +404,9 @@ let TcSequenceExpression (cenv: TcFileState) env tpenv comp (overallTy: OverallT
405404

406405
Some(resultExpr, tpenv)
407406

408-
// A range is not yielded or returned as one value (FS-1031); recover as 'yield! e1..e2'.
409-
| SynExpr.YieldOrReturn(flags = (isYield, _); expr = BareRangeExpr _ as synRange; range = m) when
410-
g.langVersion.SupportsFeature LanguageFeature.AllowMixedRangesAndValuesInSeqExpressions
411-
->
407+
| SynExpr.YieldOrReturn(flags = (isYield, _); expr = RangeAsSingleValue cenv as synRange; range = m) ->
412408
if isYield then
413-
errorR (Error(FSComp.SR.tcRangeIsNotASingleValue (), synRange.Range))
409+
rejectRangeOperand cenv synRange |> ignore
414410
else
415411
errorR (Error(FSComp.SR.tcSeqResultsUseYield (), m))
416412

‎src/Compiler/Optimize/LowerComputedCollections.fs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -199,8 +199,6 @@ let LowerComputedListOrArraySeqExpr tcVal g amap m collectorTy overallSeqExpr =
199199
Result.Ok (flag, exprR)
200200
| Result.Error msg -> Result.Error msg
201201

202-
// yield! start..step..finish over an integral type: a counted loop adding each element,
203-
// instead of AddMany over the range's enumerator.
204202
| IntegralRange g (rangeTy, (start, step, finish)) when
205203
not isUninteresting
206204
&& g.langVersion.SupportsFeature LanguageFeature.LowerIntegralRangesToFastLoops

‎src/Compiler/TypedTree/TypedTreeOps.Transforms.fs‎

Lines changed: 9 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1468,7 +1468,7 @@ module internal TupleCompilation =
14681468
start <= 0xffffffffUL
14691469
&& step <= 0xffffffffUL
14701470
&& finish <= 0xffffffffUL
1471-
&& finish <= start
1471+
&& finish < start
14721472
->
14731473
ValueSome EmptyRange
14741474

@@ -1520,12 +1520,6 @@ module internal TupleCompilation =
15201520
ValueSome(Const.UInt64((uint64 finish - uint64 start) / (uint64 Int64.MaxValue + 1UL) + 1UL))
15211521
| Expr.Const(value = Const.Int64 start), Expr.Const(value = Const.Int64 Int64.MinValue), Expr.Const(value = Const.Int64 finish) ->
15221522
ValueSome(Const.UInt64((uint64 start - uint64 finish) / (uint64 Int64.MaxValue + 1UL) + 1UL))
1523-
| Expr.Const(value = Const.IntPtr start), Expr.Const(value = Const.IntPtr Int64.MinValue), Expr.Const(value = Const.IntPtr finish) when
1524-
start <= finish
1525-
->
1526-
ValueSome(Const.UIntPtr((uint64 start - uint64 finish) / (uint64 Int64.MaxValue + 1UL) + 1UL))
1527-
| Expr.Const(value = Const.IntPtr start), Expr.Const(value = Const.IntPtr Int64.MinValue), Expr.Const(value = Const.IntPtr finish) ->
1528-
ValueSome(Const.UIntPtr((uint64 start - uint64 finish) / (uint64 Int64.MaxValue + 1UL) + 1UL))
15291523

15301524
| Expr.Const(value = Const.Int64 start), Expr.Const(value = Const.Int64 step), Expr.Const(value = Const.Int64 finish) when
15311525
start <= finish
@@ -1998,34 +1992,14 @@ module internal TupleCompilation =
19981992
(buildLoop: Count -> ((Idx -> Elem -> Body) -> Loop) -> Expr)
19991993
=
20001994
let inline mkLetBindingsIfNeeded f =
2001-
match start, step, finish with
2002-
| (Expr.Const _ | Expr.Val _), (Expr.Const _ | Expr.Val _), (Expr.Const _ | Expr.Val _) -> f start step finish
2003-
2004-
| (Expr.Const _ | Expr.Val _), (Expr.Const _ | Expr.Val _), _ ->
2005-
mkCompGenLetIn finish.Range (nameof finish) rangeTy finish (fun (_, finish) -> f start step finish)
2006-
2007-
| _, (Expr.Const _ | Expr.Val _), (Expr.Const _ | Expr.Val _) ->
2008-
mkCompGenLetIn start.Range (nameof start) rangeTy start (fun (_, start) -> f start step finish)
2009-
2010-
| (Expr.Const _ | Expr.Val _), _, (Expr.Const _ | Expr.Val _) ->
2011-
mkCompGenLetIn step.Range (nameof step) rangeTy step (fun (_, step) -> f start step finish)
2012-
2013-
| _, (Expr.Const _ | Expr.Val _), _ ->
2014-
mkCompGenLetIn start.Range (nameof start) rangeTy start (fun (_, start) ->
2015-
mkCompGenLetIn finish.Range (nameof finish) rangeTy finish (fun (_, finish) -> f start step finish))
2016-
2017-
| (Expr.Const _ | Expr.Val _), _, _ ->
2018-
mkCompGenLetIn step.Range (nameof step) rangeTy step (fun (_, step) ->
2019-
mkCompGenLetIn finish.Range (nameof finish) rangeTy finish (fun (_, finish) -> f start step finish))
2020-
2021-
| _, _, (Expr.Const _ | Expr.Val _) ->
2022-
mkCompGenLetIn start.Range (nameof start) rangeTy start (fun (_, start) ->
2023-
mkCompGenLetIn step.Range (nameof step) rangeTy step (fun (_, step) -> f start step finish))
2024-
2025-
| _, _, _ ->
2026-
mkCompGenLetIn start.Range (nameof start) rangeTy start (fun (_, start) ->
2027-
mkCompGenLetIn step.Range (nameof step) rangeTy step (fun (_, step) ->
2028-
mkCompGenLetIn finish.Range (nameof finish) rangeTy finish (fun (_, finish) -> f start step finish)))
1995+
let bindIfNeeded name operand k =
1996+
match operand with
1997+
| Expr.Const _ -> k operand
1998+
| Expr.Val(valRef, _, _) when not valRef.IsMutable -> k operand
1999+
| _ -> mkCompGenLetIn operand.Range name rangeTy operand (fun (_, bound) -> k bound)
2000+
2001+
bindIfNeeded (nameof start) start (fun start ->
2002+
bindIfNeeded (nameof step) step (fun step -> bindIfNeeded (nameof finish) finish (fun finish -> f start step finish)))
20292003

20302004
mkLetBindingsIfNeeded (fun start step finish ->
20312005
/// Start at 0 and count up through count - 1.

0 commit comments

Comments
 (0)