Merge remote-tracking branch 'dotnet/features/ioperation' into inullc…

…oalescingexpression-refactor

* dotnet/features/ioperation:
  Update CSharpReplaceMethodWithPropertyService.cs
  Add VB side of fix.
  Remove unneeded function.
  Improve trivia preservation when converting methods into a property.
  VB side of do-not-simplify-nameof if it changes semantics.
  Do not simplify to an alias in a nameof if it changes the value of hte nameof.
  Include System.Runtime.Serialization.Primitives and System.Security.Cryptography.Csp in PortableFacades CoreXT package. (dotnet#21438)
  Address one more refactoring feedback
  Address PR feedback
  Fix possible race conditions in TestExtensionErrorHandler
  Fix expected test results to properly consider trivia
  Improve messages when tests fail due to expected text
  Default to considering trivia during testing
  Remove RemoveUnneededReferences from LamdaRewriter (dotnet#21367)
  Enable embedding sources to Windows PDBs (dotnet#21391)
  re-enabled assert we have disabled
  Do not insert Microsoft.DiaSymReader.Native (dotnet#21420)
  Resolving merge conflict
  Don't pick a project arbitrarily when navigating to symbols

build/Targets/Packages.props

@@ -34,10 +34,10 @@
     <MicrosoftCSharpVersion>4.3.0</MicrosoftCSharpVersion>
     <MicrosoftDiagnosticsRuntimeVersion>0.8.31-beta</MicrosoftDiagnosticsRuntimeVersion>
     <MicrosoftDiagnosticsTracingTraceEventVersion>1.0.35</MicrosoftDiagnosticsTracingTraceEventVersion>
-    <MicrosoftDiaSymReaderVersion>1.1.0</MicrosoftDiaSymReaderVersion>
-    <MicrosoftDiaSymReaderConverterXmlVersion>1.0.0-beta1-61708-01</MicrosoftDiaSymReaderConverterXmlVersion>
-    <MicrosoftDiaSymReaderNativeVersion>1.6.0-beta2-25304</MicrosoftDiaSymReaderNativeVersion>
-    <MicrosoftDiaSymReaderPortablePdbVersion>1.3.0</MicrosoftDiaSymReaderPortablePdbVersion>
+    <MicrosoftDiaSymReaderVersion>1.2.0-beta1-62008-01</MicrosoftDiaSymReaderVersion>
+    <MicrosoftDiaSymReaderConverterXmlVersion>1.1.0-beta1-62008-01</MicrosoftDiaSymReaderConverterXmlVersion>
+    <MicrosoftDiaSymReaderNativeVersion>1.7.0-private-25604</MicrosoftDiaSymReaderNativeVersion>
+    <MicrosoftDiaSymReaderPortablePdbVersion>1.3.0-beta1-62008-01</MicrosoftDiaSymReaderPortablePdbVersion>
     <MicrosoftDotNetIBCMerge>4.7.2-alpha-00001</MicrosoftDotNetIBCMerge>
     <MicrosoftDotNetVersionToolsVersion>1.0.27-prerelease-01811-02</MicrosoftDotNetVersionToolsVersion>
     <MicrosoftIdentityModelClientsActiveDirectoryVersion>3.13.8</MicrosoftIdentityModelClientsActiveDirectoryVersion>
@@ -194,6 +194,7 @@
     <SystemRuntimeSerializationPrimitivesVersion>4.3.0</SystemRuntimeSerializationPrimitivesVersion>
     <SystemSecurityAccessControlVersion>4.3.0</SystemSecurityAccessControlVersion>
     <SystemSecurityCryptographyAlgorithmsVersion>4.3.0</SystemSecurityCryptographyAlgorithmsVersion>
+    <SystemSecurityCryptographyCspVersion>4.3.0</SystemSecurityCryptographyCspVersion>
     <SystemSecurityCryptographyEncodingVersion>4.3.0</SystemSecurityCryptographyEncodingVersion>
     <SystemSecurityCryptographyPrimitivesVersion>4.3.0</SystemSecurityCryptographyPrimitivesVersion>
     <SystemSecurityCryptographyX509CertificatesVersion>4.3.0</SystemSecurityCryptographyX509CertificatesVersion>

src/Compilers/CSharp/Portable/CSharpResources.resx

@@ -4986,7 +4986,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
     <value>/sourcelink switch is only supported when emitting PDB.</value>
   </data>
   <data name="ERR_CannotEmbedWithoutPdb" xml:space="preserve">
-    <value>/embed switch is only supported when emitting Portable PDB (/debug:portable or /debug:embedded).</value>
+    <value>/embed switch is only supported when emitting a PDB.</value>
   </data>
   <data name="ERR_InvalidInstrumentationKind" xml:space="preserve">
     <value>Invalid instrumentation kind: {0}</value>

src/Compilers/CSharp/Portable/CommandLine/CSharpCommandLineParser.cs

@@ -1243,14 +1243,9 @@ public new CSharpCommandLineArguments Parse(IEnumerable<string> args, string bas
                 embeddedFiles.AddRange(sourceFiles);
             }
 
-            if (embeddedFiles.Count > 0)
+            if (embeddedFiles.Count > 0 && !emitPdb)
             {
-                // Restricted to portable PDBs for now, but the IsPortable condition should be removed
-                // and the error message adjusted accordingly when native PDB support is added.
-                if (!emitPdb || !debugInformationFormat.IsPortable())
-                {
-                    AddDiagnostic(diagnostics, ErrorCode.ERR_CannotEmbedWithoutPdb);
-                }
+                AddDiagnostic(diagnostics, ErrorCode.ERR_CannotEmbedWithoutPdb);
             }
 
             var parsedFeatures = ParseFeatures(features);

src/Compilers/CSharp/Portable/Compiler/TypeCompilationState.cs

@@ -66,7 +66,7 @@ internal MethodWithBody(MethodSymbol method, BoundStatement body, ImportChain im
 
         public readonly CSharpCompilation Compilation;
 
-        public ClosureEnvironment StaticLambdaFrame;
+        public SynthesizedClosureEnvironment StaticLambdaFrame;
 
         /// <summary>
         /// A graph of method->method references for this(...) constructor initializers.

src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaCapturedVariable.cs

@@ -31,7 +31,7 @@ private LambdaCapturedVariable(SynthesizedContainer frame, TypeSymbol type, stri
             _isThis = isThisParameter;
         }
 
-        public static LambdaCapturedVariable Create(ClosureEnvironment frame, Symbol captured, ref int uniqueId)
+        public static LambdaCapturedVariable Create(SynthesizedClosureEnvironment frame, Symbol captured, ref int uniqueId)
         {
             Debug.Assert(captured is LocalSymbol || captured is ParameterSymbol);
 
@@ -87,7 +87,7 @@ private static TypeSymbol GetCapturedVariableFieldType(SynthesizedContainer fram
             if ((object)local != null)
             {
                 // if we're capturing a generic frame pointer, construct it with the new frame's type parameters
-                var lambdaFrame = local.Type.OriginalDefinition as ClosureEnvironment;
+                var lambdaFrame = local.Type.OriginalDefinition as SynthesizedClosureEnvironment;
                 if ((object)lambdaFrame != null)
                 {
                     // lambdaFrame may have less generic type parameters than frame, so trim them down (the first N will always match)

src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.Tree.cs

@@ -119,6 +119,23 @@ public sealed class Closure
 
                 public ClosureEnvironment ContainingEnvironmentOpt;
 
+                private bool _capturesThis;
+
+                /// <summary>
+                /// True if this closure directly or transitively captures 'this' (captures
+                /// a local function which directly or indirectly captures 'this').
+                /// Calculated in <see cref="MakeAndAssignEnvironments"/>.
+                /// </summary>
+                public bool CapturesThis
+                {
+                    get => _capturesThis;
+                    set
+                    {
+                        Debug.Assert(value);
+                        _capturesThis = value;
+                    }
+                }
+
                 public Closure(MethodSymbol symbol)
                 {
                     Debug.Assert(symbol != null);
@@ -132,81 +149,29 @@ public void Free()
                 }
             }
 
-            /// <summary>
-            /// Optimizes local functions such that if a local function only references other local functions
-            /// that capture no variables, we don't need to create capture environments for any of them.
-            /// </summary>
-            private void RemoveUnneededReferences(ParameterSymbol thisParam)
+            public sealed class ClosureEnvironment
             {
-                var methodGraph = new MultiDictionary<MethodSymbol, MethodSymbol>();
-                var capturesThis = new HashSet<MethodSymbol>();
-                var capturesVariable = new HashSet<MethodSymbol>();
-                var visitStack = new Stack<MethodSymbol>();
-                VisitClosures(ScopeTree, (scope, closure) =>
-                {
-                    foreach (var capture in closure.CapturedVariables)
-                    {
-                        if (capture is MethodSymbol localFunc)
-                        {
-                            methodGraph.Add(localFunc, closure.OriginalMethodSymbol);
-                        }
-                        else if (capture == thisParam)
-                        {
-                            if (capturesThis.Add(closure.OriginalMethodSymbol))
-                            {
-                                visitStack.Push(closure.OriginalMethodSymbol);
-                            }
-                        }
-                        else if (capturesVariable.Add(closure.OriginalMethodSymbol) &&
-                                 !capturesThis.Contains(closure.OriginalMethodSymbol))
-                        {
-                            visitStack.Push(closure.OriginalMethodSymbol);
-                        }
-                    }
-                });
-
-                while (visitStack.Count > 0)
-                {
-                    var current = visitStack.Pop();
-                    var setToAddTo = capturesVariable.Contains(current) ? capturesVariable : capturesThis;
-                    foreach (var capturesCurrent in methodGraph[current])
-                    {
-                        if (setToAddTo.Add(capturesCurrent))
-                        {
-                            visitStack.Push(capturesCurrent);
-                        }
-                    }
-                }
+                public readonly SetWithInsertionOrder<Symbol> CapturedVariables;
+
+                /// <summary>
+                /// Represents a <see cref="SynthesizedEnvironment"/> that had its environment
+                /// pointer (a local pointing to the environment) captured like a captured
+                /// variable. Assigned in
+                /// <see cref="ComputeLambdaScopesAndFrameCaptures(ParameterSymbol)"/>
+                /// </summary>
+                public bool CapturesParent;
 
-                // True if there are any closures in the tree which
-                // capture 'this' and another variable
-                bool captureMoreThanThis = false;
+                public readonly bool IsStruct;
+                internal SynthesizedClosureEnvironment SynthesizedEnvironment;
 
-                VisitClosures(ScopeTree, (scope, closure) =>
+                public ClosureEnvironment(IEnumerable<Symbol> capturedVariables, bool isStruct)
                 {
-                    if (!capturesVariable.Contains(closure.OriginalMethodSymbol))
-                    {
-                        closure.CapturedVariables.Clear();
-                    }
-
-                    if (capturesThis.Contains(closure.OriginalMethodSymbol))
+                    CapturedVariables = new SetWithInsertionOrder<Symbol>();
+                    foreach (var item in capturedVariables)
                     {
-                        closure.CapturedVariables.Add(thisParam);
-                        if (closure.CapturedVariables.Count > 1)
-                        {
-                            captureMoreThanThis |= true;
-                        }
+                        CapturedVariables.Add(item);
                     }
-                });
-
-                if (!captureMoreThanThis && capturesThis.Count > 0)
-                {
-                    // If we have closures which capture 'this', and nothing else, we can
-                    // remove 'this' from the declared variables list, since we don't need
-                    // to create an environment to hold 'this' (since we can emit the
-                    // lowered methods directly onto the containing class)
-                    bool removed = ScopeTree.DeclaredVariables.Remove(thisParam);
-                    Debug.Assert(removed);
+                    IsStruct = isStruct;
                 }
             }
 
@@ -226,6 +191,31 @@ public static void VisitClosures(Scope scope, Action<Scope, Closure> action)
                 }
             }
 
+            /// <summary>
+            /// Visit all the closures and return true when the <paramref name="func"/> returns
+            /// true. Otherwise, returns false.
+            /// </summary>
+            public static bool CheckClosures(Scope scope, Func<Scope, Closure, bool> func)
+            {
+                foreach (var closure in scope.Closures)
+                {
+                    if (func(scope, closure))
+                    {
+                        return true;
+                    }
+                }
+
+                foreach (var nested in scope.NestedScopes)
+                {
+                    if (CheckClosures(nested, func))
+                    {
+                        return true;
+                    }
+                }
+
+                return false;
+            }
+
             /// <summary>
             /// Visit the tree with the given root and run the <paramref name="action"/>
             /// </summary>
@@ -491,6 +481,13 @@ private void AddIfCaptured(Symbol symbol, SyntaxNode syntax)
                         return;
                     }
 
+                    if (symbol is MethodSymbol method &&
+                        _currentClosure.OriginalMethodSymbol == method)
+                    {
+                        // Is this recursion? If so there's no capturing
+                        return;
+                    }
+
                     if (symbol.ContainingSymbol != _currentClosure.OriginalMethodSymbol)
                     {
                         // Restricted types can't be hoisted, so they are not permitted to be captured