diff --git a/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/CSharpInstructionDecoder.cs b/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/CSharpInstructionDecoder.cs index 7353e57348e5f35412fee37de1dd029419d74e07..df37da58bbca013739c9b72d008faecce90a3b7e 100644 --- a/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/CSharpInstructionDecoder.cs +++ b/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/CSharpInstructionDecoder.cs @@ -3,6 +3,8 @@ using System; using System.Collections.Immutable; using System.Diagnostics; +using System.Reflection.Metadata; +using System.Reflection.Metadata.Ecma335; using System.Text; using Microsoft.CodeAnalysis.CSharp.Symbols; using Microsoft.CodeAnalysis.CSharp.Symbols.Metadata.PE; @@ -152,7 +154,8 @@ internal override CSharpCompilation GetCompilation(DkmClrModuleInstance moduleIn internal override MethodSymbol GetMethod(CSharpCompilation compilation, DkmClrInstructionAddress instructionAddress) { - return compilation.GetSourceMethod(instructionAddress.ModuleInstance.Mvid, instructionAddress.MethodId.Token); + var methodHandle = (MethodDefinitionHandle)MetadataTokens.Handle(instructionAddress.MethodId.Token); + return compilation.GetSourceMethod(instructionAddress.ModuleInstance.Mvid, methodHandle); } internal override TypeNameDecoder GetTypeNameDecoder(CSharpCompilation compilation, MethodSymbol method) diff --git a/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/CompilationContext.cs b/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/CompilationContext.cs index 5715fdab21ce3534e45fc93ffb065ba4dce5ca00..465ec0cff05456ab80b2fa9a19a9d5025a7c50eb 100644 --- a/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/CompilationContext.cs +++ b/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/CompilationContext.cs @@ -37,7 +37,7 @@ internal sealed class CompilationContext private readonly MethodSymbol _currentFrame; private readonly ImmutableArray _locals; private readonly ImmutableDictionary _displayClassVariables; - private readonly ImmutableHashSet _hoistedParameterNames; + private readonly ImmutableArray _sourceMethodParametersInOrder; private readonly ImmutableArray _localsForBinding; private readonly bool _methodNotType; @@ -47,6 +47,7 @@ internal sealed class CompilationContext internal CompilationContext( CSharpCompilation compilation, MethodSymbol currentFrame, + MethodSymbol currentSourceMethod, ImmutableArray locals, ImmutableSortedSet inScopeHoistedLocalSlots, MethodDebugInfo methodDebugInfo) @@ -77,11 +78,12 @@ internal sealed class CompilationContext ImmutableArray displayClassVariableNamesInOrder; GetDisplayClassVariables( currentFrame, + currentSourceMethod, _locals, inScopeHoistedLocalSlots, out displayClassVariableNamesInOrder, out _displayClassVariables, - out _hoistedParameterNames); + out _sourceMethodParametersInOrder); Debug.Assert(displayClassVariableNamesInOrder.Length == _displayClassVariables.Count); _localsForBinding = GetLocalsForBinding(_locals, displayClassVariableNamesInOrder, _displayClassVariables); } @@ -350,7 +352,7 @@ private static string GetNextMethodName(ArrayBuilder builder) // "this" for non-static methods that are not display class methods or // display class methods where the display class contains "<>4__this". - if ((!m.IsStatic && !IsDisplayClassType(m.ContainingType)) || GetThisProxy( _displayClassVariables) != null) + if ((!m.IsStatic && !IsDisplayClassType(m.ContainingType)) || GetThisProxy(_displayClassVariables) != null) { var methodName = GetNextMethodName(methodBuilder); var method = this.GetThisMethod(container, methodName); @@ -359,46 +361,56 @@ private static string GetNextMethodName(ArrayBuilder builder) } } - // Hoisted method parameters (represented as locals in the EE). - if (!_hoistedParameterNames.IsEmpty) - { - int localIndex = 0; - foreach (var local in _localsForBinding) - { - // Since we are showing hoisted method parameters first, the parameters may appear out of order - // in the Locals window if only some of the parameters are hoisted. This is consistent with the - // behavior of the old EE. - if (_hoistedParameterNames.Contains(local.Name)) - { - AppendLocalAndMethod(localBuilder, methodBuilder, local, container, localIndex, GetLocalResultFlags(local)); - } + var itemsAdded = PooledHashSet.GetInstance(); - localIndex++; - } - } - - // Method parameters (except those that have been hoisted). + // Method parameters int parameterIndex = m.IsStatic ? 0 : 1; foreach (var parameter in m.Parameters) { var parameterName = parameter.Name; - if (!_hoistedParameterNames.Contains(parameterName) && - GeneratedNames.GetKind(parameterName) == GeneratedNameKind.None && + if (GeneratedNames.GetKind(parameterName) == GeneratedNameKind.None && !IsDisplayClassParameter(parameter)) { + itemsAdded.Add(parameterName); AppendParameterAndMethod(localBuilder, methodBuilder, parameter, container, parameterIndex); } parameterIndex++; } + // In case of iterator or async state machine, the 'm' method has no parameters + // but the source method can have parameters to iterate over. + if (itemsAdded.Count == 0 && _sourceMethodParametersInOrder.Length != 0) + { + var localsDictionary = PooledDictionary.GetInstance(); + int localIndex = 0; + foreach (var local in _localsForBinding) + { + localsDictionary.Add(local.Name, (local, localIndex)); + localIndex++; + } + + foreach (var argumentName in _sourceMethodParametersInOrder) + { + (LocalSymbol local, int localIndex) localSymbolAndIndex; + if (localsDictionary.TryGetValue(argumentName, out localSymbolAndIndex)) + { + itemsAdded.Add(argumentName); + var local = localSymbolAndIndex.local; + AppendLocalAndMethod(localBuilder, methodBuilder, local, container, localSymbolAndIndex.localIndex, GetLocalResultFlags(local)); + } + } + + localsDictionary.Free(); + } + if (!argumentsOnly) { - // Locals. + // Locals which were not added as parameters or parameters of the source method. int localIndex = 0; foreach (var local in _localsForBinding) { - if (!_hoistedParameterNames.Contains(local.Name)) + if (!itemsAdded.Contains(local.Name)) { AppendLocalAndMethod(localBuilder, methodBuilder, local, container, localIndex, GetLocalResultFlags(local)); } @@ -421,6 +433,7 @@ private static string GetNextMethodName(ArrayBuilder builder) } } + itemsAdded.Free(); return methodBuilder.ToImmutableAndFree(); }); @@ -1201,11 +1214,12 @@ private static DkmClrCompilationResultFlags GetLocalResultFlags(LocalSymbol loca /// private static void GetDisplayClassVariables( MethodSymbol method, + MethodSymbol sourceMethod, ImmutableArray locals, ImmutableSortedSet inScopeHoistedLocalSlots, out ImmutableArray displayClassVariableNamesInOrder, out ImmutableDictionary displayClassVariables, - out ImmutableHashSet hoistedParameterNames) + out ImmutableArray sourceMethodParametersInOrder) { // Calculated the shortest paths from locals to instances of display // classes. There should not be two instances of the same display @@ -1250,6 +1264,48 @@ private static DkmClrCompilationResultFlags GetLocalResultFlags(LocalSymbol loca isIteratorOrAsyncMethod = GeneratedNames.GetKind(containingType.Name) == GeneratedNameKind.StateMachineType; } + + var parameterNamesInOrder = ArrayBuilder.GetInstance(); + // For version before .NET 4.5, we cannot find the sourceMethod properly: + // The source method coincides with the original method in the case. + // Therefore, for iterators and async state machines, we have to get parameters from the containingType. + // This does not guarantee the proper order of parameters. + if (isIteratorOrAsyncMethod && method == sourceMethod) + { + Debug.Assert(IsDisplayClassType(containingType)); + foreach (var member in containingType.GetMembers()) + { + if (member.Kind != SymbolKind.Field) + { + continue; + } + + var field = (FieldSymbol)member; + var fieldName = field.Name; + if (GeneratedNames.GetKind(fieldName) == GeneratedNameKind.None) + { + parameterNamesInOrder.Add(fieldName); + } + } + } + else + { + if (sourceMethod != null) + { + foreach (var p in sourceMethod.Parameters) + { + parameterNamesInOrder.Add(p.Name); + } + } + } + + var parameterNames = PooledHashSet.GetInstance(); + foreach (var name in parameterNamesInOrder) + { + parameterNames.Add(name); + } + + sourceMethodParametersInOrder = parameterNamesInOrder.ToImmutableAndFree(); if (displayClassInstances.Any()) { @@ -1262,31 +1318,6 @@ private static DkmClrCompilationResultFlags GetLocalResultFlags(LocalSymbol loca var displayClassVariableNamesInOrderBuilder = ArrayBuilder.GetInstance(); var displayClassVariablesBuilder = PooledDictionary.GetInstance(); - var parameterNames = PooledHashSet.GetInstance(); - if (isIteratorOrAsyncMethod) - { - Debug.Assert(IsDisplayClassType(containingType)); - - foreach (var field in containingType.GetMembers().OfType()) - { - // All iterator and async state machine fields (including hoisted locals) have mangled names, except - // for hoisted parameters (whose field names are always the same as the original source parameters). - var fieldName = field.Name; - if (GeneratedNames.GetKind(fieldName) == GeneratedNameKind.None) - { - parameterNames.Add(fieldName); - } - } - } - else - { - foreach (var p in method.Parameters) - { - parameterNames.Add(p.Name); - } - } - - var pooledHoistedParameterNames = PooledHashSet.GetInstance(); foreach (var instance in displayClassInstances) { GetDisplayClassVariables( @@ -1294,25 +1325,20 @@ private static DkmClrCompilationResultFlags GetLocalResultFlags(LocalSymbol loca displayClassVariablesBuilder, parameterNames, inScopeHoistedLocalSlots, - instance, - pooledHoistedParameterNames); + instance); } - hoistedParameterNames = pooledHoistedParameterNames.ToImmutableHashSet(); - pooledHoistedParameterNames.Free(); - parameterNames.Free(); - displayClassVariableNamesInOrder = displayClassVariableNamesInOrderBuilder.ToImmutableAndFree(); displayClassVariables = displayClassVariablesBuilder.ToImmutableDictionary(); displayClassVariablesBuilder.Free(); } else { - hoistedParameterNames = ImmutableHashSet.Empty; displayClassVariableNamesInOrder = ImmutableArray.Empty; displayClassVariables = ImmutableDictionary.Empty; } + parameterNames.Free(); displayClassTypes.Free(); displayClassInstances.Free(); } @@ -1417,8 +1443,7 @@ private static bool IsDisplayClassParameter(ParameterSymbol parameter) Dictionary displayClassVariablesBuilder, HashSet parameterNames, ImmutableSortedSet inScopeHoistedLocalSlots, - DisplayClassInstanceAndFields instance, - HashSet hoistedParameterNames) + DisplayClassInstanceAndFields instance) { // Display class instance. The display class fields are variables. foreach (var member in instance.Type.GetMembers()) @@ -1477,7 +1502,6 @@ private static bool IsDisplayClassParameter(ParameterSymbol parameter) if (parameterNames.Contains(variableName)) { variableKind = DisplayClassVariableKind.Parameter; - hoistedParameterNames.Add(variableName); } else { diff --git a/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/CompilationExtensions.cs b/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/CompilationExtensions.cs index d74861abb8a4fc95876799602e93e0b7adedd3f4..07591465a09ddf394657ca508c9229424cc4e724 100644 --- a/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/CompilationExtensions.cs +++ b/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/CompilationExtensions.cs @@ -25,9 +25,8 @@ internal static PENamedTypeSymbol GetType(this CSharpCompilation compilation, Gu return GetType(compilation.GetModule(moduleVersionId), (TypeDefinitionHandle)MetadataTokens.Handle(typeToken)); } - internal static PEMethodSymbol GetSourceMethod(this CSharpCompilation compilation, Guid moduleVersionId, int methodToken) + internal static PEMethodSymbol GetSourceMethod(this CSharpCompilation compilation, Guid moduleVersionId, MethodDefinitionHandle methodHandle) { - var methodHandle = (MethodDefinitionHandle)MetadataTokens.Handle(methodToken); var method = GetMethod(compilation, moduleVersionId, methodHandle); var metadataDecoder = new MetadataDecoder((PEModuleSymbol)method.ContainingModule); var containingType = method.ContainingType; diff --git a/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/EvaluationContext.cs b/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/EvaluationContext.cs index 3d6808a579e8c78e13627be4056fd5cad7aae9f0..e57096694c314c21edeea4c776b9a7d88fc33e71 100644 --- a/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/EvaluationContext.cs +++ b/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/EvaluationContext.cs @@ -31,6 +31,7 @@ internal sealed class EvaluationContext : EvaluationContextBase internal readonly CSharpCompilation Compilation; private readonly MethodSymbol _currentFrame; + private readonly MethodSymbol _currentSourceMethod; private readonly ImmutableArray _locals; private readonly ImmutableSortedSet _inScopeHoistedLocalSlots; private readonly MethodDebugInfo _methodDebugInfo; @@ -39,6 +40,7 @@ internal sealed class EvaluationContext : EvaluationContextBase MethodContextReuseConstraints? methodContextReuseConstraints, CSharpCompilation compilation, MethodSymbol currentFrame, + MethodSymbol currentSourceMethod, ImmutableArray locals, ImmutableSortedSet inScopeHoistedLocalSlots, MethodDebugInfo methodDebugInfo) @@ -49,6 +51,7 @@ internal sealed class EvaluationContext : EvaluationContextBase this.MethodContextReuseConstraints = methodContextReuseConstraints; this.Compilation = compilation; _currentFrame = currentFrame; + _currentSourceMethod = currentSourceMethod; _locals = locals; _inScopeHoistedLocalSlots = inScopeHoistedLocalSlots; _methodDebugInfo = methodDebugInfo; @@ -93,6 +96,7 @@ internal sealed class EvaluationContext : EvaluationContextBase null, compilation, currentFrame, + null, default(ImmutableArray), ImmutableSortedSet.Empty, MethodDebugInfo.None); @@ -180,6 +184,7 @@ internal sealed class EvaluationContext : EvaluationContextBase int localSignatureToken) { var methodHandle = (MethodDefinitionHandle)MetadataTokens.Handle(methodToken); + var currentSourceMethod = compilation.GetSourceMethod(moduleVersionId, methodHandle); var localSignatureHandle = (localSignatureToken != 0) ? (StandaloneSignatureHandle)MetadataTokens.Handle(localSignatureToken) : default(StandaloneSignatureHandle); var currentFrame = compilation.GetMethod(moduleVersionId, methodHandle); @@ -211,6 +216,7 @@ internal sealed class EvaluationContext : EvaluationContextBase new MethodContextReuseConstraints(moduleVersionId, methodToken, methodVersion, reuseSpan), compilation, currentFrame, + currentSourceMethod, localsBuilder.ToImmutableAndFree(), inScopeHoistedLocals, debugInfo); @@ -221,6 +227,7 @@ internal CompilationContext CreateCompilationContext() return new CompilationContext( this.Compilation, _currentFrame, + _currentSourceMethod, _locals, _inScopeHoistedLocalSlots, _methodDebugInfo); diff --git a/src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/InstructionDecoderTests.cs b/src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/InstructionDecoderTests.cs index 1531923f7c3cec400a94ab8c585596ec74fc4622..0ea335a4b87815ec04a7210779282dd4862e5271 100644 --- a/src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/InstructionDecoderTests.cs +++ b/src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/InstructionDecoderTests.cs @@ -453,7 +453,7 @@ private MethodSymbol GetConstructedMethod(string source, string methodName, stri // async/iterator "MoveNext" methods to the original source method. MethodSymbol method = compilation.GetSourceMethod( ((PEModuleSymbol)frame.ContainingModule).Module.GetModuleVersionIdOrThrow(), - MetadataTokens.GetToken(frame.Handle)); + frame.Handle); if (serializedTypeArgumentNames != null) { Assert.NotEmpty(serializedTypeArgumentNames); diff --git a/src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/LocalsTests.cs b/src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/LocalsTests.cs index 13ab25d03c2299acd38c9f3cc882d880b8b07a69..48cc05bba2f94aa6deb98e3000be28545be41705 100644 --- a/src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/LocalsTests.cs +++ b/src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/LocalsTests.cs @@ -1554,7 +1554,8 @@ static void Main() string typeName; context.CompileGetLocals(locals, argumentsOnly: false, typeName: out typeName, testData: testData); - VerifyLocal(testData, typeName, locals[0], "<>m0", "x2", expectedILOpt: + VerifyLocal(testData, typeName, locals[0], "<>m0", "x1"); + VerifyLocal(testData, typeName, locals[1], "<>m1", "x2", expectedILOpt: @" { // Code size 7 (0x7) @@ -1566,9 +1567,9 @@ .maxstack 1 IL_0001: ldfld ""object C.<>c__DisplayClass0_0.x2"" IL_0006: ret }"); - VerifyLocal(testData, typeName, locals[1], "<>m1", "x3"); - VerifyLocal(testData, typeName, locals[2], "<>m2", "x4"); - VerifyLocal(testData, typeName, locals[3], "<>m3", "x1"); + VerifyLocal(testData, typeName, locals[2], "<>m2", "x3"); + VerifyLocal(testData, typeName, locals[3], "<>m3", "x4"); + Assert.Equal(locals.Count, 4); locals.Free(); @@ -1581,7 +1582,8 @@ .maxstack 1 locals = ArrayBuilder.GetInstance(); context.CompileGetLocals(locals, argumentsOnly: false, typeName: out typeName, testData: testData); - VerifyLocal(testData, typeName, locals[0], "<>m0", "y2", expectedILOpt: + VerifyLocal(testData, typeName, locals[0], "<>m0", "y1"); + VerifyLocal(testData, typeName, locals[1], "<>m1", "y2", expectedILOpt: @"{ // Code size 7 (0x7) .maxstack 1 @@ -1592,8 +1594,7 @@ .maxstack 1 IL_0001: ldfld ""object C.<>c__DisplayClass0_1.y2"" IL_0006: ret }"); - VerifyLocal(testData, typeName, locals[1], "<>m1", "y3"); - VerifyLocal(testData, typeName, locals[2], "<>m2", "y1"); + VerifyLocal(testData, typeName, locals[2], "<>m2", "y3"); VerifyLocal(testData, typeName, locals[3], "<>m3", "x2"); VerifyLocal(testData, typeName, locals[4], "<>m4", "x3", expectedILOpt: @"{ @@ -1618,7 +1619,8 @@ .maxstack 1 locals = ArrayBuilder.GetInstance(); context.CompileGetLocals(locals, argumentsOnly: false, typeName: out typeName, testData: testData); - VerifyLocal(testData, typeName, locals[0], "<>m0", "z2", expectedILOpt: + VerifyLocal(testData, typeName, locals[0], "<>m0", "z1"); + VerifyLocal(testData, typeName, locals[1], "<>m1", "z2", expectedILOpt: @"{ // Code size 7 (0x7) .maxstack 1 @@ -1629,7 +1631,6 @@ .maxstack 1 IL_0001: ldfld ""object C.<>c__DisplayClass0_2.z2"" IL_0006: ret }"); - VerifyLocal(testData, typeName, locals[1], "<>m1", "z1"); VerifyLocal(testData, typeName, locals[2], "<>m2", "y2"); VerifyLocal(testData, typeName, locals[3], "<>m3", "y3"); VerifyLocal(testData, typeName, locals[4], "<>m4", "x2"); @@ -2839,6 +2840,7 @@ static IEnumerable M2(int x, int y) yield return local; } }"; + var compilation = CreateStandardCompilation(source, options: TestOptions.DebugDll); WithRuntimeInstance(compilation, runtime => { @@ -3806,6 +3808,54 @@ static void Main() }); } + [WorkItem(298297, "https://devdiv.visualstudio.com/DevDiv/_workitems?id=298297")] + [Fact] + public void OrderOfArguments_ArgumentsOnly() + { + var source = +@"using System.Collections.Generic; +class C +{ + static IEnumerable F(object y, object x) + { + yield return x; +#line 500 + DummySequencePoint(); + yield return y; + } + static void DummySequencePoint() + { + } +}"; + var comp = CreateCompilationWithMscorlib45(source, options: TestOptions.ReleaseDll); + WithRuntimeInstance(comp, runtime => + { + EvaluationContext context; + context = CreateMethodContext(runtime, "C.d__0.MoveNext", atLineNumber: 500); + string unused; + var locals = new ArrayBuilder(); + context.CompileGetLocals(locals, argumentsOnly: true, typeName: out unused, testData: null); + var names = locals.Select(l => l.LocalName).ToArray(); + // The order must confirm the order of the arguments in the method signature. + Assert.Equal(names, new[] { "y", "x" }); + locals.Free(); + }); + + comp = CreateStandardCompilation(source, options: TestOptions.ReleaseDll); + WithRuntimeInstance(comp, runtime => + { + EvaluationContext context; + context = CreateMethodContext(runtime, "C.d__0.MoveNext", atLineNumber: 500); + string unused; + var locals = new ArrayBuilder(); + context.CompileGetLocals(locals, argumentsOnly: true, typeName: out unused, testData: null); + var names = locals.Select(l => l.LocalName).ToArray(); + // The problem is not fixed in versions before 4.5: the order of arguments can be wrong. + Assert.Equal(names, new[] { "x", "y" }); + locals.Free(); + }); + } + private static void GetLocals(RuntimeInstance runtime, string methodName, bool argumentsOnly, ArrayBuilder locals, int count, out string typeName, out CompilationTestData testData) { var context = CreateMethodContext(runtime, methodName); diff --git a/src/ExpressionEvaluator/VisualBasic/Source/ExpressionCompiler/CompilationContext.vb b/src/ExpressionEvaluator/VisualBasic/Source/ExpressionCompiler/CompilationContext.vb index 91542809ddfa54d05cf9fd15941c2ba3159ea892..e07b06ac90f2328a46328e54015f0071552f9c77 100644 --- a/src/ExpressionEvaluator/VisualBasic/Source/ExpressionCompiler/CompilationContext.vb +++ b/src/ExpressionEvaluator/VisualBasic/Source/ExpressionCompiler/CompilationContext.vb @@ -33,7 +33,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ExpressionEvaluator Private ReadOnly _currentFrame As MethodSymbol Private ReadOnly _locals As ImmutableArray(Of LocalSymbol) Private ReadOnly _displayClassVariables As ImmutableDictionary(Of String, DisplayClassVariable) - Private ReadOnly _hoistedParameterNames As ImmutableHashSet(Of String) + Private ReadOnly _sourceMethodParametersInOrder As ImmutableArray(Of String) Private ReadOnly _localsForBinding As ImmutableArray(Of LocalSymbol) Private ReadOnly _methodNotType As Boolean Private ReadOnly _voidType As NamedTypeSymbol @@ -44,6 +44,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ExpressionEvaluator Friend Sub New( compilation As VisualBasicCompilation, currentFrame As MethodSymbol, + currentSourceMethod As MethodSymbol, locals As ImmutableArray(Of LocalSymbol), inScopeHoistedLocalSlots As ImmutableSortedSet(Of Integer), methodDebugInfo As MethodDebugInfo(Of TypeSymbol, LocalSymbol), @@ -96,7 +97,14 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ExpressionEvaluator If _methodNotType Then _locals = locals Dim displayClassVariableNamesInOrder As ImmutableArray(Of String) = Nothing - GetDisplayClassVariables(currentFrame, locals, inScopeHoistedLocalSlots, displayClassVariableNamesInOrder, _displayClassVariables, _hoistedParameterNames) + GetDisplayClassVariables( + currentFrame, + currentSourceMethod, + locals, + inScopeHoistedLocalSlots, + displayClassVariableNamesInOrder, + _displayClassVariables, + _sourceMethodParametersInOrder) Debug.Assert(displayClassVariableNamesInOrder.Length = _displayClassVariables.Count) _localsForBinding = GetLocalsForBinding(locals, displayClassVariableNamesInOrder, _displayClassVariables) Else @@ -257,38 +265,48 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ExpressionEvaluator End If End If - ' Hoisted method parameters (represented as locals in the EE). - If Not _hoistedParameterNames.IsEmpty Then - Dim localIndex As Integer = 0 - - For Each local In _localsForBinding - ' Since we are showing hoisted method parameters first, the parameters may appear out of order - ' in the Locals window if only some of the parameters are hoisted. This is consistent with the - ' behavior of the old EE. - If _hoistedParameterNames.Contains(local.Name) Then - AppendLocalAndMethod(localBuilder, methodBuilder, local, container, localIndex, GetLocalResultFlags(local)) - End If + Dim itemsAdded = PooledHashSet(Of String).GetInstance() - localIndex += 1 - Next - End If - - ' Method parameters (except those that have been hoisted). + ' Method parameters Dim parameterIndex = If(m.IsShared, 0, 1) For Each parameter In m.Parameters Dim parameterName As String = parameter.Name - If Not _hoistedParameterNames.Contains(parameterName) AndAlso GeneratedNames.GetKind(parameterName) = GeneratedNameKind.None Then + If GeneratedNames.GetKind(parameterName) = GeneratedNameKind.None Then AppendParameterAndMethod(localBuilder, methodBuilder, parameter, container, parameterIndex) + itemsAdded.Add(parameterName) End If parameterIndex += 1 Next + ' In case of iterator Or async state machine, the 'm' method has no parameters + ' but the source method can have parameters to iterate over. + If itemsAdded.Count = 0 AndAlso _sourceMethodParametersInOrder.Length <> 0 Then + Dim localsDictionary = PooledDictionary(Of String, (LocalSymbol, Integer)).GetInstance() + Dim localIndex = 0 + For Each local In _localsForBinding + localsDictionary.Add(local.Name, (local, localIndex)) + localIndex += 1 + Next + + For Each argumentName In _sourceMethodParametersInOrder + + Dim localSymbolAndIndex As (LocalSymbol, Integer) = Nothing + If localsDictionary.TryGetValue(argumentName, localSymbolAndIndex) Then + itemsAdded.Add(argumentName) + Dim local = localSymbolAndIndex.Item1 + AppendLocalAndMethod(localBuilder, methodBuilder, local, container, localSymbolAndIndex.Item2, GetLocalResultFlags(local)) + End If + Next + + localsDictionary.Free() + End If + If Not argumentsOnly Then ' Locals. Dim localIndex As Integer = 0 For Each local In _localsForBinding - If Not _hoistedParameterNames.Contains(local.Name) Then + If Not itemsAdded.Contains(local.Name) Then AppendLocalAndMethod(localBuilder, methodBuilder, local, container, localIndex, GetLocalResultFlags(local)) End If @@ -309,6 +327,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ExpressionEvaluator End If End If + itemsAdded.Free() Return methodBuilder.ToImmutableAndFree() End Function) @@ -998,11 +1017,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ExpressionEvaluator ''' Private Shared Sub GetDisplayClassVariables( method As MethodSymbol, + sourceMethod As MethodSymbol, locals As ImmutableArray(Of LocalSymbol), inScopeHoistedLocalSlots As ImmutableSortedSet(Of Integer), ByRef displayClassVariableNamesInOrder As ImmutableArray(Of String), ByRef displayClassVariables As ImmutableDictionary(Of String, DisplayClassVariable), - ByRef hoistedParameterNames As ImmutableHashSet(Of String)) + ByRef sourceMethodParametersInOrder As ImmutableArray(Of String)) ' Calculated the shortest paths from locals to instances of display classes. ' There should not be two instances of the same display class immediately @@ -1041,6 +1061,44 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ExpressionEvaluator isIteratorOrAsyncMethod = containingType.IsStateMachineType() End If + Dim parameterNamesInOrder = ArrayBuilder(Of String).GetInstance() + + ' For version before .NET 4.5, we cannot find the sourceMethod properly: + ' The source method coincides with the original method in the case. + ' Here, we have to get parameters from the containingType. + ' This does not guarantee the proper order of parameters. + If isIteratorOrAsyncMethod AndAlso method = sourceMethod Then + Debug.Assert(containingType.IsClosureOrStateMachineType()) + + For Each member In containingType.GetMembers + ' All iterator and async state machine fields in VB have mangled names. + ' The ones beginning with "$VB$Local_" are the hoisted parameters. + If member.Kind <> SymbolKind.Field Then + Continue For + End If + + Dim field = DirectCast(member, FieldSymbol) + Dim fieldName = field.Name + Dim parameterName As String = Nothing + If GeneratedNames.TryParseHoistedUserVariableName(fieldName, parameterName) Then + parameterNamesInOrder.Add(parameterName) + End If + Next + Else + If sourceMethod <> Nothing Then + For Each parameter In sourceMethod.Parameters + parameterNamesInOrder.Add(parameter.Name) + Next + End If + End If + + Dim parameterNames = PooledHashSet(Of String).GetInstance() + For Each p In parameterNamesInOrder + parameterNames.Add(p) + Next + + sourceMethodParametersInOrder = parameterNamesInOrder.ToImmutableAndFree() + If displayClassInstances.Any() Then ' Find any additional display class instances breadth first. Dim depth = 0 @@ -1052,49 +1110,24 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ExpressionEvaluator Dim displayClassVariableNamesInOrderBuilder = ArrayBuilder(Of String).GetInstance() Dim displayClassVariablesBuilder = PooledDictionary(Of String, DisplayClassVariable).GetInstance() - Dim parameterNames = PooledHashSet(Of String).GetInstance() - If isIteratorOrAsyncMethod Then - Debug.Assert(containingType.IsClosureOrStateMachineType()) - - For Each field In containingType.GetMembers.OfType(Of FieldSymbol)() - ' All iterator and async state machine fields in VB have mangled names. - ' The ones beginning with "$VB$Local_" are the hoisted parameters. - Dim fieldName = field.Name - Dim parameterName As String = Nothing - If GeneratedNames.TryParseHoistedUserVariableName(fieldName, parameterName) Then - parameterNames.Add(parameterName) - End If - Next - Else - For Each parameter In method.Parameters - parameterNames.Add(parameter.Name) - Next - End If - - Dim pooledHoistedParameterNames = PooledHashSet(Of String).GetInstance() For Each instance In displayClassInstances GetDisplayClassVariables( displayClassVariableNamesInOrderBuilder, displayClassVariablesBuilder, parameterNames, inScopeHoistedLocalSlots, - instance, - pooledHoistedParameterNames) + instance) Next - hoistedParameterNames = pooledHoistedParameterNames.ToImmutableHashSet() - pooledHoistedParameterNames.Free() - parameterNames.Free() - displayClassVariableNamesInOrder = displayClassVariableNamesInOrderBuilder.ToImmutableAndFree() displayClassVariables = displayClassVariablesBuilder.ToImmutableDictionary() displayClassVariablesBuilder.Free() Else - hoistedParameterNames = ImmutableHashSet(Of String).Empty displayClassVariableNamesInOrder = ImmutableArray(Of String).Empty displayClassVariables = ImmutableDictionary(Of String, DisplayClassVariable).Empty End If + parameterNames.Free() displayClassTypes.Free() displayClassInstances.Free() End Sub @@ -1211,8 +1244,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ExpressionEvaluator displayClassVariablesBuilder As Dictionary(Of String, DisplayClassVariable), parameterNames As HashSet(Of String), inScopeHoistedLocalSlots As ImmutableSortedSet(Of Integer), - instance As DisplayClassInstanceAndFields, - hoistedParameterNames As HashSet(Of String)) + instance As DisplayClassInstanceAndFields) ' Display class instance. The display class fields are variables. For Each member In instance.Type.GetMembers() @@ -1258,7 +1290,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ExpressionEvaluator variableName = fieldName ' As in C#, we retain the mangled name. It shouldn't be used, other than as a dictionary key. ElseIf fieldName.StartsWith(StringConstants.LambdaCacheFieldPrefix, StringComparison.Ordinal) Then Continue For - ElseIf GeneratedNames.GetKind(fieldName) = GeneratedNameKind.TransparentIdentifier + ElseIf GeneratedNames.GetKind(fieldName) = GeneratedNameKind.TransparentIdentifier Then ' A transparent identifier (field) in an anonymous type synthesized for a transparent identifier. Debug.Assert(Not field.IsShared) Continue For @@ -1273,7 +1305,6 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ExpressionEvaluator If variableKind = DisplayClassVariableKind.Local AndAlso parameterNames.Contains(variableName) Then variableKind = DisplayClassVariableKind.Parameter - hoistedParameterNames.Add(variableName) End If If displayClassVariablesBuilder.ContainsKey(variableName) Then diff --git a/src/ExpressionEvaluator/VisualBasic/Source/ExpressionCompiler/CompilationExtensions.vb b/src/ExpressionEvaluator/VisualBasic/Source/ExpressionCompiler/CompilationExtensions.vb index 14d01bdb220cdbd3166d95c58aca82c34acf1302..95a070c910f99eff6f155485ebf31bdd566166ab 100644 --- a/src/ExpressionEvaluator/VisualBasic/Source/ExpressionCompiler/CompilationExtensions.vb +++ b/src/ExpressionEvaluator/VisualBasic/Source/ExpressionCompiler/CompilationExtensions.vb @@ -24,8 +24,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ExpressionEvaluator End Function - Friend Function GetSourceMethod(compilation As VisualBasicCompilation, moduleVersionId As Guid, methodToken As Integer) As PEMethodSymbol - Dim methodHandle = CType(MetadataTokens.Handle(methodToken), MethodDefinitionHandle) + Friend Function GetSourceMethod(compilation As VisualBasicCompilation, moduleVersionId As Guid, methodHandle As MethodDefinitionHandle) As PEMethodSymbol Dim method = GetMethod(compilation, moduleVersionId, methodHandle) Dim metadataDecoder = New MetadataDecoder(DirectCast(method.ContainingModule, PEModuleSymbol)) Dim containingType = method.ContainingType diff --git a/src/ExpressionEvaluator/VisualBasic/Source/ExpressionCompiler/EvaluationContext.vb b/src/ExpressionEvaluator/VisualBasic/Source/ExpressionCompiler/EvaluationContext.vb index c060e30e3da68965755de934cbc4a587b3ad2d9d..9a3d5b00f989128e5b9d2fd06faf919d36070be2 100644 --- a/src/ExpressionEvaluator/VisualBasic/Source/ExpressionCompiler/EvaluationContext.vb +++ b/src/ExpressionEvaluator/VisualBasic/Source/ExpressionCompiler/EvaluationContext.vb @@ -36,6 +36,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ExpressionEvaluator Friend ReadOnly Compilation As VisualBasicCompilation Private ReadOnly _currentFrame As MethodSymbol + Private ReadOnly _currentSourceMethod As MethodSymbol Private ReadOnly _locals As ImmutableArray(Of LocalSymbol) Private ReadOnly _inScopeHoistedLocalSlots As ImmutableSortedSet(Of Integer) Private ReadOnly _methodDebugInfo As MethodDebugInfo(Of TypeSymbol, LocalSymbol) @@ -44,6 +45,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ExpressionEvaluator methodContextReuseConstraints As MethodContextReuseConstraints?, compilation As VisualBasicCompilation, currentFrame As MethodSymbol, + currentSourceMethod As MethodSymbol, locals As ImmutableArray(Of LocalSymbol), inScopeHoistedLocalSlots As ImmutableSortedSet(Of Integer), methodDebugInfo As MethodDebugInfo(Of TypeSymbol, LocalSymbol)) @@ -51,6 +53,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ExpressionEvaluator Me.MethodContextReuseConstraints = methodContextReuseConstraints Me.Compilation = compilation _currentFrame = currentFrame + _currentSourceMethod = currentSourceMethod _locals = locals _inScopeHoistedLocalSlots = inScopeHoistedLocalSlots _methodDebugInfo = methodDebugInfo @@ -96,6 +99,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ExpressionEvaluator Nothing, compilation, currentFrame, + Nothing, locals:=Nothing, inScopeHoistedLocalSlots:=Nothing, methodDebugInfo:=MethodDebugInfo(Of TypeSymbol, LocalSymbol).None) @@ -184,6 +188,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ExpressionEvaluator localSignatureToken As Integer) As EvaluationContext Dim methodHandle = CType(MetadataTokens.Handle(methodToken), MethodDefinitionHandle) + Dim currentSourceMethod = compilation.GetSourceMethod(moduleVersionId, methodHandle) Dim localSignatureHandle = If(localSignatureToken <> 0, CType(MetadataTokens.Handle(localSignatureToken), StandaloneSignatureHandle), Nothing) Dim currentFrame = compilation.GetMethod(moduleVersionId, methodHandle) @@ -224,6 +229,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ExpressionEvaluator New MethodContextReuseConstraints(moduleVersionId, methodToken, methodVersion, reuseSpan), compilation, currentFrame, + currentSourceMethod, localsBuilder.ToImmutableAndFree(), inScopeHoistedLocalSlots, debugInfo) @@ -363,6 +369,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ExpressionEvaluator Return New CompilationContext( Compilation, _currentFrame, + _currentSourceMethod, _locals, _inScopeHoistedLocalSlots, _methodDebugInfo, diff --git a/src/ExpressionEvaluator/VisualBasic/Source/ExpressionCompiler/VisualBasicInstructionDecoder.vb b/src/ExpressionEvaluator/VisualBasic/Source/ExpressionCompiler/VisualBasicInstructionDecoder.vb index d579bffaa6e105a92352ec96b876ee5506dcb578..57c4c65f278f7578b05b8b16e7d4b736f2904194 100644 --- a/src/ExpressionEvaluator/VisualBasic/Source/ExpressionCompiler/VisualBasicInstructionDecoder.vb +++ b/src/ExpressionEvaluator/VisualBasic/Source/ExpressionCompiler/VisualBasicInstructionDecoder.vb @@ -1,6 +1,8 @@ ' Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. Imports System.Collections.Immutable +Imports System.Reflection.Metadata +Imports System.Reflection.Metadata.Ecma335 Imports Microsoft.CodeAnalysis.ExpressionEvaluator Imports Microsoft.CodeAnalysis.VisualBasic.Symbols Imports Microsoft.CodeAnalysis.VisualBasic.Symbols.Metadata.PE @@ -97,7 +99,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ExpressionEvaluator End Function Friend Overrides Function GetMethod(compilation As VisualBasicCompilation, instructionAddress As DkmClrInstructionAddress) As MethodSymbol - Return compilation.GetSourceMethod(instructionAddress.ModuleInstance.Mvid, instructionAddress.MethodId.Token) + Dim methodHandle = CType(MetadataTokens.Handle(instructionAddress.MethodId.Token), MethodDefinitionHandle) + Return compilation.GetSourceMethod(instructionAddress.ModuleInstance.Mvid, methodHandle) End Function Friend Overrides Function GetTypeNameDecoder(compilation As VisualBasicCompilation, method As MethodSymbol) As TypeNameDecoder(Of PEModuleSymbol, TypeSymbol) diff --git a/src/ExpressionEvaluator/VisualBasic/Test/ExpressionCompiler/InstructionDecoderTests.vb b/src/ExpressionEvaluator/VisualBasic/Test/ExpressionCompiler/InstructionDecoderTests.vb index 42cb988d9533a9db1a2a02241159b516a2d4d482..694d01b9e2ff52579499572e1149d8a37d557797 100644 --- a/src/ExpressionEvaluator/VisualBasic/Test/ExpressionCompiler/InstructionDecoderTests.vb +++ b/src/ExpressionEvaluator/VisualBasic/Test/ExpressionCompiler/InstructionDecoderTests.vb @@ -515,7 +515,7 @@ End Class" ' async/ iterator "MoveNext" methods to the original source method. Dim method As MethodSymbol = compilation.GetSourceMethod( DirectCast(frame.ContainingModule, PEModuleSymbol).Module.GetModuleVersionIdOrThrow(), - MetadataTokens.GetToken(frame.Handle)) + frame.Handle) If serializedTypeArgumentNames IsNot Nothing Then Assert.NotEmpty(serializedTypeArgumentNames) Dim typeParameters = instructionDecoder.GetAllTypeParameters(method) diff --git a/src/ExpressionEvaluator/VisualBasic/Test/ExpressionCompiler/LocalsTests.vb b/src/ExpressionEvaluator/VisualBasic/Test/ExpressionCompiler/LocalsTests.vb index 385141e3215d112b1663f4471c5c85b149c0123f..dbfabd7e69248bc3d6ac44e7932c544ff909f1ce 100644 --- a/src/ExpressionEvaluator/VisualBasic/Test/ExpressionCompiler/LocalsTests.vb +++ b/src/ExpressionEvaluator/VisualBasic/Test/ExpressionCompiler/LocalsTests.vb @@ -910,7 +910,8 @@ End Class Assert.Equal(locals.Count, 4) - VerifyLocal(testData, typeName, locals(0), "<>m0", "x2", expectedILOpt:= + VerifyLocal(testData, typeName, locals(0), "<>m0", "x1") + VerifyLocal(testData, typeName, locals(1), "<>m1", "x2", expectedILOpt:= "{ // Code size 7 (0x7) .maxstack 1 @@ -921,9 +922,8 @@ End Class IL_0001: ldfld ""C._Closure$__1-0.$VB$Local_x2 As Object"" IL_0006: ret }") - VerifyLocal(testData, typeName, locals(1), "<>m1", "x3") - VerifyLocal(testData, typeName, locals(2), "<>m2", "x4") - VerifyLocal(testData, typeName, locals(3), "<>m3", "x1") + VerifyLocal(testData, typeName, locals(2), "<>m2", "x3") + VerifyLocal(testData, typeName, locals(3), "<>m3", "x4") context = CreateMethodContext(runtime, methodName:="C._Closure$__1-0._Lambda$__1") testData = New CompilationTestData() @@ -933,7 +933,8 @@ End Class Assert.Equal(locals.Count, 6) - VerifyLocal(testData, typeName, locals(0), "<>m0", "y2", expectedILOpt:= + VerifyLocal(testData, typeName, locals(0), "<>m0", "y1") + VerifyLocal(testData, typeName, locals(1), "<>m1", "y2", expectedILOpt:= "{ // Code size 7 (0x7) .maxstack 1 @@ -944,8 +945,7 @@ End Class IL_0001: ldfld ""C._Closure$__1-1.$VB$Local_y2 As Object"" IL_0006: ret }") - VerifyLocal(testData, typeName, locals(1), "<>m1", "y3") - VerifyLocal(testData, typeName, locals(2), "<>m2", "y1") + VerifyLocal(testData, typeName, locals(2), "<>m2", "y3") VerifyLocal(testData, typeName, locals(3), "<>m3", "x2") VerifyLocal(testData, typeName, locals(4), "<>m4", "x3", expectedILOpt:= "{ @@ -968,7 +968,8 @@ End Class Assert.Equal(locals.Count, 7) - VerifyLocal(testData, typeName, locals(0), "<>m0", "z2", expectedILOpt:= + VerifyLocal(testData, typeName, locals(0), "<>m0", "z1") + VerifyLocal(testData, typeName, locals(1), "<>m1", "z2", expectedILOpt:= "{ // Code size 7 (0x7) .maxstack 1 @@ -979,7 +980,6 @@ End Class IL_0001: ldfld ""C._Closure$__1-2.$VB$Local_z2 As Object"" IL_0006: ret }") - VerifyLocal(testData, typeName, locals(1), "<>m1", "z1") VerifyLocal(testData, typeName, locals(2), "<>m2", "y2") VerifyLocal(testData, typeName, locals(3), "<>m3", "y3") VerifyLocal(testData, typeName, locals(4), "<>m4", "x2") @@ -3244,6 +3244,41 @@ End Module" End Sub) End Sub + + + Public Sub OrderOfArguments_ArgumentsOnly() + Const source = " +Imports System.Collections.Generic +Class C + Iterator Shared Function F(y As Object, x As Object) As IEnumerable(Of Object) + Yield x + #ExternalSource(""test"", 999) + DummySequencePoint() + #End ExternalSource + Yield y + End Function + + Shared Sub DummySequencePoint() + End Sub +End Class" + Dim comp = CreateCompilationWithMscorlib({source}, {MsvbRef}, options:=TestOptions.DebugDll) + WithRuntimeInstance(comp, + Sub(runtime) + Dim context As EvaluationContext + context = CreateMethodContext(runtime, "C.VB$StateMachine_1_F.MoveNext", atLineNumber:=999) + Dim unused As String = Nothing + Dim locals = ArrayBuilder(Of LocalAndMethod).GetInstance() + context.CompileGetLocals(locals, argumentsOnly:=True, typeName:=unused, testData:=Nothing) + Assert.Equal(2, locals.Count) + ' The order must confirm the order of the arguments in the method signature. + Dim typeName As String = Nothing + Dim testData As CompilationTestData = Nothing + Assert.Equal("y", locals(0).LocalName) + Assert.Equal("x", locals(1).LocalName) + locals.Free() + End Sub) + End Sub + Private Shared Sub GetLocals(runtime As RuntimeInstance, methodName As String, argumentsOnly As Boolean, locals As ArrayBuilder(Of LocalAndMethod), count As Integer, ByRef typeName As String, ByRef testData As CompilationTestData) Dim context = CreateMethodContext(runtime, methodName)