From d0e4f34b1a0b19779ee989050092fe92c6e1bd26 Mon Sep 17 00:00:00 2001 From: Brian Rourke Boll Date: Wed, 10 Apr 2024 12:05:03 -0400 Subject: [PATCH 1/4] Add tests for indexed setters with tuples --- .../FSharp.Compiler.ComponentTests.fsproj | 1 + .../Language/IndexedSetTests.fs | 182 ++++++++++++++++++ 2 files changed, 183 insertions(+) create mode 100644 tests/FSharp.Compiler.ComponentTests/Language/IndexedSetTests.fs diff --git a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj index 4f04946a466..ddafa873e23 100644 --- a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj +++ b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj @@ -233,6 +233,7 @@ + diff --git a/tests/FSharp.Compiler.ComponentTests/Language/IndexedSetTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/IndexedSetTests.fs new file mode 100644 index 00000000000..573685aabd9 --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/Language/IndexedSetTests.fs @@ -0,0 +1,182 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module Language.IndexedSetTests + +open FSharp.Test.Compiler +open Xunit + +module Array = + [] + let ``Dotless indexed set of parenthesized tuple compiles`` () = + FSharp " + let xs = Array.zeroCreate 1 + xs[0] <- (1, 2, 3) + " + |> typecheck + |> shouldSucceed + + [] + let ``Dotless indexed set of unparenthesized tuple compiles`` () = + FSharp " + let xs = Array.zeroCreate 1 + xs[0] <- 1, 2, 3 + " + |> typecheck + |> shouldSucceed + + [] + let ``Dotless indexed set of double-parenthesized tuple compiles`` () = + FSharp " + let xs = Array.zeroCreate 1 + xs[0] <- ((1, 2, 3)) + " + |> typecheck + |> shouldSucceed + + [] + let ``Dot-indexed set of parenthesized tuple compiles`` () = + FSharp " + let xs = Array.zeroCreate 1 + xs.[0] <- (1, 2, 3) + " + |> typecheck + |> shouldSucceed + + [] + let ``Dot-indexed set of unparenthesized tuple compiles`` () = + FSharp " + let xs = Array.zeroCreate 1 + xs.[0] <- 1, 2, 3 + " + |> typecheck + |> shouldSucceed + + [] + let ``Dot-indexed set of double-parenthesized tuple compiles`` () = + FSharp " + let xs = Array.zeroCreate 1 + xs.[0] <- ((1, 2, 3)) + " + |> typecheck + |> shouldSucceed + +module Dictionary = + // Parsed as SynExpr.Set. + [] + let ``Dotless indexed set of parenthesized tuple compiles`` () = + FSharp " + let xs = System.Collections.Generic.Dictionary () + xs[0] <- (1, 2, 3) + " + |> typecheck + |> shouldSucceed + + // Parsed as SynExpr.Set. + [] + let ``Dotless indexed set of unparenthesized tuple compiles`` () = + FSharp " + let xs = System.Collections.Generic.Dictionary () + xs[0] <- 1, 2, 3 + " + |> typecheck + |> shouldSucceed + + // Parsed as SynExpr.Set. + [] + let ``Dotless indexed set of double-parenthesized tuple compiles`` () = + FSharp " + let xs = System.Collections.Generic.Dictionary () + xs[0] <- ((1, 2, 3)) + " + |> typecheck + |> shouldSucceed + + // Parsed as SynExpr.DotIndexedSet. + [] + let ``Dot-indexed set of parenthesized tuple compiles`` () = + FSharp " + let xs = System.Collections.Generic.Dictionary () + xs.[0] <- (1, 2, 3) + " + |> typecheck + |> shouldSucceed + + // Parsed as SynExpr.DotIndexedSet. + [] + let ``Dot-indexed set of unparenthesized tuple compiles`` () = + FSharp " + let xs = System.Collections.Generic.Dictionary () + xs.[0] <- 1, 2, 3 + " + |> typecheck + |> shouldSucceed + + // Parsed as SynExpr.DotIndexedSet. + [] + let ``Dot-indexed set of double-parenthesized tuple compiles`` () = + FSharp " + let xs = System.Collections.Generic.Dictionary () + xs.[0] <- ((1, 2, 3)) + " + |> typecheck + |> shouldSucceed + + // Parsed as SynExpr.NamedIndexedPropertySet. + [] + let ``Named indexed property set of parenthesized tuple compiles`` () = + FSharp " + let xs = System.Collections.Generic.Dictionary () + xs.Item 0 <- (1, 2, 3) + " + |> typecheck + |> shouldSucceed + + // Parsed as SynExpr.NamedIndexedPropertySet. + [] + let ``Named indexed property set of unparenthesized tuple compiles`` () = + FSharp " + let xs = System.Collections.Generic.Dictionary () + xs.Item 0 <- 1, 2, 3 + " + |> typecheck + |> shouldSucceed + + // Parsed as SynExpr.NamedIndexedPropertySet. + [] + let ``Named indexed property set of double-parenthesized tuple compiles`` () = + FSharp " + let xs = System.Collections.Generic.Dictionary () + xs.Item 0 <- ((1, 2, 3)) + " + |> typecheck + |> shouldSucceed + + // Parsed as SynExpr.DotNamedIndexedPropertySet. + [] + let ``Dot-named indexed property set of parenthesized tuple compiles`` () = + FSharp " + let xs = System.Collections.Generic.Dictionary () + (xs).Item 0 <- (1, 2, 3) + " + |> typecheck + |> shouldSucceed + + // Parsed as SynExpr.DotNamedIndexedPropertySet. + [] + let ``Dot-named indexed property set of unparenthesized tuple compiles`` () = + FSharp " + let xs = System.Collections.Generic.Dictionary () + (xs).Item 0 <- 1, 2, 3 + " + |> typecheck + |> shouldSucceed + + // Parsed as SynExpr.DotNamedIndexedPropertySet. + [] + let ``Dot-named indexed property set of double-parenthesized tuple compiles`` () = + FSharp " + let xs = System.Collections.Generic.Dictionary () + (xs).Item 0 <- ((1, 2, 3)) + " + |> typecheck + |> shouldSucceed From 2ba76f37ec71e292003f8f2f32cabe544a8c2f24 Mon Sep 17 00:00:00 2001 From: Brian Rourke Boll Date: Wed, 10 Apr 2024 12:05:24 -0400 Subject: [PATCH 2/4] Wrap value expr in extra parens in 3 more cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * In the scenarios where assigning an unparenthesized tuple using an indexed setter already works, `MakeDelayedSet` is actually being called twice — once before `TcIndexingThen` is called, and once inside of it. For `SynExpr.NamedIndexedPropertySet`, `SynExpr.DotIndexedSet`, and, `SynExpr.DotNamedIndexedPropertySet`, we don't actually want to delay it; it just needs an extra set of parens (whether it should really need double parens is a different question). In this commit, I am simply adding the extra parens inline. I could alternatively have updated `MakeDelayedSet` itself to always wrap in double parens, although that would mean that in the codepaths where that function is itself already being called twice, we'd be adding extra parens parens unnecessarily. While extra parens do not affect typechecking (unlike insufficient parens), it seems undesirable to allocate more nodes than we really need, only to strip them later anyway. --- src/Compiler/Checking/CheckExpressions.fs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/Compiler/Checking/CheckExpressions.fs b/src/Compiler/Checking/CheckExpressions.fs index 4f7b56ebf11..3466b162c3a 100644 --- a/src/Compiler/Checking/CheckExpressions.fs +++ b/src/Compiler/Checking/CheckExpressions.fs @@ -5412,7 +5412,10 @@ and TcExprThen (cenv: cenv) overallTy env tpenv isArg synExpr delayed = TcNonControlFlowExpr env <| fun env -> if g.langVersion.SupportsFeature LanguageFeature.IndexerNotationWithoutDot then warning(Error(FSComp.SR.tcIndexNotationDeprecated(), mDot)) - TcIndexerThen cenv env overallTy mWholeExpr mDot tpenv (Some (expr3, mOfLeftOfSet)) expr1 indexArgs delayed + // Wrap in extra parens: like MakeDelayedSet, + // but we don't actually want to delay it here. + let setInfo = SynExpr.Paren (expr3, range0, None, expr3.Range), mOfLeftOfSet + TcIndexerThen cenv env overallTy mWholeExpr mDot tpenv (Some setInfo) expr1 indexArgs delayed // Part of 'T.Ident | SynExpr.Typar (typar, m) -> @@ -5827,7 +5830,10 @@ and TcExprUndelayed (cenv: cenv) (overallTy: OverallTy) env tpenv (synExpr: SynE // synExpr1.longId(synExpr2) <- expr3, very rarely used named property setters | SynExpr.DotNamedIndexedPropertySet (synExpr1, synLongId, synExpr2, expr3, mStmt) -> TcNonControlFlowExpr env <| fun env -> - TcExprDotNamedIndexedPropertySet cenv overallTy env tpenv (synExpr1, synLongId, synExpr2, expr3, mStmt) + // Wrap in extra parens: like MakeDelayedSet, + // but we don't actually want to delay it here. + let setInfo = SynExpr.Paren (expr3, range0, None, expr3.Range) + TcExprDotNamedIndexedPropertySet cenv overallTy env tpenv (synExpr1, synLongId, synExpr2, setInfo, mStmt) | SynExpr.LongIdentSet (synLongId, synExpr2, m) -> TcNonControlFlowExpr env <| fun env -> @@ -5836,7 +5842,10 @@ and TcExprUndelayed (cenv: cenv) (overallTy: OverallTy) env tpenv (synExpr: SynE // Type.Items(synExpr1) <- synExpr2 | SynExpr.NamedIndexedPropertySet (synLongId, synExpr1, synExpr2, mStmt) -> TcNonControlFlowExpr env <| fun env -> - TcExprNamedIndexPropertySet cenv overallTy env tpenv (synLongId, synExpr1, synExpr2, mStmt) + // Wrap in extra parens: like MakeDelayedSet, + // but we don't actually want to delay it here. + let setInfo = SynExpr.Paren (synExpr2, range0, None, synExpr2.Range) + TcExprNamedIndexPropertySet cenv overallTy env tpenv (synLongId, synExpr1, setInfo, mStmt) | SynExpr.TraitCall (TypesForTypar tps, synMemberSig, arg, m) -> TcNonControlFlowExpr env <| fun env -> From 278666379da3685e3dc6ffe6681886de39d378bf Mon Sep 17 00:00:00 2001 From: Brian Rourke Boll Date: Wed, 10 Apr 2024 12:24:01 -0400 Subject: [PATCH 3/4] Update release notes --- docs/release-notes/.FSharp.Compiler.Service/8.0.300.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md b/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md index 57bc1578db3..6dc81132ceb 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md +++ b/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md @@ -31,6 +31,7 @@ * Obsolete attribute is ignored in constructor property assignment ([PR #16900](https://github.com/dotnet/fsharp/pull/16900)) * Completion: fix completion in empty dot lambda prefix ([#16829](https://github.com/dotnet/fsharp/pull/16829)) * Fix StackOverflow when checking non-recursive bindings in module or namespace in `fscAnyCpu`/`fsiAnyCpu`. ([PR #16908](https://github.com/dotnet/fsharp/pull/16908)) +* Make typechecking of indexed setters with tuples on the right more consistent. ([Issue #16987](https://github.com/dotnet/fsharp/issues/16987), [PR #17017](https://github.com/dotnet/fsharp/pull/17017)) ### Added From 81cf689649d8a9771eb6cefe0fbe4d829e3d70c7 Mon Sep 17 00:00:00 2001 From: Brian Rourke Boll Date: Thu, 11 Apr 2024 18:14:57 -0400 Subject: [PATCH 4/4] Move release notes --- docs/release-notes/.FSharp.Compiler.Service/8.0.300.md | 1 - docs/release-notes/.FSharp.Compiler.Service/8.0.400.md | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) create mode 100644 docs/release-notes/.FSharp.Compiler.Service/8.0.400.md diff --git a/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md b/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md index 6dc81132ceb..57bc1578db3 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md +++ b/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md @@ -31,7 +31,6 @@ * Obsolete attribute is ignored in constructor property assignment ([PR #16900](https://github.com/dotnet/fsharp/pull/16900)) * Completion: fix completion in empty dot lambda prefix ([#16829](https://github.com/dotnet/fsharp/pull/16829)) * Fix StackOverflow when checking non-recursive bindings in module or namespace in `fscAnyCpu`/`fsiAnyCpu`. ([PR #16908](https://github.com/dotnet/fsharp/pull/16908)) -* Make typechecking of indexed setters with tuples on the right more consistent. ([Issue #16987](https://github.com/dotnet/fsharp/issues/16987), [PR #17017](https://github.com/dotnet/fsharp/pull/17017)) ### Added diff --git a/docs/release-notes/.FSharp.Compiler.Service/8.0.400.md b/docs/release-notes/.FSharp.Compiler.Service/8.0.400.md new file mode 100644 index 00000000000..fd3c5d501b0 --- /dev/null +++ b/docs/release-notes/.FSharp.Compiler.Service/8.0.400.md @@ -0,0 +1,3 @@ +### Fixed + +* Make typechecking of indexed setters with tuples on the right more consistent. ([Issue #16987](https://github.com/dotnet/fsharp/issues/16987), [PR #17017](https://github.com/dotnet/fsharp/pull/17017))