Merge pull request #16653 from dotnet/merges/main-to-release/dev17.10

Merge main to release/dev17.10

Author: vzarytovskii

DEVGUIDE.md

@@ -275,9 +275,25 @@ Where `<version>` corresponds to the latest Visual Studio version on your machin
 
 Use the `Debug` configuration to test your changes locally. It is the default. Do not use the `Release` configuration! Local development and testing of Visual Studio tooling is not designed for the `Release` configuration.
 
-### Writing and running benchmarks
+### Benchmarking
 
-Existing compiler benchmarks can be found in `tests\benchmarks\`.
+Existing compiler benchmarks can be found in `tests\benchmarks\`. The folder contains READMEs describing specific benchmark projects as well as guidelines for creating new benchmarks. There is also `FSharp.Benchmarks.sln` solution containing all the benchmark project and their dependencies.
+
+To exercise the benchmarking infrastructure locally, run:
+
+(Windows)
+```cmd
+build.cmd -configuration Release -testBenchmarks
+```
+
+(Linux/Mac)
+```shell
+./build.sh --configuration Release --testBenchmarks
+```
+
+This is executed in CI as well. It does the following:
+- builds all the benchmarking projects
+- does smoke testing for fast benchmarks (executes them once to check they don't fail in the runtime)
 
 ### Benchmarking and profiling the compiler
 
@@ -286,151 +302,6 @@ Existing compiler benchmarks can be found in `tests\benchmarks\`.
 * Always build both versions of compiler/FCS from source and not use pre-built binaries from SDK (SDK binaries are crossgen'd, which can affect performance).
 * To run `Release` build of compiler/FCS.
 
-### Example benchmark setup using [BenchmarkDotNet](https://github.com/dotnet/BenchmarkDotNet)
-
-1. Perform a clean build of the compiler and FCS from source (as described in this document, build can be done with `-noVisualStudio` in case if FCS/FSharp.Core is being benchmarked/profiled).
-
-2. Create a benchmark project (in this example, the project will be created in `tests\benchmarks\FCSBenchmarks`).
-
-      ```shell
-      cd tests\benchmarks\FCSBenchmarks
-      dotnet new console -o FcsBench --name FcsBench -lang F#
-      ```
-
-3. Add needed packages and project references.
-
-    ```shell
-    cd FcsBench
-    dotnet add package BenchmarkDotNet
-    dotnet add reference ..\..\..\src\Compiler\FSharp.Compiler.Service.fsproj
-    ```
-
-4. Additionally, if you want to test changes to the FSharp.Core (note that the relative path can be different)
-
-     ```shell
-     dotnet add reference ..\..\..\src\FSharp.Core\FSharp.Core.fsproj
-     ```
-
-    > as well as the following property have to be added to `FcsBench.fsproj`:
-
-    ```xml
-    <PropertyGroup>
-        <DisableImplicitFSharpCoreReference>true</DisableImplicitFSharpCoreReference>
-    </PropertyGroup>
-    ```
-
-5. Add a new benchmark for FCS/FSharp.Core by editing `Program.fs`.
-
-      ```fsharp
-      open System.IO
-      open FSharp.Compiler.CodeAnalysis
-      open FSharp.Compiler.Diagnostics
-      open FSharp.Compiler.Text
-      open BenchmarkDotNet.Attributes
-      open BenchmarkDotNet.Running
-
-      [<MemoryDiagnoser>]
-      type CompilerService() =
-          let mutable checkerOpt = None
-          let mutable sourceOpt = None
-
-          let parsingOptions =
-              {
-                  SourceFiles = [|"CheckExpressions.fs"|]
-                  ConditionalDefines = []
-                  DiagnosticOptions = FSharpDiagnosticOptions.Default
-                  LangVersionText = "default"
-                  IsInteractive = false
-                  LightSyntax = None
-                  CompilingFsLib = false
-                  IsExe = false
-              }
-
-          [<GlobalSetup>]
-          member _.Setup() =
-              match checkerOpt with
-              | None ->
-                  checkerOpt <- Some(FSharpChecker.Create(projectCacheSize = 200))
-              | _ -> ()
-
-              match sourceOpt with
-              | None ->
-                  sourceOpt <- Some <| SourceText.ofString(File.ReadAllText("""C:\Users\vlza\code\fsharp\src\Compiler\Checking\CheckExpressions.fs"""))
-              | _ -> ()
-
-
-          [<Benchmark>]
-          member _.ParsingTypeCheckerFs() =
-              match checkerOpt, sourceOpt with
-              | None, _ -> failwith "no checker"
-              | _, None -> failwith "no source"
-              | Some(checker), Some(source) ->
-                  let results = checker.ParseFile("CheckExpressions.fs",  source, parsingOptions) |> Async.RunSynchronously
-                  if results.ParseHadErrors then failwithf "parse had errors: %A" results.Diagnostics
-
-          [<IterationCleanup(Target = "ParsingTypeCheckerFs")>]
-          member _.ParsingTypeCheckerFsSetup() =
-              match checkerOpt with
-              | None -> failwith "no checker"
-              | Some(checker) ->
-                  checker.InvalidateAll()
-                  checker.ClearLanguageServiceRootCachesAndCollectAndFinalizeAllTransients()
-                  checker.ParseFile("dummy.fs", SourceText.ofString "dummy", parsingOptions) |> Async.RunSynchronously |> ignore
-
-      [<EntryPoint>]
-      let main _ =
-          BenchmarkRunner.Run<CompilerService>() |> ignore
-          0
-      ```
-
-      > For more detailed information about available BenchmarkDotNet options, please refer to [BenchmarkDotNet Documentation](https://benchmarkdotnet.org/articles/overview.html).
-
-6. Build and run the benchmark.
-
-      ```shell
-      dotnet build -c Release
-      dotnet run -c Release
-      ```
-
-7. You can find results in `.\BenchmarkDotNet.Artifacts\results\` in the current benchmark project directory.
-
-    ```shell
-    > ls .\BenchmarkDotNet.Artifacts\results\
-
-        Directory: C:\Users\vlza\code\fsharp\tests\benchmarks\FCSBenchmarks\FcsBench\BenchmarkDotNet.Artifacts\results
-
-    Mode                 LastWriteTime         Length Name
-    ----                 -------------         ------ ----
-    -a---           4/25/2022  1:42 PM            638 Program.CompilerService-report-github.md
-    -a---           4/25/2022  1:42 PM           1050 Program.CompilerService-report.csv
-    -a---           4/25/2022  1:42 PM           1169 Program.CompilerService-report.html
-    ```
-
-    > *-report-github.md can be used to post benchmark results to GitHub issue/PR/discussion or RFC.
-    >
-    >*-report.csv can be used for comparison purposes.
-
-    **Example output:**
-
-    ``` ini
-
-    BenchmarkDotNet=v0.13.1, OS=Windows 10.0.25102
-    Intel Core i7-8750H CPU 2.20GHz (Coffee Lake), 1 CPU, 12 logical and 6 physical cores
-    .NET SDK=6.0.200
-      [Host]     : .NET 6.0.3 (6.0.322.12309), X64 RyuJIT DEBUG
-      Job-GDIBXX : .NET 6.0.3 (6.0.322.12309), X64 RyuJIT
-
-    InvocationCount=1  UnrollFactor=1
-
-    ```
-
-    |               Method |     Mean |   Error |  StdDev |   Median |     Gen 0 |     Gen 1 | Allocated |
-    |--------------------- |---------:|--------:|--------:|---------:|----------:|----------:|----------:|
-    | ParsingTypeCheckerFs | 199.4 ms | 3.84 ms | 9.78 ms | 195.5 ms | 4000.0000 | 1000.0000 |     28 MB |
-
-8. Repeat for any number of changes you would like to test.
-9. **Optionally:** benchmark code and results can be included as part of the PR for future reference.
-
 ## Additional resources
 
 The primary technical guide to the core compiler code is [The F# Compiler Technical Guide](https://github.com/dotnet/fsharp/blob/main/docs/index.md). Please read and contribute to that guide.

docs/release-notes/.FSharp.Compiler.Service/8.0.300.md

@@ -2,7 +2,7 @@
 
 * Code generated files with > 64K methods and generated symbols crash when loaded. Use infered sequence points for debugging. ([Issue #16399](https://github.com/dotnet/fsharp/issues/16399), [#PR 16514](https://github.com/dotnet/fsharp/pull/16514))
 * `nameof Module` expressions and patterns are processed to link files in `--test:GraphBasedChecking`. ([PR #16550](https://github.com/dotnet/fsharp/pull/16550))
-* Graph Based Checking doesn't throw on invalid parsed input so it can be used for IDE scenarios ([PR #16575](https://github.com/dotnet/fsharp/pull/16575), [PR #16588](https://github.com/dotnet/fsharp/pull/16588))
+* Graph Based Checking doesn't throw on invalid parsed input so it can be used for IDE scenarios ([PR #16575](https://github.com/dotnet/fsharp/pull/16575), [PR #16588](https://github.com/dotnet/fsharp/pull/16588), [PR #16643](https://github.com/dotnet/fsharp/pull/16643))
 * Keep parens for problematic exprs (`if`, `match`, etc.) in `$"{(…):N0}"`, `$"{(…),-3}"`, etc. ([PR #16578](https://github.com/dotnet/fsharp/pull/16578))
 * Fix crash in DOTNET_SYSTEM_GLOBALIZATION_INVARIANT mode [#PR 16471](https://github.com/dotnet/fsharp/pull/16471))
 
@@ -13,6 +13,7 @@
 * Parser recovers on complex primary constructor patterns, better tree representation for primary constructor patterns. ([PR #16425](https://github.com/dotnet/fsharp/pull/16425))
 * Name resolution: keep type vars in subsequent checks ([PR #16456](https://github.com/dotnet/fsharp/pull/16456))
 * Higher-order-function-based API for working with the untyped abstract syntax tree. ([PR #16462](https://github.com/dotnet/fsharp/pull/16462))
+* Allow returning bool instead of unit option for partial active patterns. ([Language suggestion #1041](https://github.com/fsharp/fslang-suggestions/issues/1041), [PR #16473](https://github.com/dotnet/fsharp/pull/16473))
 
 ### Changed
 

docs/release-notes/.Language/preview.md

@@ -3,6 +3,7 @@
 * Better generic unmanaged structs handling. ([Language suggestion #692](https://github.com/fsharp/fslang-suggestions/issues/692), [PR #12154](https://github.com/dotnet/fsharp/pull/12154))
 * Bidirectional F#/C# interop for 'unmanaged' constraint. ([PR #12154](https://github.com/dotnet/fsharp/pull/12154))
 * Make `.Is*` discriminated union properties visible. ([Language suggestion #222](https://github.com/fsharp/fslang-suggestions/issues/222), [PR #16341](https://github.com/dotnet/fsharp/pull/16341))
+* Allow returning bool instead of unit option for partial active patterns. ([Language suggestion #1041](https://github.com/fsharp/fslang-suggestions/issues/1041), [PR #16473](https://github.com/dotnet/fsharp/pull/16473))
 
 ### Fixed
 

eng/Version.Details.xml

@@ -1,11 +1,13 @@
 <?xml version="1.0" encoding="utf-8"?>
 <Dependencies>
   <ProductDependencies>
+    <!-- Intermediate is necessary for source build. -->
     <Dependency Name="Microsoft.SourceBuild.Intermediate.source-build-reference-packages" Version="9.0.0-alpha.1.24073.1">
       <Uri>https://github.com/dotnet/source-build-reference-packages</Uri>
       <Sha>412264fd6c04712d1d31ff05d37c6919101ef4f4</Sha>
       <SourceBuild RepoName="source-build-reference-packages" ManagedOnly="true" />
     </Dependency>
+    <!-- Intermediate is necessary for source build. -->
     <Dependency Name="Microsoft.SourceBuild.Intermediate.msbuild" Version="17.7.0-preview-23217-02">
       <Uri>https://github.com/dotnet/msbuild</Uri>
       <Sha>2cbc8b6aef648cf21c6a68a0dab7fe09a614e475</Sha>
@@ -37,6 +39,11 @@
     <Dependency Name="Microsoft.DotNet.XliffTasks" Version="1.0.0-beta.23475.1" CoherentParentDependency="Microsoft.DotNet.Arcade.Sdk">
       <Uri>https://github.com/dotnet/xliff-tasks</Uri>
       <Sha>73f0850939d96131c28cf6ea6ee5aacb4da0083a</Sha>
+    </Dependency>
+    <!-- Intermediate is necessary for source build. -->
+    <Dependency Name="Microsoft.SourceBuild.Intermediate.xliff-tasks" Version="1.0.0-beta.23475.1" CoherentParentDependency="Microsoft.DotNet.Arcade.Sdk">
+      <Uri>https://github.com/dotnet/xliff-tasks</Uri>
+      <Sha>73f0850939d96131c28cf6ea6ee5aacb4da0083a</Sha>
       <SourceBuild RepoName="xliff-tasks" ManagedOnly="true" />
     </Dependency>
     <Dependency Name="optimization.windows_nt-x64.MIBC.Runtime" Version="1.0.0-prerelease.23614.4">

eng/build-utils.ps1

@@ -179,7 +179,7 @@ function Get-PackageDir([string]$name, [string]$version = "") {
 }
 
 function Run-MSBuild([string]$projectFilePath, [string]$buildArgs = "", [string]$logFileName = "", [switch]$parallel = $true, [switch]$summary = $true, [switch]$warnAsError = $true, [string]$configuration = $script:configuration, [string]$verbosity = $script:verbosity) {
-    # Because we override the C#/VB toolset to build against our LKG package, it is important
+    # Because we override the C#/VB toolset to build against our LKG (Last Known Good) package, it is important
     # that we do not reuse MSBuild nodes from other jobs/builds on the machine. Otherwise,
     # we'll run into issues such as https://github.com/dotnet/roslyn/issues/6211.
     # MSBuildAdditionalCommandLineArgs=

eng/build.sh

@@ -25,6 +25,7 @@ usage()
   echo "Test actions:"
   echo "  --testcoreclr                  Run unit tests on .NET Core (short: --test, -t)"
   echo "  --testCompilerComponentTests   Run FSharp.Compiler.ComponentTests on .NET Core"
+  echo "  --testBenchmarks               Build and Run Benchmark suite"
   echo ""
   echo "Advanced settings:"
   echo "  --ci                           Building in CI"
@@ -56,6 +57,7 @@ pack=false
 publish=false
 test_core_clr=false
 test_compilercomponent_tests=false
+test_benchmarks=false
 configuration="Debug"
 verbosity='minimal'
 binary_log=false
@@ -126,6 +128,9 @@ while [[ $# > 0 ]]; do
     --testcompilercomponenttests)
       test_compilercomponent_tests=true
       ;;
+      --testbenchmarks)
+      test_benchmarks=true
+      ;;
     --ci)
       ci=true
       ;;
@@ -330,4 +335,10 @@ if [[ "$test_compilercomponent_tests" == true ]]; then
   TestUsingNUnit --testproject "$repo_root/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj" --targetframework $coreclrtestframework  --notestfilter 
 fi
 
+if [[ "$test_benchmarks" == true ]]; then
+  pushd "$repo_root/tests/benchmarks"
+  ./SmokeTestBenchmarks.sh
+  popd
+fi
+
 ExitWithExitCode 0

src/Compiler/Checking/CheckExpressions.fs

@@ -5093,7 +5093,11 @@ and TcPatLongIdentActivePatternCase warnOnUpper (cenv: cenv) (env: TcEnv) vFlags
 
             if dtys.Length = args.Length + 1 &&
                 ((isOptionTy g retTy && isUnitTy g (destOptionTy g retTy)) ||
-                (isValueOptionTy g retTy && isUnitTy g (destValueOptionTy g retTy))) then
+                (isValueOptionTy g retTy && isUnitTy g (destValueOptionTy g retTy))) ||
+                // `bool` partial AP always be treated as `unit option`
+                // For `val (|P|_|) : _ -> bool`, only allow `match x with | P -> ...`
+                // For `val (|P|_|) : _ -> _ -> bool`, only allow `match x with | P parameter -> ...`
+                (not apinfo.IsTotal && isBoolTy g retTy) then
                 args, SynPat.Const(SynConst.Unit, m)
             else
                 List.frontAndBack args
@@ -10752,14 +10756,25 @@ and TcNormalizedBinding declKind (cenv: cenv) env tpenv overallTy safeThisValOpt
         | Some (apinfo, apOverallTy, _) ->
             let activePatResTys = NewInferenceTypes g apinfo.ActiveTags
             let _, apReturnTy = stripFunTy g apOverallTy
-
-            if isStructRetTy && apinfo.IsTotal then
-                errorR(Error(FSComp.SR.tcInvalidStructReturn(), mBinding))
-
-            if isStructRetTy then
+            let apRetTy =
+                if apinfo.IsTotal then 
+                    if isStructRetTy then errorR(Error(FSComp.SR.tcInvalidStructReturn(), mBinding))
+                    ActivePatternReturnKind.RefTypeWrapper
+                else
+                    if isStructRetTy || isValueOptionTy cenv.g apReturnTy then ActivePatternReturnKind.StructTypeWrapper
+                    elif isBoolTy cenv.g apReturnTy then ActivePatternReturnKind.Boolean
+                    else ActivePatternReturnKind.RefTypeWrapper
+            
+            match apRetTy with
+            | ActivePatternReturnKind.Boolean ->
+                checkLanguageFeatureError g.langVersion LanguageFeature.BooleanReturningAndReturnTypeDirectedPartialActivePattern mBinding
+            | ActivePatternReturnKind.StructTypeWrapper when not isStructRetTy ->
+                checkLanguageFeatureError g.langVersion LanguageFeature.BooleanReturningAndReturnTypeDirectedPartialActivePattern mBinding
+            | ActivePatternReturnKind.StructTypeWrapper ->
                 checkLanguageFeatureError g.langVersion LanguageFeature.StructActivePattern mBinding
+            | ActivePatternReturnKind.RefTypeWrapper -> ()
 
-            UnifyTypes cenv env mBinding (apinfo.ResultType g rhsExpr.Range activePatResTys isStructRetTy) apReturnTy
+            UnifyTypes cenv env mBinding (apinfo.ResultType g rhsExpr.Range activePatResTys apRetTy) apReturnTy
 
         | None ->
             if isStructRetTy then

src/Compiler/Checking/NameResolution.fs

@@ -87,18 +87,20 @@ let ActivePatternElemsOfValRef g (vref: ValRef) =
     match TryGetActivePatternInfo vref with
     | Some apinfo ->
 
-        let isStructRetTy = 
+        let retKind = 
             if apinfo.IsTotal then
-                false
+                ActivePatternReturnKind.RefTypeWrapper
             else
                 let _, apReturnTy = stripFunTy g vref.TauType
                 let hasStructAttribute() = 
                     vref.Attribs
                     |> List.exists (function 
                         | Attrib(targetsOpt = Some(System.AttributeTargets.ReturnValue)) as a -> IsMatchingFSharpAttribute g g.attrib_StructAttribute a  
                         | _ -> false)
-                isStructTy g apReturnTy || hasStructAttribute()
-        apinfo.ActiveTags |> List.mapi (fun i _ -> APElemRef(apinfo, vref, i, isStructRetTy))
+                if isValueOptionTy g apReturnTy || hasStructAttribute() then ActivePatternReturnKind.StructTypeWrapper
+                elif isBoolTy g apReturnTy then ActivePatternReturnKind.Boolean
+                else ActivePatternReturnKind.RefTypeWrapper
+        apinfo.ActiveTags |> List.mapi (fun i _ -> APElemRef(apinfo, vref, i, retKind))
     | None -> []
 
 /// Try to make a reference to a value in a module.