From 24489de5fd35c0d517ad07bf32f517f4c7873c3c Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sat, 19 Sep 2026 08:41:50 +0200 Subject: [PATCH 1/4] Rename CallTransformation to ReferenceTransformation The enum names the ways a member reference can be made more explicit so that it resolves back to the intended member. Field accesses and method groups escalate the same way calls do, and are about to share this vocabulary, at which point "Call" in the name would describe only one of its users. Assisted-by: Claude:claude-opus-5[1m]:Claude Code --- ICSharpCode.Decompiler/CSharp/CallBuilder.cs | 58 ++++++++++---------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/ICSharpCode.Decompiler/CSharp/CallBuilder.cs b/ICSharpCode.Decompiler/CSharp/CallBuilder.cs index 1836d8b7f12..f0ecf61a831 100644 --- a/ICSharpCode.Decompiler/CSharp/CallBuilder.cs +++ b/ICSharpCode.Decompiler/CSharp/CallBuilder.cs @@ -37,7 +37,7 @@ namespace ICSharpCode.Decompiler.CSharp { - struct CallBuilder + partial struct CallBuilder { struct ExpectedTargetDetails { @@ -587,7 +587,7 @@ public ExpressionWithResolveResult Build(OpCode callOpCode, IMethod method, } var transform = GetRequiredTransformationsForCall(expectedTargetDetails, method, ref target, - ref argumentList, CallTransformation.All, out IParameterizedMember? foundMethod); + ref argumentList, ReferenceTransformation.All, out IParameterizedMember? foundMethod); // GetRequiredTransformationsForCall always assigns foundMethod (the resolved overload or 'method'). Debug.Assert(foundMethod != null); @@ -606,15 +606,15 @@ public ExpressionWithResolveResult Build(OpCode callOpCode, IMethod method, Expression targetExpr; string methodName = method.Name; AstNodeCollection typeArgumentList; - if ((transform & CallTransformation.NoNamedArgsForPrettiness) != 0) + if ((transform & ReferenceTransformation.NoNamedArgsForPrettiness) != 0) { argumentList.AddNamesToPrimitiveValues = false; } - if ((transform & CallTransformation.NoOptionalArgumentAllowed) != 0) + if ((transform & ReferenceTransformation.NoOptionalArgumentAllowed) != 0) { argumentList.FirstOptionalArgumentIndex = -1; } - if ((transform & CallTransformation.RequireTarget) != 0) + if ((transform & ReferenceTransformation.RequireTarget) != 0) { targetExpr = new MemberReferenceExpression(target.Expression, methodName); typeArgumentList = ((MemberReferenceExpression)targetExpr).TypeArguments; @@ -640,7 +640,7 @@ public ExpressionWithResolveResult Build(OpCode callOpCode, IMethod method, typeArgumentList = ((IdentifierExpression)targetExpr).TypeArguments; } - if ((transform & CallTransformation.RequireTypeArguments) != 0 && (!settings.AnonymousTypes || !method.TypeArguments.Any(a => a.ContainsAnonymousType()))) + if ((transform & ReferenceTransformation.RequireTypeArguments) != 0 && (!settings.AnonymousTypes || !method.TypeArguments.Any(a => a.ContainsAnonymousType()))) typeArgumentList.AddRange(method.TypeArguments.Select(expressionBuilder.ConvertType)); return new InvocationExpression(targetExpr, argumentList.GetArgumentExpressions()) .WithRR(new CSharpInvocationResolveResult(target.ResolveResult, foundMethod, @@ -752,8 +752,8 @@ public ExpressionWithResolveResult BuildCollectionInitializerExpression(OpCode c argumentList.AddNamesToPrimitiveValues = false; argumentList.UseImplicitlyTypedOut = false; var transform = GetRequiredTransformationsForCall(expectedTargetDetails, method, ref unused, - ref argumentList, CallTransformation.None, out _); - Debug.Assert((transform & ~(CallTransformation.NoOptionalArgumentAllowed | CallTransformation.NoNamedArgsForPrettiness)) == 0); + ref argumentList, ReferenceTransformation.None, out _); + Debug.Assert((transform & ~(ReferenceTransformation.NoOptionalArgumentAllowed | ReferenceTransformation.NoNamedArgsForPrettiness)) == 0); // Calls with only one argument do not need an array initializer expression to wrap them. // Any special cases are handled by the caller (i.e., ExpressionBuilder.TranslateObjectAndCollectionInitializer) @@ -772,7 +772,7 @@ public ExpressionWithResolveResult BuildCollectionInitializerExpression(OpCode c skipCount = 0; } - if ((transform & CallTransformation.NoOptionalArgumentAllowed) != 0) + if ((transform & ReferenceTransformation.NoOptionalArgumentAllowed) != 0) argumentList.FirstOptionalArgumentIndex = -1; return new ArrayInitializerExpression(argumentList.GetArgumentExpressions(skipCount)) @@ -1229,7 +1229,7 @@ bool IsOptionalArgument(IParameter parameter, TranslatedExpression arg) } [Flags] - enum CallTransformation + enum ReferenceTransformation { None = 0, RequireTarget = 1, @@ -1243,15 +1243,15 @@ enum CallTransformation All = 0x1f, } - private CallTransformation GetRequiredTransformationsForCall(ExpectedTargetDetails expectedTargetDetails, IMethod method, - ref TranslatedExpression target, ref ArgumentList argumentList, CallTransformation allowedTransforms, out IParameterizedMember? foundMethod) + private ReferenceTransformation GetRequiredTransformationsForCall(ExpectedTargetDetails expectedTargetDetails, IMethod method, + ref TranslatedExpression target, ref ArgumentList argumentList, ReferenceTransformation allowedTransforms, out IParameterizedMember? foundMethod) { - CallTransformation transform = CallTransformation.None; + ReferenceTransformation transform = ReferenceTransformation.None; // initialize requireTarget flag bool requireTarget; ResolveResult? targetResolveResult; - if ((allowedTransforms & CallTransformation.RequireTarget) != 0) + if ((allowedTransforms & ReferenceTransformation.RequireTarget) != 0) { if (settings.AlwaysQualifyMemberReferences || expressionBuilder.HidesVariableWithName(method.Name)) { @@ -1284,7 +1284,7 @@ private CallTransformation GetRequiredTransformationsForCall(ExpectedTargetDetai bool requireTypeArguments; IType[] typeArguments; bool appliedRequireTypeArgumentsShortcut = false; - if (method.TypeParameters.Count > 0 && (allowedTransforms & CallTransformation.RequireTypeArguments) != 0 + if (method.TypeParameters.Count > 0 && (allowedTransforms & ReferenceTransformation.RequireTypeArguments) != 0 && !IsPossibleExtensionMethodCallOnNull(method, argumentList.Arguments)) { // The ambiguity resolution below only adds type arguments as last resort measure, however there are @@ -1340,13 +1340,13 @@ private CallTransformation GetRequiredTransformationsForCall(ExpectedTargetDetai argumentList.UseImplicitlyTypedOut = false; continue; case OverloadResolutionErrors.TypeInferenceFailed: - if ((allowedTransforms & CallTransformation.RequireTypeArguments) != 0) + if ((allowedTransforms & ReferenceTransformation.RequireTypeArguments) != 0) { goto case OverloadResolutionErrors.WrongNumberOfTypeArguments; } goto default; case OverloadResolutionErrors.WrongNumberOfTypeArguments: - Debug.Assert((allowedTransforms & CallTransformation.RequireTypeArguments) != 0); + Debug.Assert((allowedTransforms & ReferenceTransformation.RequireTypeArguments) != 0); if (requireTypeArguments) goto default; requireTypeArguments = true; @@ -1382,19 +1382,19 @@ private CallTransformation GetRequiredTransformationsForCall(ExpectedTargetDetai argumentList.UseImplicitlyTypedOut = false; CastArguments(argumentList.Arguments, argumentList.ExpectedParameters); } - else if ((allowedTransforms & CallTransformation.RequireTarget) != 0 && !requireTarget) + else if ((allowedTransforms & ReferenceTransformation.RequireTarget) != 0 && !requireTarget) { requireTarget = true; targetResolveResult = target.ResolveResult; } - else if ((allowedTransforms & CallTransformation.RequireTarget) != 0 && !targetCasted) + else if ((allowedTransforms & ReferenceTransformation.RequireTarget) != 0 && !targetCasted) { if (skipTargetCast && requireTarget != originalRequireTarget) { requireTarget = originalRequireTarget; if (!originalRequireTarget) targetResolveResult = null; - allowedTransforms &= ~CallTransformation.RequireTarget; + allowedTransforms &= ~ReferenceTransformation.RequireTarget; } else { @@ -1403,15 +1403,15 @@ private CallTransformation GetRequiredTransformationsForCall(ExpectedTargetDetai targetResolveResult = target.ResolveResult; } } - else if ((allowedTransforms & CallTransformation.RequireTypeArguments) != 0 && !requireTypeArguments) + else if ((allowedTransforms & ReferenceTransformation.RequireTypeArguments) != 0 && !requireTypeArguments) { requireTypeArguments = true; typeArguments = method.TypeArguments.ToArray(); } - else if ((allowedTransforms & CallTransformation.EnforceExplicitIn) != 0) + else if ((allowedTransforms & ReferenceTransformation.EnforceExplicitIn) != 0) { EnforceExplicitIn(argumentList.Arguments, argumentList.ExpectedParameters); - allowedTransforms &= ~CallTransformation.EnforceExplicitIn; + allowedTransforms &= ~ReferenceTransformation.EnforceExplicitIn; } else { @@ -1423,14 +1423,14 @@ private CallTransformation GetRequiredTransformationsForCall(ExpectedTargetDetai foundMethod = method; break; } - if ((allowedTransforms & CallTransformation.RequireTarget) != 0 && requireTarget) - transform |= CallTransformation.RequireTarget; - if ((allowedTransforms & CallTransformation.RequireTypeArguments) != 0 && requireTypeArguments) - transform |= CallTransformation.RequireTypeArguments; + if ((allowedTransforms & ReferenceTransformation.RequireTarget) != 0 && requireTarget) + transform |= ReferenceTransformation.RequireTarget; + if ((allowedTransforms & ReferenceTransformation.RequireTypeArguments) != 0 && requireTypeArguments) + transform |= ReferenceTransformation.RequireTypeArguments; if (argumentList.FirstOptionalArgumentIndex < 0) - transform |= CallTransformation.NoOptionalArgumentAllowed; + transform |= ReferenceTransformation.NoOptionalArgumentAllowed; if (!argumentList.AddNamesToPrimitiveValues) - transform |= CallTransformation.NoNamedArgsForPrettiness; + transform |= ReferenceTransformation.NoNamedArgsForPrettiness; return transform; } From a7f2a98f8bfaf6c98aaeef49bcd322755c1e551a Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sat, 19 Sep 2026 08:42:37 +0200 Subject: [PATCH 2/4] Share one escalation loop across the member-reference builders Emitting a member reference starts from its shortest spelling, which in the output's name-lookup context may bind to a different member than the one the IL referenced. Six places re-resolved what they were about to write and, while it did not bind back, escalated to a more explicit spelling and tried again: field accesses, calls, constructor calls, accessors, and the two method group forms. Each kept its own retry state, and each held targetResolveResult in sync with requireTarget by hand. Disambiguator owns that loop. A For* factory per form of reference builds one, escalates and hands it back finished, naming the steps that form prefers and whether it is written with invocation syntax - the one thing an IMethod does not say, since a method group is not, a property access is not even though it is a call in IL, and a parameterized property's accessor is despite being an accessor. Everything that answers "does this spelling still reach the member" moves with the loop, including the two checks that are asked standalone, and the decision to write type arguments up front for a method whose arguments cannot infer them, which belongs next to the step that takes that guess back. What is left in CallBuilder builds call syntax. CheckSimpleCall now takes its compilation from the resolver rather than the type system; they are the same one. The escalations are already named by ReferenceTransformation, which is also the record of what has been applied, so the allowed-set gating becomes one test and clearing a bit to stop a step repeating becomes unnecessary: one use spends a step, which is also what ends the loop. Spent is tracked apart from what is in effect for the single escalation that is not monotone, where a protected member of a base type cannot be reached through a cast target and the qualifier has to be dropped again. Assisted-by: Claude:claude-opus-5[1m]:Claude Code --- ICSharpCode.Decompiler/CSharp/CallBuilder.cs | 987 +++-------------- .../CSharp/Disambiguator.cs | 995 ++++++++++++++++++ .../CSharp/ExpressionBuilder.cs | 49 +- 3 files changed, 1171 insertions(+), 860 deletions(-) create mode 100644 ICSharpCode.Decompiler/CSharp/Disambiguator.cs diff --git a/ICSharpCode.Decompiler/CSharp/CallBuilder.cs b/ICSharpCode.Decompiler/CSharp/CallBuilder.cs index f0ecf61a831..2dd2aec84a2 100644 --- a/ICSharpCode.Decompiler/CSharp/CallBuilder.cs +++ b/ICSharpCode.Decompiler/CSharp/CallBuilder.cs @@ -37,154 +37,155 @@ namespace ICSharpCode.Decompiler.CSharp { - partial struct CallBuilder + internal struct ExpectedTargetDetails { - struct ExpectedTargetDetails + public OpCode CallOpCode; + public bool NeedsBoxingConversion; + } + + internal struct ArgumentList + { + public TranslatedExpression[] Arguments; + public IParameter[] ExpectedParameters; + public string[] ParameterNames; + public string[]? ArgumentNames; + public int FirstOptionalArgumentIndex; + public BitSet IsPrimitiveValue; + public IReadOnlyList? ArgumentToParameterMap; + + public bool AddNamesToPrimitiveValues; + public bool UseImplicitlyTypedOut; + public bool IsExpandedForm; + public int Length => Arguments.Length; + + private int GetActualArgumentCount() { - public OpCode CallOpCode; - public bool NeedsBoxingConversion; + if (FirstOptionalArgumentIndex < 0) + return Arguments.Length; + return FirstOptionalArgumentIndex; } - struct ArgumentList + public string[]? GetArgumentNames(int skipCount = 0) { - public TranslatedExpression[] Arguments; - public IParameter[] ExpectedParameters; - public string[] ParameterNames; - public string[]? ArgumentNames; - public int FirstOptionalArgumentIndex; - public BitSet IsPrimitiveValue; - public IReadOnlyList? ArgumentToParameterMap; - - public bool AddNamesToPrimitiveValues; - public bool UseImplicitlyTypedOut; - public bool IsExpandedForm; - public int Length => Arguments.Length; - - private int GetActualArgumentCount() + string[]? argumentNames = ArgumentNames; + if (AddNamesToPrimitiveValues && IsPrimitiveValue.Any() && !IsExpandedForm + && !ParameterNames.Any(string.IsNullOrEmpty)) { - if (FirstOptionalArgumentIndex < 0) - return Arguments.Length; - return FirstOptionalArgumentIndex; - } - - public string[]? GetArgumentNames(int skipCount = 0) - { - string[]? argumentNames = ArgumentNames; - if (AddNamesToPrimitiveValues && IsPrimitiveValue.Any() && !IsExpandedForm - && !ParameterNames.Any(string.IsNullOrEmpty)) + Debug.Assert(skipCount == 0); + if (argumentNames == null) { - Debug.Assert(skipCount == 0); - if (argumentNames == null) - { - argumentNames = new string[Arguments.Length]; - } + argumentNames = new string[Arguments.Length]; + } - for (int i = 0; i < Arguments.Length; i++) + for (int i = 0; i < Arguments.Length; i++) + { + if (IsPrimitiveValue[i] && argumentNames[i] == null) { - if (IsPrimitiveValue[i] && argumentNames[i] == null) - { - argumentNames[i] = ParameterNames[i]; - } + argumentNames[i] = ParameterNames[i]; } } - - return argumentNames; } - public IList GetArgumentResolveResults(int skipCount = 0) - { - var expectedParameters = ExpectedParameters; - var useImplicitlyTypedOut = UseImplicitlyTypedOut; + return argumentNames; + } + + public IList GetArgumentResolveResults(int skipCount = 0) + { + var expectedParameters = ExpectedParameters; + var useImplicitlyTypedOut = UseImplicitlyTypedOut; - return Arguments - .SelectWithIndex(GetResolveResult) - .Skip(skipCount) - .Take(GetActualArgumentCount()) - .ToArray(); + return Arguments + .SelectWithIndex(GetResolveResult) + .Skip(skipCount) + .Take(GetActualArgumentCount()) + .ToArray(); - ResolveResult GetResolveResult(int index, TranslatedExpression expression) - { - var param = expectedParameters[index]; - if (useImplicitlyTypedOut && param.ReferenceKind == ReferenceKind.Out && expression.Type is ByReferenceType brt) - return new OutVarResolveResult(brt.ElementType); - return expression.ResolveResult; - } + ResolveResult GetResolveResult(int index, TranslatedExpression expression) + { + var param = expectedParameters[index]; + if (useImplicitlyTypedOut && param.ReferenceKind == ReferenceKind.Out && expression.Type is ByReferenceType brt) + return new OutVarResolveResult(brt.ElementType); + return expression.ResolveResult; } + } - public IList GetArgumentResolveResultsDirect(int skipCount = 0) + public IList GetArgumentResolveResultsDirect(int skipCount = 0) + { + return Arguments + .Skip(skipCount) + .Take(GetActualArgumentCount()) + .Select(a => a.ResolveResult) + .ToArray(); + } + + public IEnumerable GetArgumentExpressions(int skipCount = 0) + { + var argumentNames = GetArgumentNames(skipCount); + int argumentCount = GetActualArgumentCount(); + var useImplicitlyTypedOut = UseImplicitlyTypedOut; + if (argumentNames == null) { - return Arguments - .Skip(skipCount) - .Take(GetActualArgumentCount()) - .Select(a => a.ResolveResult) - .ToArray(); + return Arguments.Skip(skipCount).Take(argumentCount).Select(arg => AddAnnotations(arg.Expression)); } - - public IEnumerable GetArgumentExpressions(int skipCount = 0) + else { - var argumentNames = GetArgumentNames(skipCount); - int argumentCount = GetActualArgumentCount(); - var useImplicitlyTypedOut = UseImplicitlyTypedOut; - if (argumentNames == null) - { - return Arguments.Skip(skipCount).Take(argumentCount).Select(arg => AddAnnotations(arg.Expression)); - } - else - { - Debug.Assert(skipCount == 0); - return Arguments.Take(argumentCount).Zip(argumentNames.Take(argumentCount), - (arg, name) => { - if (name == null) - return AddAnnotations(arg.Expression); - else - return new NamedArgumentExpression(name, AddAnnotations(arg.Expression)); - }); - } + Debug.Assert(skipCount == 0); + return Arguments.Take(argumentCount).Zip(argumentNames.Take(argumentCount), + (arg, name) => { + if (name == null) + return AddAnnotations(arg.Expression); + else + return new NamedArgumentExpression(name, AddAnnotations(arg.Expression)); + }); + } - Expression AddAnnotations(Expression expression) - { - if (!useImplicitlyTypedOut) - return expression; - if (expression.GetResolveResult() is ByReferenceResolveResult { ReferenceKind: ReferenceKind.Out } brrr) - { - expression.AddAnnotation(UseImplicitlyTypedOutAnnotation.Instance); - } + Expression AddAnnotations(Expression expression) + { + if (!useImplicitlyTypedOut) return expression; + if (expression.GetResolveResult() is ByReferenceResolveResult { ReferenceKind: ReferenceKind.Out } brrr) + { + expression.AddAnnotation(UseImplicitlyTypedOutAnnotation.Instance); } + return expression; } + } - public bool CanInferAnonymousTypePropertyNamesFromArguments() + public bool CanInferAnonymousTypePropertyNamesFromArguments() + { + for (int i = 0; i < Arguments.Length; i++) { - for (int i = 0; i < Arguments.Length; i++) + string? inferredName; + switch (Arguments[i].Expression) { - string? inferredName; - switch (Arguments[i].Expression) - { - case IdentifierExpression identifier: - inferredName = identifier.Identifier; - break; - case MemberReferenceExpression member: - inferredName = member.MemberName; - break; - default: - inferredName = null; - break; - } + case IdentifierExpression identifier: + inferredName = identifier.Identifier; + break; + case MemberReferenceExpression member: + inferredName = member.MemberName; + break; + default: + inferredName = null; + break; + } - if (inferredName != ExpectedParameters[i].Name) - { - return false; - } + if (inferredName != ExpectedParameters[i].Name) + { + return false; } - return true; } + return true; + } - [Conditional("DEBUG")] - public void CheckNoNamedOrOptionalArguments() - { - Debug.Assert(ArgumentToParameterMap == null && ArgumentNames == null && FirstOptionalArgumentIndex < 0); - } + [Conditional("DEBUG")] + public void CheckNoNamedOrOptionalArguments() + { + Debug.Assert(ArgumentToParameterMap == null && ArgumentNames == null && FirstOptionalArgumentIndex < 0); } + } + + struct CallBuilder + { readonly DecompilerSettings settings; readonly ExpressionBuilder expressionBuilder; @@ -504,7 +505,7 @@ public ExpressionWithResolveResult Build(OpCode callOpCode, IMethod method, if (method.IsAccessor && (method.AccessorOwner.SymbolKind == SymbolKind.Indexer || argumentList.ExpectedParameters.Length == allowedParamCount)) { argumentList.CheckNoNamedOrOptionalArguments(); - return HandleAccessorCall(expectedTargetDetails, method, target, argumentList.Arguments.ToList(), argumentList.ArgumentNames); + return HandleAccessorCall(expectedTargetDetails, method, target, argumentList); } if (IsDelegateEqualityComparison(method, argumentList.Arguments)) @@ -751,8 +752,13 @@ public ExpressionWithResolveResult BuildCollectionInitializerExpression(OpCode c argumentList.ArgumentNames = null; argumentList.AddNamesToPrimitiveValues = false; argumentList.UseImplicitlyTypedOut = false; + // Collection initializer syntax has nowhere to put a target or type arguments, so + // casting the arguments is the only way left to pin the Add method down. var transform = GetRequiredTransformationsForCall(expectedTargetDetails, method, ref unused, - ref argumentList, ReferenceTransformation.None, out _); + ref argumentList, + ReferenceTransformation.CastArguments | ReferenceTransformation.NoOptionalArgumentAllowed + | ReferenceTransformation.NoNamedArgsForPrettiness, + out _); Debug.Assert((transform & ~(ReferenceTransformation.NoOptionalArgumentAllowed | ReferenceTransformation.NoNamedArgsForPrettiness)) == 0); // Calls with only one argument do not need an array initializer expression to wrap them. @@ -795,8 +801,7 @@ public ExpressionWithResolveResult BuildDictionaryInitializerExpression(OpCode c var argumentList = BuildArgumentList(expectedTargetDetails, target, method, 1, callArguments, null); var unused = new IdentifierExpression("initializedObject").WithRR(target).WithoutILInstruction(); - var assignment = HandleAccessorCall(expectedTargetDetails, method, unused, - argumentList.Arguments.ToList(), argumentList.ArgumentNames); + var assignment = HandleAccessorCall(expectedTargetDetails, method, unused, argumentList); if (((AssignmentExpression)assignment).Left is IndexerExpression indexer && indexer.Target is not null) indexer.Target.Remove(); @@ -1132,7 +1137,7 @@ private bool TransformParamsArgument(ExpectedTargetDetails expectedTargetDetails { expandedParameters.InsertRange(0, expectedParameters); expandedArguments.InsertRange(0, arguments); - if (IsUnambiguousCall(expectedTargetDetails, method, targetResolveResult, Empty.Array, + if (Disambiguator.IsUnambiguousCall(expressionBuilder, expectedTargetDetails, method, targetResolveResult, Empty.Array, expandedArguments.SelectArray(a => a.ResolveResult), argumentNames: null, firstOptionalArgumentIndex: -1, out _, out var bestCandidateIsExpandedForm) == OverloadResolutionErrors.None && bestCandidateIsExpandedForm) @@ -1228,21 +1233,6 @@ bool IsOptionalArgument(IParameter parameter, TranslatedExpression arg) return object.Equals(parameter.GetConstantValue(), arg.ResolveResult.ConstantValue); } - [Flags] - enum ReferenceTransformation - { - None = 0, - RequireTarget = 1, - RequireTypeArguments = 2, - NoOptionalArgumentAllowed = 4, - /// - /// Add calls to AsRefReadOnly for in parameters that did not have an explicit DirectionExpression yet. - /// - EnforceExplicitIn = 8, - NoNamedArgsForPrettiness = 0x10, - All = 0x1f, - } - private ReferenceTransformation GetRequiredTransformationsForCall(ExpectedTargetDetails expectedTargetDetails, IMethod method, ref TranslatedExpression target, ref ArgumentList argumentList, ReferenceTransformation allowedTransforms, out IParameterizedMember? foundMethod) { @@ -1250,7 +1240,6 @@ private ReferenceTransformation GetRequiredTransformationsForCall(ExpectedTarget // initialize requireTarget flag bool requireTarget; - ResolveResult? targetResolveResult; if ((allowedTransforms & ReferenceTransformation.RequireTarget) != 0) { if (settings.AlwaysQualifyMemberReferences || expressionBuilder.HidesVariableWithName(method.Name)) @@ -1270,159 +1259,24 @@ private ReferenceTransformation GetRequiredTransformationsForCall(ExpectedTarget else requireTarget = target.Expression is not ThisReferenceExpression; } - targetResolveResult = requireTarget ? target.ResolveResult : null; } else { // HACK: this is a special case for collection initializer calls, they do not allow a target to be // emitted, but we still need it for overload resolution. requireTarget = true; - targetResolveResult = target.ResolveResult; - } - - // initialize requireTypeArguments flag - bool requireTypeArguments; - IType[] typeArguments; - bool appliedRequireTypeArgumentsShortcut = false; - if (method.TypeParameters.Count > 0 && (allowedTransforms & ReferenceTransformation.RequireTypeArguments) != 0 - && !IsPossibleExtensionMethodCallOnNull(method, argumentList.Arguments)) - { - // The ambiguity resolution below only adds type arguments as last resort measure, however there are - // methods, such as Enumerable.OfType(IEnumerable input) that always require type arguments, - // as those cannot be inferred from the parameters, which leads to bloated expressions full of extra casts - // that are no longer required once we add the type arguments. - // We lend overload resolution a hand by detecting such cases beforehand and requiring type arguments, - // if necessary. - if (!CanInferTypeArgumentsFromArguments(method, argumentList, expressionBuilder.typeInference)) - { - if (settings.AnonymousTypes - && method.TypeArguments.Any(a => a.ContainsAnonymousType()) - && PinTypesOfNullArguments(argumentList) - && CanInferTypeArgumentsFromArguments(method, argumentList, expressionBuilder.typeInference)) - { - // Anonymous types cannot be written as explicit type arguments; instead the - // null arguments were rewritten so that all type arguments are inferable. - requireTypeArguments = false; - typeArguments = Empty.Array; - } - else - { - requireTypeArguments = true; - typeArguments = method.TypeArguments.ToArray(); - appliedRequireTypeArgumentsShortcut = true; - } - } - else - { - requireTypeArguments = false; - typeArguments = Empty.Array; - } - } - else - { - requireTypeArguments = false; - typeArguments = Empty.Array; } - bool targetCasted = false; - bool argumentsCasted = false; - bool originalRequireTarget = requireTarget; - bool skipTargetCast = method.Accessibility <= Accessibility.Protected && expressionBuilder.IsBaseTypeOfCurrentType(method.DeclaringTypeDefinition); - OverloadResolutionErrors errors; - while ((errors = IsUnambiguousCall(expectedTargetDetails, method, targetResolveResult, typeArguments, - argumentList.GetArgumentResolveResults().ToArray(), argumentList.GetArgumentNames(), argumentList.FirstOptionalArgumentIndex, out foundMethod, - out var bestCandidateIsExpandedForm)) != OverloadResolutionErrors.None || bestCandidateIsExpandedForm != argumentList.IsExpandedForm) - { - switch (errors) - { - case OverloadResolutionErrors.OutVarTypeMismatch: - Debug.Assert(argumentList.UseImplicitlyTypedOut); - argumentList.UseImplicitlyTypedOut = false; - continue; - case OverloadResolutionErrors.TypeInferenceFailed: - if ((allowedTransforms & ReferenceTransformation.RequireTypeArguments) != 0) - { - goto case OverloadResolutionErrors.WrongNumberOfTypeArguments; - } - goto default; - case OverloadResolutionErrors.WrongNumberOfTypeArguments: - Debug.Assert((allowedTransforms & ReferenceTransformation.RequireTypeArguments) != 0); - if (requireTypeArguments) - goto default; - requireTypeArguments = true; - typeArguments = method.TypeArguments.ToArray(); - continue; - case OverloadResolutionErrors.MissingArgumentForRequiredParameter: - if (argumentList.FirstOptionalArgumentIndex == -1) - goto default; - argumentList.FirstOptionalArgumentIndex = -1; - continue; - default: - // TODO : implement some more intelligent algorithm that decides which of these fixes (cast args, add target, cast target, add type args) - // is best in this case. Additionally we should not cast all arguments at once, but step-by-step try to add only a minimal number of casts. - if (argumentList.AddNamesToPrimitiveValues) - { - argumentList.AddNamesToPrimitiveValues = false; - } - else if (argumentList.FirstOptionalArgumentIndex >= 0) - { - argumentList.FirstOptionalArgumentIndex = -1; - } - else if (!argumentsCasted) - { - // If we added type arguments beforehand, but that didn't make the code any better, - // undo that decision and add casts first. - if (appliedRequireTypeArgumentsShortcut) - { - requireTypeArguments = false; - typeArguments = Empty.Array; - appliedRequireTypeArgumentsShortcut = false; - } - argumentsCasted = true; - argumentList.UseImplicitlyTypedOut = false; - CastArguments(argumentList.Arguments, argumentList.ExpectedParameters); - } - else if ((allowedTransforms & ReferenceTransformation.RequireTarget) != 0 && !requireTarget) - { - requireTarget = true; - targetResolveResult = target.ResolveResult; - } - else if ((allowedTransforms & ReferenceTransformation.RequireTarget) != 0 && !targetCasted) - { - if (skipTargetCast && requireTarget != originalRequireTarget) - { - requireTarget = originalRequireTarget; - if (!originalRequireTarget) - targetResolveResult = null; - allowedTransforms &= ~ReferenceTransformation.RequireTarget; - } - else - { - targetCasted = true; - target = target.ConvertTo(method.DeclaringType, expressionBuilder); - targetResolveResult = target.ResolveResult; - } - } - else if ((allowedTransforms & ReferenceTransformation.RequireTypeArguments) != 0 && !requireTypeArguments) - { - requireTypeArguments = true; - typeArguments = method.TypeArguments.ToArray(); - } - else if ((allowedTransforms & ReferenceTransformation.EnforceExplicitIn) != 0) - { - EnforceExplicitIn(argumentList.Arguments, argumentList.ExpectedParameters); - allowedTransforms &= ~ReferenceTransformation.EnforceExplicitIn; - } - else - { - break; - } - continue; - } - // We've given up. - foundMethod = method; - break; - } + var disambiguator = Disambiguator.ForCall(expressionBuilder, method, argumentList, target, + requireTarget, expectedTargetDetails, allowedTransforms); + // Where even the most explicit spelling stays ambiguous, the call is written as it + // stands and annotated with the method it was meant to reach. + foundMethod = disambiguator.Resolved ? (IParameterizedMember?)disambiguator.FoundMember : method; + target = disambiguator.Target; + argumentList = disambiguator.Arguments; + requireTarget = disambiguator.RequireTarget; + bool requireTypeArguments = disambiguator.RequireTypeArguments; + if ((allowedTransforms & ReferenceTransformation.RequireTarget) != 0 && requireTarget) transform |= ReferenceTransformation.RequireTarget; if ((allowedTransforms & ReferenceTransformation.RequireTypeArguments) != 0 && requireTypeArguments) @@ -1434,174 +1288,11 @@ private ReferenceTransformation GetRequiredTransformationsForCall(ExpectedTarget return transform; } - private void EnforceExplicitIn(TranslatedExpression[] arguments, IParameter[] expectedParameters) - { - for (int i = 0; i < arguments.Length; i++) - { - if (expectedParameters[i].ReferenceKind != ReferenceKind.In) - continue; - if (arguments[i].Expression is DirectionExpression) - continue; - - arguments[i] = WrapInAsRefReadOnly(arguments[i]); - expressionBuilder.statementBuilder.EmitAsRefReadOnly = true; - } - } - - private TranslatedExpression WrapInAsRefReadOnly(TranslatedExpression arg) - { - return new DirectionExpression( - FieldDirection.In, - new InvocationExpression { - Target = new IdentifierExpression("ILSpyHelper_AsRefReadOnly"), - Arguments = { arg.Expression } - } - ).WithRR(new ByReferenceResolveResult(arg.Type, ReferenceKind.In)) - .WithoutILInstruction(); - } - - private bool IsPossibleExtensionMethodCallOnNull(IMethod method, IList arguments) - { - return method.IsExtensionMethod && arguments.Count > 0 && arguments[0].Expression is NullReferenceExpression; - } - - static bool CanInferTypeArgumentsFromArguments(IMethod method, ArgumentList argumentList, TypeInference typeInference) - { - if (method.TypeParameters.Count == 0) - return true; - // always use unspecialized member, otherwise type inference fails - method = (IMethod)method.MemberDefinition; - IReadOnlyList paramTypesInArgumentOrder; - if (argumentList.ArgumentToParameterMap == null) - paramTypesInArgumentOrder = method.Parameters.SelectReadOnlyArray(p => p.Type); - else - paramTypesInArgumentOrder = argumentList.ArgumentToParameterMap - .SelectReadOnlyArray( - index => index >= 0 ? method.Parameters[index].Type : SpecialType.UnknownType - ); - typeInference.InferTypeArguments(method.TypeParameters, - argumentList.Arguments.SelectReadOnlyArray(a => a.ResolveResult), paramTypesInArgumentOrder, - out bool success); - return success; - } - - /// - /// C# has no syntax to spell out an anonymous type, so a null literal cannot be given such - /// a type with a cast. The minimal expression that produces a null value of an anonymous - /// type is a conditional expression whose never-taken branch creates an instance of the - /// type: true ? null : new { A = default(int) }. - /// Replaces null-literal arguments of an anonymous type with such an expression, so that - /// type arguments involving anonymous types (which cannot be written explicitly either) - /// become inferable from the arguments. - /// Returns true, if at least one argument was replaced. - /// - private bool PinTypesOfNullArguments(ArgumentList argumentList) - { - bool anyArgumentReplaced = false; - for (int i = 0; i < argumentList.Length; i++) - { - IType expectedType = argumentList.ExpectedParameters[i].Type; - if (argumentList.Arguments[i].Expression is not NullReferenceExpression) - continue; - if (!expectedType.IsAnonymousType() || NewAnonymousTypeInstance(expectedType) is not NewObj newObj) - continue; - var nullLiteral = argumentList.Arguments[i]; - argumentList.Arguments[i] = new ConditionalExpression(new PrimitiveExpression(true), - nullLiteral.Expression.Detach(), expressionBuilder.Translate(newObj, expectedType)) - .WithILInstruction(nullLiteral.ILInstructions) - .WithRR(new ResolveResult(expectedType)); - anyArgumentReplaced = true; - } - return anyArgumentReplaced; - } - - /// - /// Builds a 'newobj' instruction creating an instance of the anonymous type - /// with default property values; translating it yields - /// object-initializer syntax, the only way to name the type in source code. Returns null - /// if a property type involves an anonymous type other than by direct nesting (e.g. an - /// array of anonymous type), because its default value expression would have to name it. - /// - private NewObj? NewAnonymousTypeInstance(IType type) - { - var newObj = new NewObj(type.GetConstructors().Single()); - foreach (var parameter in newObj.Method.Parameters) - { - ILInstruction? argument = parameter.Type.IsAnonymousType() - ? NewAnonymousTypeInstance(parameter.Type) - : parameter.Type.ContainsAnonymousType() ? null : new DefaultValue(parameter.Type); - if (argument == null) - return null; - newObj.Arguments.Add(argument); - } - return newObj; - } - - private void CastArguments(IList arguments, IList expectedParameters) - { - for (int i = 0; i < arguments.Count; i++) - { - if (settings.AnonymousTypes && expectedParameters[i].Type.ContainsAnonymousType()) - { - if (arguments[i].Expression is LambdaExpression lambda) - { - ModifyReturnTypeOfLambda(lambda); - } - } - else - { - IParameter parameter = expectedParameters[i]; - IType parameterType; - if (parameter.Type.Kind == TypeKind.Dynamic) - { - parameterType = expressionBuilder.compilation.FindType(KnownTypeCode.Object); - } - else - { - parameterType = parameter.Type; - } - - if (parameter.ReferenceKind == ReferenceKind.In && parameterType is ByReferenceType brt && arguments[i].Type is not ByReferenceType) - { - parameterType = brt.ElementType; - } - - arguments[i] = arguments[i].ConvertTo(parameterType, expressionBuilder, allowImplicitConversion: false); - } - } - } - static bool IsNullConditional(Expression expr) { return expr is UnaryOperatorExpression uoe && uoe.Operator == UnaryOperatorType.NullConditional; } - private void ModifyReturnTypeOfLambda(LambdaExpression lambda) - { - var resolveResult = (DecompiledLambdaResolveResult)lambda.GetResolveResult(); - if (lambda.Body is Expression exprBody) - lambda.Body = new TranslatedExpression(exprBody.Detach()).ConvertTo(resolveResult.ReturnType, expressionBuilder); - else - ModifyReturnStatementInsideLambda(resolveResult.ReturnType, lambda); - resolveResult.InferredReturnType = resolveResult.ReturnType; - } - - private void ModifyReturnStatementInsideLambda(IType returnType, AstNode parent) - { - foreach (var child in parent.Children) - { - if (child is LambdaExpression || child is AnonymousMethodExpression) - continue; - if (child is ReturnStatement ret) - { - if (ret.Expression is not null) - ret.Expression = new TranslatedExpression(ret.Expression.Detach()).ConvertTo(returnType, expressionBuilder); - continue; - } - ModifyReturnStatementInsideLambda(returnType, child); - } - } - private bool IsDelegateEqualityComparison(IMethod method, IList arguments) { // Comparison on a delegate type is a C# builtin operator @@ -1653,173 +1344,8 @@ private ExpressionWithResolveResult HandleImplicitConversion(IMethod method, Tra .WithRR(new ConversionResolveResult(targetType, argument.ResolveResult, conv)); } - OverloadResolutionErrors IsUnambiguousCall(ExpectedTargetDetails expectedTargetDetails, IMethod method, - ResolveResult? target, IType[] typeArguments, ResolveResult[] arguments, - string[]? argumentNames, int firstOptionalArgumentIndex, - out IParameterizedMember? foundMember, out bool bestCandidateIsExpandedForm) - { - foundMember = null; - bestCandidateIsExpandedForm = false; - var currentTypeDefinition = resolver.CurrentTypeDefinition; - var lookup = new MemberLookup(currentTypeDefinition, currentTypeDefinition.ParentModule); - - Log.WriteLine("IsUnambiguousCall: Performing overload resolution for " + method); - Log.WriteCollection(" Arguments: ", arguments); - - argumentNames = firstOptionalArgumentIndex < 0 || argumentNames == null - ? argumentNames - : argumentNames.Take(firstOptionalArgumentIndex).ToArray(); - - var or = new OverloadResolution(resolver.Compilation, - arguments, argumentNames, typeArguments, - conversions: expressionBuilder.resolver.conversions); - if (expectedTargetDetails.CallOpCode == OpCode.NewObj) - { - foreach (IMethod ctor in method.DeclaringType.GetConstructors()) - { - bool allowProtectedAccess = - resolver.CurrentTypeDefinition == method.DeclaringTypeDefinition; - if (lookup.IsAccessible(ctor, allowProtectedAccess)) - { - Log.Indent(); - OverloadResolutionErrors errors = or.AddCandidate(ctor); - Log.Unindent(); - or.LogCandidateAddingResult(" Candidate", ctor, errors); - } - } - } - else if (method.IsOperator) - { - IEnumerable operatorCandidates; - if (arguments.Length == 1) - { - IType argType = NullableType.GetUnderlyingType(arguments[0].Type); - operatorCandidates = resolver.GetUserDefinedOperatorCandidates(argType, method.Name); - if (method.Name == "op_Explicit") - { - // For casts, also consider candidates from the target type we are casting to. - var hashSet = new HashSet(operatorCandidates); - IType targetType = NullableType.GetUnderlyingType(method.ReturnType); - hashSet.UnionWith( - resolver.GetUserDefinedOperatorCandidates(targetType, method.Name) - ); - operatorCandidates = hashSet; - } - } - else if (arguments.Length == 2) - { - IType lhsType = NullableType.GetUnderlyingType(arguments[0].Type); - IType rhsType = NullableType.GetUnderlyingType(arguments[1].Type); - var hashSet = new HashSet(); - hashSet.UnionWith(resolver.GetUserDefinedOperatorCandidates(lhsType, method.Name)); - hashSet.UnionWith(resolver.GetUserDefinedOperatorCandidates(rhsType, method.Name)); - operatorCandidates = hashSet; - } - else - { - operatorCandidates = EmptyList.Instance; - } - foreach (var m in operatorCandidates) - { - or.AddCandidate(m); - } - } - else if (target == null) - { - var result = resolver.ResolveSimpleName(method.Name, typeArguments, isInvocationTarget: true) - as MethodGroupResolveResult; - if (result == null) - return OverloadResolutionErrors.AmbiguousMatch; - or.AddMethodLists(result.MethodsGroupedByDeclaringType.ToArray()); - } - else - { - var result = lookup.Lookup(target, method.Name, typeArguments, isInvocation: true) as MethodGroupResolveResult; - if (result == null) - return OverloadResolutionErrors.AmbiguousMatch; - or.AddMethodLists(result.MethodsGroupedByDeclaringType.ToArray()); - } - bestCandidateIsExpandedForm = or.BestCandidateIsExpandedForm; - if (or.BestCandidateErrors != OverloadResolutionErrors.None) - return or.BestCandidateErrors; - if (or.IsAmbiguous) - return OverloadResolutionErrors.AmbiguousMatch; - foundMember = or.GetBestCandidateWithSubstitutedTypeArguments(); - if (foundMember == null) - { - // Overload resolution reports no error for an empty candidate set - there is no - // best candidate to carry one - so a call that matched nothing has to be reported - // as unresolvable here. - return OverloadResolutionErrors.AmbiguousMatch; - } - if (!IsAppropriateCallTarget(expectedTargetDetails, method, foundMember)) - return OverloadResolutionErrors.AmbiguousMatch; - var map = or.GetArgumentToParameterMap(); - for (int i = 0; i < arguments.Length; i++) - { - ResolveResult arg = arguments[i]; - int parameterIndex = map[i]; - if (arg is OutVarResolveResult rr && parameterIndex >= 0) - { - var param = foundMember.Parameters[parameterIndex]; - var paramType = param.Type.UnwrapByRef(); - if (!paramType.Equals(rr.OriginalVariableType)) - return OverloadResolutionErrors.OutVarTypeMismatch; - } - } - - return OverloadResolutionErrors.None; - } - - bool IsUnambiguousAccess(ExpectedTargetDetails expectedTargetDetails, ResolveResult? target, IMethod method, - IList arguments, string[]? argumentNames, [NotNullWhen(true)] out IMember? foundMember) - { - Log.WriteLine("IsUnambiguousAccess: Performing overload resolution for " + method); - Log.WriteCollection(" Arguments: ", arguments.Select(a => a.ResolveResult)); - - foundMember = null; - if (target == null) - { - var result = resolver.ResolveSimpleName(method.AccessorOwner!.Name, - EmptyList.Instance, - isInvocationTarget: false) as MemberResolveResult; - if (result == null || result.IsError) - return false; - foundMember = result.Member; - } - else - { - var lookup = new MemberLookup(resolver.CurrentTypeDefinition, resolver.CurrentTypeDefinition.ParentModule); - if (method.AccessorOwner!.SymbolKind == SymbolKind.Indexer) - { - var or = new OverloadResolution(resolver.Compilation, - arguments.SelectArray(a => a.ResolveResult), - argumentNames: argumentNames, - typeArguments: Empty.Array, - conversions: expressionBuilder.resolver.conversions); - or.AddMethodLists(lookup.LookupIndexers(target)); - if (or.BestCandidateErrors != OverloadResolutionErrors.None) - return false; - if (or.IsAmbiguous) - return false; - foundMember = or.GetBestCandidateWithSubstitutedTypeArguments(); - } - else - { - var result = lookup.Lookup(target, - method.AccessorOwner!.Name, - EmptyList.Instance, - isInvocation: false) as MemberResolveResult; - if (result == null || result.IsError) - return false; - foundMember = result.Member; - } - } - return foundMember != null && IsAppropriateCallTarget(expectedTargetDetails, method.AccessorOwner, foundMember); - } - ExpressionWithResolveResult HandleAccessorCall(ExpectedTargetDetails expectedTargetDetails, IMethod method, - TranslatedExpression target, List arguments, string[]? argumentNames) + TranslatedExpression target, ArgumentList argumentList) { bool requireTarget; if (settings.AlwaysQualifyMemberReferences || method.AccessorOwner!.SymbolKind == SymbolKind.Indexer || expressionBuilder.HidesVariableWithName(method.AccessorOwner.Name)) @@ -1828,43 +1354,27 @@ ExpressionWithResolveResult HandleAccessorCall(ExpectedTargetDetails expectedTar requireTarget = !expressionBuilder.IsCurrentOrContainingType(method.DeclaringTypeDefinition); else requireTarget = !(target.Expression is ThisReferenceExpression); - bool targetCasted = false; bool isSetter = method.ReturnType.IsKnownType(KnownTypeCode.Void); - bool argumentsCasted = (isSetter && method.Parameters.Count == 1) || (!isSetter && method.Parameters.Count == 0); - var targetResolveResult = requireTarget ? target.ResolveResult : null; TranslatedExpression value = default(TranslatedExpression); + var arguments = argumentList.Arguments; if (isSetter) { - value = arguments.Last(); - arguments.Remove(value); + // The assigned value is not part of the reference being spelled out. + value = arguments[arguments.Length - 1]; + arguments = arguments.Take(arguments.Length - 1).ToArray(); } + // The accessor's own parameters describe the indices, which is what the casts and the + // ambiguity check need; the parameters the call was built with describe the call. + argumentList.Arguments = arguments; + argumentList.ExpectedParameters = method.Parameters.ToArray(); - IMember? foundMember; - while (!IsUnambiguousAccess(expectedTargetDetails, targetResolveResult, method, arguments, argumentNames, out foundMember)) - { - if (!argumentsCasted) - { - argumentsCasted = true; - CastArguments(arguments, method.Parameters.ToList()); - } - else if (!requireTarget) - { - requireTarget = true; - targetResolveResult = target.ResolveResult; - } - else if (!targetCasted) - { - targetCasted = true; - target = target.ConvertTo(method.AccessorOwner!.DeclaringType, expressionBuilder); - targetResolveResult = target.ResolveResult; - } - else - { - foundMember = method.AccessorOwner!; - break; - } - } + var disambiguator = Disambiguator.ForAccessor(expressionBuilder, method, argumentList, target, + requireTarget, expectedTargetDetails); + IMember? foundMember = disambiguator.Resolved ? disambiguator.FoundMember : method.AccessorOwner!; + requireTarget = disambiguator.RequireTarget; + target = disambiguator.Target; + arguments = disambiguator.Arguments.Arguments; var rr = new MemberResolveResult(target.ResolveResult, foundMember); @@ -1872,7 +1382,7 @@ ExpressionWithResolveResult HandleAccessorCall(ExpectedTargetDetails expectedTar { TranslatedExpression expr; - if (arguments.Count != 0) + if (arguments.Length != 0) { expr = new IndexerExpression(target.ResolveResult is InitializedObjectResolveResult ? null : target.Expression, arguments.Select(a => a.Expression)) .WithoutILInstruction().WithRR(rr); @@ -1904,7 +1414,7 @@ ExpressionWithResolveResult HandleAccessorCall(ExpectedTargetDetails expectedTar } else { - if (arguments.Count != 0) + if (arguments.Length != 0) { return new IndexerExpression(target.Expression, arguments.Select(a => a.Expression)) .WithoutILInstruction().WithRR(rr); @@ -1922,42 +1432,6 @@ ExpressionWithResolveResult HandleAccessorCall(ExpectedTargetDetails expectedTar } } - bool IsAppropriateCallTarget(ExpectedTargetDetails expectedTargetDetails, IMember expectedTarget, IMember actualTarget) - { - if (expectedTarget.Equals(actualTarget, NormalizeTypeVisitor.TypeErasure)) - return true; - - if (expectedTargetDetails.CallOpCode == OpCode.CallVirt && actualTarget.IsOverride) - { - if (expectedTargetDetails.NeedsBoxingConversion && actualTarget.DeclaringType.IsReferenceType != true) - return false; - foreach (var possibleTarget in InheritanceHelper.GetBaseMembers(actualTarget, false)) - { - if (expectedTarget.Equals(possibleTarget, NormalizeTypeVisitor.TypeErasure)) - return true; - if (!possibleTarget.IsOverride) - break; - } - } - return false; - } - - - /// - /// Checks whether calling `target.methodName()` will use `expected` as the method to invoke. - /// - public bool CheckSimpleCall(ResolveResult target, IMethod expected, OpCode expectedCallOpCode = OpCode.Call) - { - var details = new ExpectedTargetDetails { CallOpCode = expectedCallOpCode, NeedsBoxingConversion = false }; - if (resolver.ResolveMemberAccess(target, expected.Name, [], NameLookupMode.InvocationTarget) - is not MethodGroupResolveResult mgrr) - return false; - var or = mgrr.PerformOverloadResolution(typeSystem, []); - if (or.BestCandidateErrors != OverloadResolutionErrors.None || or.IsAmbiguous) - return false; - return IsAppropriateCallTarget(details, expected, or.GetBestCandidateWithSubstitutedTypeArguments()!); - } - ExpressionWithResolveResult HandleConstructorCall(ExpectedTargetDetails expectedTargetDetails, ResolveResult? target, IMethod method, ArgumentList argumentList) { if (settings.AnonymousTypes && method.DeclaringType.IsAnonymousType()) @@ -1986,24 +1460,9 @@ ExpressionWithResolveResult HandleConstructorCall(ExpectedTargetDetails expected } else { - while (IsUnambiguousCall(expectedTargetDetails, method, null, Empty.Array, - argumentList.GetArgumentResolveResults().ToArray(), - argumentList.GetArgumentNames(), argumentList.FirstOptionalArgumentIndex, out _, - out var bestCandidateIsExpandedForm) != OverloadResolutionErrors.None || bestCandidateIsExpandedForm != argumentList.IsExpandedForm) - { - if (argumentList.AddNamesToPrimitiveValues) - { - argumentList.AddNamesToPrimitiveValues = false; - continue; - } - if (argumentList.FirstOptionalArgumentIndex >= 0) - { - argumentList.FirstOptionalArgumentIndex = -1; - continue; - } - CastArguments(argumentList.Arguments, argumentList.ExpectedParameters); - break; // make sure that we don't not end up in an infinite loop - } + var disambiguator = Disambiguator.ForConstructorCall(expressionBuilder, method, argumentList, + expectedTargetDetails); + argumentList = disambiguator.Arguments; IType? returnTypeOverride = null; if (typeSystem.MainModule.TypeSystemOptions.HasFlag(TypeSystemOptions.NativeIntegersWithoutAttribute)) { @@ -2154,40 +1613,11 @@ ExpressionWithResolveResult BuildDelegateReference(IMethod method, IMethod? invo thisArg = thisArgBox.Argument; } TranslatedExpression target = expressionBuilder.Translate(thisArg!, targetType); - var currentTarget = target; - bool targetCasted = false; - bool addTypeArguments = false; - // Initial inputs for IsUnambiguousMethodReference: - ResolveResult targetResolveResult = target.ResolveResult; - IReadOnlyList typeArguments = EmptyList.Instance; - if (thisArg!.MatchLdNull()) - { - targetCasted = true; - currentTarget = currentTarget.ConvertTo(targetType, expressionBuilder); - targetResolveResult = currentTarget.ResolveResult; - } - // Find somewhat minimal solution: - ResolveResult? result; - while (!IsUnambiguousMethodReference(expectedTargetDetails, method, targetResolveResult, typeArguments, true, out result)) - { - if (!targetCasted) - { - // try casting target - targetCasted = true; - currentTarget = currentTarget.ConvertTo(targetType, expressionBuilder); - targetResolveResult = currentTarget.ResolveResult; - continue; - } - if (!addTypeArguments) - { - // try adding type arguments - addTypeArguments = true; - typeArguments = method.TypeArguments; - continue; - } - break; - } - return (currentTarget, addTypeArguments, method.Name, result!); + // A null literal carries no type at all, so its cast cannot wait its turn. + var disambiguator = Disambiguator.ForMethodReference(expressionBuilder, method, targetType, + target, requireTarget: true, expectedTargetDetails, isExtensionMethodReference: true, + castTargetUpFront: thisArg!.MatchLdNull()); + return (disambiguator.Target, disambiguator.RequireTypeArguments, method.Name, disambiguator.Result!); } else { @@ -2213,50 +1643,17 @@ ExpressionWithResolveResult BuildDelegateReference(IMethod method, IMethod? invo // check if target is required bool requireTarget = expressionBuilder.HidesVariableWithName(method.Name) || (method.IsStatic ? !expressionBuilder.IsCurrentOrContainingType(method.DeclaringTypeDefinition) : !(target.Expression is ThisReferenceExpression)); - // Try to find minimal expression - // If target is required, include it from the start - bool targetAdded = requireTarget; - TranslatedExpression currentTarget = targetAdded ? target : default; - // Remember other decisions: - bool targetCasted = false; - bool addTypeArguments = false; - // Initial inputs for IsUnambiguousMethodReference: - ResolveResult? targetResolveResult = targetAdded ? target.ResolveResult : null; - IReadOnlyList typeArguments = EmptyList.Instance; - // Find somewhat minimal solution: - ResolveResult? result; - while (!IsUnambiguousMethodReference(expectedTargetDetails, method, targetResolveResult, typeArguments, false, out result)) - { - if (!addTypeArguments) - { - // try adding type arguments - addTypeArguments = true; - typeArguments = method.TypeArguments; - continue; - } - if (!targetAdded) - { - // try adding target - targetAdded = true; - currentTarget = target; - targetResolveResult = target.ResolveResult; - continue; - } - if (!targetCasted) - { - // try casting target - targetCasted = true; - currentTarget = currentTarget.ConvertTo(targetType, expressionBuilder); - targetResolveResult = currentTarget.ResolveResult; - continue; - } - break; - } + var disambiguator = Disambiguator.ForMethodReference(expressionBuilder, method, targetType, + target, requireTarget, expectedTargetDetails, isExtensionMethodReference: false); + ResolveResult? result = disambiguator.Result; if (result is MethodGroupResolveResult mgrr) { result = mgrr.WithChosenMethod(method); } - return (currentTarget, addTypeArguments, method.Name, result!); + // BuildDelegateReference tells a qualified reference from an unqualified one by + // whether it got a target expression at all. + return (disambiguator.RequireTarget ? disambiguator.Target : default, + disambiguator.RequireTypeArguments, method.Name, result!); } } @@ -2273,54 +1670,6 @@ TranslatedExpression HandleDelegateConstruction(IType delegateType, IMethod meth return oce; } - bool IsUnambiguousMethodReference(ExpectedTargetDetails expectedTargetDetails, IMethod method, ResolveResult? target, IReadOnlyList typeArguments, bool isExtensionMethodReference, [NotNullWhen(true)] out ResolveResult? result) - { - Log.WriteLine("IsUnambiguousMethodReference: Performing overload resolution for " + method); - - var lookup = new MemberLookup(resolver.CurrentTypeDefinition, resolver.CurrentTypeDefinition.ParentModule); - OverloadResolution or; - - if (isExtensionMethodReference) - { - result = resolver.ResolveMemberAccess(target, method.Name, typeArguments, NameLookupMode.InvocationTarget) as MethodGroupResolveResult; - if (result == null) - return false; - or = ((MethodGroupResolveResult)result).PerformOverloadResolution(resolver.CurrentTypeResolveContext.Compilation, - method.Parameters.SelectReadOnlyArray(p => new TypeResolveResult(p.Type)), - argumentNames: null, allowExtensionMethods: true); - if (or == null || or.IsAmbiguous) - return false; - } - else - { - or = new OverloadResolution(resolver.Compilation, - arguments: method.Parameters.SelectReadOnlyArray(p => new TypeResolveResult(p.Type)), // there are no arguments, use parameter types - argumentNames: null, // argument names are not possible - typeArguments.ToArray(), - conversions: expressionBuilder.resolver.conversions - ); - if (target == null) - { - result = resolver.ResolveSimpleName(method.Name, typeArguments, isInvocationTarget: false); - if (!(result is MethodGroupResolveResult mgrr)) - return false; - or.AddMethodLists(mgrr.MethodsGroupedByDeclaringType.ToArray()); - } - else - { - result = lookup.Lookup(target, method.Name, typeArguments, isInvocation: false); - if (!(result is MethodGroupResolveResult mgrr)) - return false; - or.AddMethodLists(mgrr.MethodsGroupedByDeclaringType.ToArray()); - } - } - - var foundMethod = or.GetBestCandidateWithSubstitutedTypeArguments(); - if (!IsAppropriateCallTarget(expectedTargetDetails, method, foundMethod)) - return false; - return result is MethodGroupResolveResult; - } - static MethodGroupResolveResult ToMethodGroup(IMethod method, ILFunction localFunction) { return new MethodGroupResolveResult( diff --git a/ICSharpCode.Decompiler/CSharp/Disambiguator.cs b/ICSharpCode.Decompiler/CSharp/Disambiguator.cs new file mode 100644 index 00000000000..6ca7dff76cc --- /dev/null +++ b/ICSharpCode.Decompiler/CSharp/Disambiguator.cs @@ -0,0 +1,995 @@ +// Copyright (c) 2026 Siegfried Pammer +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using System.Linq; + +using ICSharpCode.Decompiler.CSharp.Resolver; +using ICSharpCode.Decompiler.CSharp.Syntax; +using ICSharpCode.Decompiler.IL; +using ICSharpCode.Decompiler.Semantics; +using ICSharpCode.Decompiler.TypeSystem; +using ICSharpCode.Decompiler.Util; + +#nullable enable + +namespace ICSharpCode.Decompiler.CSharp +{ + /// + /// A way of making a member reference more explicit, so that it resolves back to the + /// member the IL referenced instead of to something the shorter spelling happens to hit. + /// + [Flags] + internal enum ReferenceTransformation + { + None = 0, + RequireTarget = 1, + RequireTypeArguments = 2, + NoOptionalArgumentAllowed = 4, + /// + /// Add calls to AsRefReadOnly for in parameters that did not have an explicit DirectionExpression yet. + /// + EnforceExplicitIn = 8, + NoNamedArgsForPrettiness = 0x10, + /// Cast every argument to its parameter type. + CastArguments = 0x20, + /// Cast the target to the declaring type. + CastTarget = 0x40, + All = 0x7f, + } + + /// + /// Begins escalating a field access. A field is only ever qualified and then cast; it has + /// no arguments and no type arguments to make explicit. + /// + + /// + /// Finds the shortest spelling of a member reference that still resolves back to the member + /// the IL referenced. A short spelling may bind to something else in the output's + /// name-lookup context, so it is re-resolved and, while it does not bind back, made more + /// explicit and tried again. Which check re-resolves it, and which escalations are open to it + /// in what order, are both settled when it is built. + /// + /// One of the For* factories builds it, escalates, and hands it back finished; the results + /// are the fields below, not a return value. + /// + /// In: the member, its declaring type, and how much room the surrounding syntax leaves for + /// escalations. + /// + /// In and out: , and + /// are seeded with the shortest spelling and escalated in place. + /// + /// Out: and say whether the + /// target and the type arguments are written at all; and + /// are what the last check resolved to. is + /// false when no spelling resolved, leaving the most explicit one tried for the caller to + /// emit anyway. + /// + internal struct Disambiguator + { + + static readonly ReferenceTransformation[] FieldSteps = { + ReferenceTransformation.RequireTarget, + ReferenceTransformation.CastTarget, + }; + + /// A field access: only ever qualified and then cast. + internal static Disambiguator ForField(ExpressionBuilder expressionBuilder, IField field, + TranslatedExpression target, bool requireTarget) + { + var disambiguator = new Disambiguator(expressionBuilder, field, field.DeclaringType, target, + requireTarget, FieldSteps, ReferenceTransformation.All); + disambiguator.Resolved = disambiguator.Run(); + return disambiguator; + } + + static readonly ReferenceTransformation[] MethodReferenceSteps = { + ReferenceTransformation.RequireTypeArguments, + ReferenceTransformation.RequireTarget, + ReferenceTransformation.CastTarget, + }; + + /// + /// An extension method reference always carries its target, so the only escalations + /// left are making that target's type and then the type arguments explicit. + /// + static readonly ReferenceTransformation[] ExtensionMethodReferenceSteps = { + ReferenceTransformation.CastTarget, + ReferenceTransformation.RequireTypeArguments, + }; + + /// + /// A method group. It has no arguments to cast, but unlike a field it can spell out its + /// type arguments. is for a target that carries no + /// type of its own, such as a null literal, where the cast cannot wait its turn. + /// + internal static Disambiguator ForMethodReference(ExpressionBuilder expressionBuilder, IMethod method, + IType targetType, TranslatedExpression target, bool requireTarget, + ExpectedTargetDetails expectedTargetDetails, bool isExtensionMethodReference, + bool castTargetUpFront = false) + { + var disambiguator = new Disambiguator(expressionBuilder, method, targetType, target, requireTarget, + isExtensionMethodReference ? ExtensionMethodReferenceSteps : MethodReferenceSteps, + ReferenceTransformation.All, expectedTargetDetails, + isExtensionMethodReference: isExtensionMethodReference); + if (castTargetUpFront) + { + disambiguator.ApplyUpFront(ReferenceTransformation.CastTarget); + } + disambiguator.Resolved = disambiguator.Run(); + return disambiguator; + } + + static readonly ReferenceTransformation[] CallSteps = { + ReferenceTransformation.NoNamedArgsForPrettiness, + ReferenceTransformation.NoOptionalArgumentAllowed, + ReferenceTransformation.CastArguments, + ReferenceTransformation.RequireTarget, + ReferenceTransformation.CastTarget, + ReferenceTransformation.RequireTypeArguments, + ReferenceTransformation.EnforceExplicitIn, + }; + + /// + /// An ordinary method call - the one reference that can reach for every escalation there + /// is. withholds the ones the syntax around the call has no + /// room for. + /// + internal static Disambiguator ForCall(ExpressionBuilder expressionBuilder, IMethod method, + ArgumentList argumentList, TranslatedExpression target, bool requireTarget, + ExpectedTargetDetails expectedTargetDetails, ReferenceTransformation allowed) + { + var disambiguator = new Disambiguator(expressionBuilder, method, method.DeclaringType, target, + requireTarget, CallSteps, allowed, expectedTargetDetails, invocationSyntax: true, + castingArgumentsDropsImplicitlyTypedOut: true, + skipTargetCast: method.Accessibility <= Accessibility.Protected + && expressionBuilder.IsBaseTypeOfCurrentType(method.DeclaringTypeDefinition)) { + Arguments = argumentList, + }; + disambiguator.WriteTypeArgumentsInferenceCannotReach(); + disambiguator.Resolved = disambiguator.Run(); + return disambiguator; + } + + static readonly ReferenceTransformation[] ConstructorCallSteps = { + ReferenceTransformation.NoNamedArgsForPrettiness, + ReferenceTransformation.NoOptionalArgumentAllowed, + ReferenceTransformation.CastArguments, + }; + + /// + /// A constructor call. There is no target to qualify or cast and the type arguments come + /// from the type being constructed, so only the arguments are left. + /// + internal static Disambiguator ForConstructorCall(ExpressionBuilder expressionBuilder, IMethod method, + ArgumentList argumentList, ExpectedTargetDetails expectedTargetDetails) + { + var disambiguator = new Disambiguator(expressionBuilder, method, method.DeclaringType, default, + requireTarget: false, ConstructorCallSteps, ReferenceTransformation.All, + expectedTargetDetails, invocationSyntax: true) { + Arguments = argumentList, + }; + disambiguator.Resolved = disambiguator.Run(); + return disambiguator; + } + + static readonly ReferenceTransformation[] AccessorSteps = { + ReferenceTransformation.CastArguments, + ReferenceTransformation.RequireTarget, + ReferenceTransformation.CastTarget, + }; + + /// + /// A property, indexer or event access. holds the indices, + /// with the parameters the accessor declares: a setter's value is not part of the + /// reference and must already have been split off. + /// + internal static Disambiguator ForAccessor(ExpressionBuilder expressionBuilder, IMethod accessor, + ArgumentList argumentList, TranslatedExpression target, bool requireTarget, + ExpectedTargetDetails expectedTargetDetails) + { + var disambiguator = new Disambiguator(expressionBuilder, accessor, + accessor.AccessorOwner!.DeclaringType, target, requireTarget, AccessorSteps, + ReferenceTransformation.All, expectedTargetDetails) { + Arguments = argumentList, + }; + if (argumentList.Length == 0) + { + // Nothing to cast: a property access, or an indexer assignment whose value has + // already been split off. + disambiguator.MarkApplied(ReferenceTransformation.CastArguments); + } + disambiguator.Resolved = disambiguator.Run(); + return disambiguator; + } + + // In. Fixed for the life of the escalation. + + readonly ExpressionBuilder expressionBuilder; + readonly IMember member; + readonly IType declaringType; + /// The escalations open to this reference, in the order it prefers them. + readonly ReferenceTransformation[] steps; + /// + /// Whether the reference is written with invocation syntax. Nothing on an IMethod says + /// that: a method group is not, a property access is not even though it is a call in IL, + /// and a parameterized property's accessor is, despite being an accessor. + /// + readonly bool invocationSyntax; + readonly bool isExtensionMethodReference; + /// The escalations this caller has room for; the rest are never offered. + readonly ReferenceTransformation allowed; + readonly ExpectedTargetDetails expectedTargetDetails; + /// + /// Whether casting the arguments also means giving up "out var". It does for a call, + /// whose out arguments are written from the argument list; a constructor call keeps + /// the sugar, and an accessor has no out parameters to write. + /// + readonly bool castingArgumentsDropsImplicitlyTypedOut; + /// + /// Whether a protected member declared in a base type lets the reference drop its + /// qualifier again rather than cast it, which would not compile. + /// + readonly bool skipTargetCast; + /// Whether the reference was qualified before any escalation. + readonly bool initiallyRequiredTarget; + + // In and out. The spelling: seeded short, escalated in place, read back after Run. + + public TranslatedExpression Target; + /// The type arguments to write out; empty while they stay inferred. + public IType[] TypeArguments; + /// The arguments, and the decisions about how to write them. + public ArgumentList Arguments; + /// The escalations that are currently in effect. + ReferenceTransformation Applied; + /// + /// The escalations already offered. Normally the same as , but an + /// escalation that was tried and then taken back again stays spent, so it is not + /// offered again. + /// + ReferenceTransformation spent; + /// + /// Whether the type arguments were written up front because they cannot be inferred. + /// Casting the arguments takes that guess back, so that casts are tried first. + /// + bool TypeArgumentsWereAppliedUpFront; + + // Out. Read back after Run. + + /// Whether the reference is written with its target. + public bool RequireTarget => (Applied & ReferenceTransformation.RequireTarget) != 0; + /// Whether the type arguments are written out rather than left to inference. + public bool RequireTypeArguments => (Applied & ReferenceTransformation.RequireTypeArguments) != 0; + /// What the last check resolved to. + public ResolveResult? Result; + /// The member the last check bound to. + public IMember? FoundMember; + /// Whether a spelling was found that resolves back to the member. + public bool Resolved; + + Disambiguator(ExpressionBuilder expressionBuilder, IMember member, IType declaringType, + TranslatedExpression target, bool requireTarget, + ReferenceTransformation[] steps, ReferenceTransformation allowed, + ExpectedTargetDetails expectedTargetDetails = default, bool invocationSyntax = false, + bool isExtensionMethodReference = false, + bool castingArgumentsDropsImplicitlyTypedOut = false, bool skipTargetCast = false) + { + this.expressionBuilder = expressionBuilder; + this.member = member; + this.declaringType = declaringType; + this.allowed = allowed; + this.Target = target; + this.Applied = requireTarget ? ReferenceTransformation.RequireTarget : ReferenceTransformation.None; + this.spent = this.Applied; + this.TypeArguments = Empty.Array; + this.Result = null; + this.expectedTargetDetails = expectedTargetDetails; + this.steps = steps; + this.invocationSyntax = invocationSyntax; + this.isExtensionMethodReference = isExtensionMethodReference; + this.castingArgumentsDropsImplicitlyTypedOut = castingArgumentsDropsImplicitlyTypedOut; + this.skipTargetCast = skipTargetCast; + this.initiallyRequiredTarget = requireTarget; + this.TypeArgumentsWereAppliedUpFront = false; + this.Arguments = default; + this.FoundMember = null; + this.Resolved = false; + } + + CSharpResolver resolver => expressionBuilder.resolver; + + static bool IsPossibleExtensionMethodCallOnNull(IMethod method, IList arguments) + { + return method.IsExtensionMethod && arguments.Count > 0 && arguments[0].Expression is NullReferenceExpression; + } + + bool CanInferTypeArgumentsFromArguments(IMethod method) + { + if (method.TypeParameters.Count == 0) + return true; + // always use unspecialized member, otherwise type inference fails + method = (IMethod)method.MemberDefinition; + IReadOnlyList paramTypesInArgumentOrder; + if (Arguments.ArgumentToParameterMap == null) + paramTypesInArgumentOrder = method.Parameters.SelectReadOnlyArray(p => p.Type); + else + paramTypesInArgumentOrder = Arguments.ArgumentToParameterMap + .SelectReadOnlyArray( + index => index >= 0 ? method.Parameters[index].Type : SpecialType.UnknownType + ); + expressionBuilder.typeInference.InferTypeArguments(method.TypeParameters, + Arguments.Arguments.SelectReadOnlyArray(a => a.ResolveResult), paramTypesInArgumentOrder, + out bool success); + return success; + } + + /// + /// C# has no syntax to spell out an anonymous type, so a null literal cannot be given such + /// a type with a cast. The minimal expression that produces a null value of an anonymous + /// type is a conditional expression whose never-taken branch creates an instance of the + /// type: true ? null : new { A = default(int) }. + /// Replaces null-literal arguments of an anonymous type with such an expression, so that + /// type arguments involving anonymous types (which cannot be written explicitly either) + /// become inferable from the arguments. + /// Returns true, if at least one argument was replaced. + /// + bool PinTypesOfNullArguments() + { + bool anyArgumentReplaced = false; + for (int i = 0; i < Arguments.Length; i++) + { + IType expectedType = Arguments.ExpectedParameters[i].Type; + if (Arguments.Arguments[i].Expression is not NullReferenceExpression) + continue; + if (!expectedType.IsAnonymousType() || NewAnonymousTypeInstance(expectedType) is not NewObj newObj) + continue; + var nullLiteral = Arguments.Arguments[i]; + Arguments.Arguments[i] = new ConditionalExpression(new PrimitiveExpression(true), + nullLiteral.Expression.Detach(), expressionBuilder.Translate(newObj, expectedType)) + .WithILInstruction(nullLiteral.ILInstructions) + .WithRR(new ResolveResult(expectedType)); + anyArgumentReplaced = true; + } + return anyArgumentReplaced; + } + + /// + /// Builds a 'newobj' instruction creating an instance of the anonymous type + /// with default property values; translating it yields + /// object-initializer syntax, the only way to name the type in source code. Returns null + /// if a property type involves an anonymous type other than by direct nesting (e.g. an + /// array of anonymous type), because its default value expression would have to name it. + /// + NewObj? NewAnonymousTypeInstance(IType type) + { + var newObj = new NewObj(type.GetConstructors().Single()); + foreach (var parameter in newObj.Method.Parameters) + { + ILInstruction? argument = parameter.Type.IsAnonymousType() + ? NewAnonymousTypeInstance(parameter.Type) + : parameter.Type.ContainsAnonymousType() ? null : new DefaultValue(parameter.Type); + if (argument == null) + return null; + newObj.Arguments.Add(argument); + } + return newObj; + } + /// + /// The steps only reach for type arguments as a last resort, but a method such as + /// Enumerable.OfType<TResult>(IEnumerable) can never infer them from its arguments, + /// and waiting leaves the expression full of casts that writing them would have made + /// unnecessary. So they are written up front where inference cannot succeed. Casting + /// the arguments takes that guess back, so casts still come first where they do help. + /// + void WriteTypeArgumentsInferenceCannotReach() + { + if (member is not IMethod method || method.TypeParameters.Count == 0) + return; + if ((allowed & ReferenceTransformation.RequireTypeArguments) == 0) + return; + if (IsPossibleExtensionMethodCallOnNull(method, Arguments.Arguments)) + return; + if (CanInferTypeArgumentsFromArguments(method)) + return; + if (expressionBuilder.settings.AnonymousTypes + && method.TypeArguments.Any(a => a.ContainsAnonymousType()) + && PinTypesOfNullArguments() + && CanInferTypeArgumentsFromArguments(method)) + { + // Anonymous types cannot be written as explicit type arguments; instead the + // null arguments were rewritten so that all type arguments are inferable. + return; + } + ApplyUpFront(ReferenceTransformation.RequireTypeArguments); + TypeArgumentsWereAppliedUpFront = true; + } + + /// + /// Performs an escalation before the first lookup rather than waiting for its turn, for a case the caller already knows the short spelling cannot serve. + /// Spends it either way. + /// + void ApplyUpFront(ReferenceTransformation step) + { + TryApply(step); + } + + /// + /// Escalates until the reference is unambiguous. Returns false once every escalation + /// is spent and it still is not, leaving the caller to emit its best effort. + /// Every escalation is spent by one use, which is what ends the loop. + /// + bool Run() + { + OverloadResolutionErrors errors; + while ((errors = Probe()) != OverloadResolutionErrors.None) + { + if (!TryRepair(errors) && !Escalate()) + return false; + } + return true; + } + + /// + /// Answers one specific overload resolution failure with the escalation that addresses + /// it, instead of taking the steps from the top. Returns false to fall back to the + /// steps, either because the failure has no targeted answer or because the answer is + /// already spent. + /// + bool TryRepair(OverloadResolutionErrors errors) + { + switch (errors) + { + case OverloadResolutionErrors.OutVarTypeMismatch: + Debug.Assert(Arguments.UseImplicitlyTypedOut); + Arguments.UseImplicitlyTypedOut = false; + return true; + case OverloadResolutionErrors.TypeInferenceFailed: + case OverloadResolutionErrors.WrongNumberOfTypeArguments: + return TryApply(ReferenceTransformation.RequireTypeArguments); + case OverloadResolutionErrors.MissingArgumentForRequiredParameter: + return TryApply(ReferenceTransformation.NoOptionalArgumentAllowed); + default: + return false; + } + } + + /// + /// The target as the checks below must see it: null while the reference is still + /// unqualified. Not the target to annotate a result with - an unqualified reference is + /// still translated against . + /// + ResolveResult? LookupTarget => RequireTarget ? Target.ResolveResult : null; + + /// + /// Re-resolves the current spelling. None means it binds back to . + /// + OverloadResolutionErrors Probe() + { + switch (member) + { + case IField field: + return ProbeField(field); + // Invocation syntax first: a parameterized property's accessor is written as a + // call, so IsAccessor must not claim it. + case IMethod method when invocationSyntax: + return ProbeCall(method); + case IMethod { IsAccessor: true } accessor: + // The check can resolve a member and still reject it, so the result is + // what it returns, not whether it found something. + bool unambiguous = IsUnambiguousAccess(expectedTargetDetails, + LookupTarget, accessor, Arguments.Arguments, Arguments.ArgumentNames, + out var foundAccessorOwner); + FoundMember = foundAccessorOwner; + return unambiguous + ? OverloadResolutionErrors.None + : OverloadResolutionErrors.AmbiguousMatch; + case IMethod method: + return IsUnambiguousMethodReference(expectedTargetDetails, method, LookupTarget, + TypeArguments, isExtensionMethodReference, out Result) + ? OverloadResolutionErrors.None + : OverloadResolutionErrors.AmbiguousMatch; + default: + throw new NotSupportedException(member.SymbolKind.ToString()); + } + } + + OverloadResolutionErrors ProbeField(IField field) + { + MemberResolveResult? result; + if (LookupTarget == null) + { + result = resolver.ResolveSimpleName(field.Name, EmptyList.Instance, + isInvocationTarget: false) as MemberResolveResult; + } + else + { + var lookup = CreateLookup(resolver); + result = lookup.Lookup(Target.ResolveResult, field.Name, EmptyList.Instance, + isInvocation: false) as MemberResolveResult; + } + Result = result; + if (result == null || result.IsError || !result.Member.Equals(field, NormalizeTypeVisitor.TypeErasure)) + return OverloadResolutionErrors.AmbiguousMatch; + return OverloadResolutionErrors.None; + } + + OverloadResolutionErrors ProbeCall(IMethod method) + { + var errors = IsUnambiguousCall(expressionBuilder, expectedTargetDetails, method, LookupTarget, + TypeArguments, Arguments.GetArgumentResolveResults().ToArray(), + Arguments.GetArgumentNames(), Arguments.FirstOptionalArgumentIndex, + out var foundMember, out bool bestCandidateIsExpandedForm); + FoundMember = foundMember; + if (errors != OverloadResolutionErrors.None) + return errors; + // Resolving to the same method in the other of its normal and expanded form still + // means the spelling is wrong, and no single error describes that. + return bestCandidateIsExpandedForm != Arguments.IsExpandedForm + ? OverloadResolutionErrors.AmbiguousMatch + : OverloadResolutionErrors.None; + } + + /// + /// Records an escalation the caller performed itself, or one that has nothing to do + /// here, so that it will not be offered. + /// + void MarkApplied(ReferenceTransformation step) + { + Applied |= step; + spent |= step; + } + + /// + /// Performs the first escalation still available, in the order this form of reference + /// prefers them. + /// + bool Escalate() + { + // TODO : implement some more intelligent algorithm that decides which of these fixes (cast args, add target, cast target, add type args) + // is best in this case. Additionally we should not cast all arguments at once, but step-by-step try to add only a minimal number of casts. + foreach (ReferenceTransformation step in steps) + { + if (TryApply(step)) + return true; + } + return false; + } + + /// + /// Performs one escalation, unless it is already spent, forbidden by the caller, or + /// does not apply to this kind of reference. + /// + bool TryApply(ReferenceTransformation step) + { + if ((spent & step) != 0 || (allowed & step) == 0 || !Apply(step)) + return false; + Applied |= step; + spent |= step; + return true; + } + + static MemberLookup CreateLookup(CSharpResolver resolver) + { + return new MemberLookup(resolver.CurrentTypeDefinition, resolver.CurrentTypeDefinition.ParentModule); + } + + /// + /// Overload resolution as the ambiguity checks need it: over the compilation the + /// reference is written in, and with its conversions, which decide whether an argument + /// fits a parameter at all. + /// + static OverloadResolution CreateOverloadResolution(CSharpResolver resolver, + IReadOnlyList arguments, string[]? argumentNames, IType[] typeArguments) + { + return new OverloadResolution(resolver.Compilation, arguments.ToArray(), argumentNames, + typeArguments, conversions: resolver.conversions); + } + + internal static OverloadResolutionErrors IsUnambiguousCall(ExpressionBuilder expressionBuilder, + ExpectedTargetDetails expectedTargetDetails, IMethod method, + ResolveResult? target, IType[] typeArguments, ResolveResult[] arguments, + string[]? argumentNames, int firstOptionalArgumentIndex, + out IParameterizedMember? foundMember, out bool bestCandidateIsExpandedForm) + { + CSharpResolver resolver = expressionBuilder.resolver; + foundMember = null; + bestCandidateIsExpandedForm = false; + var lookup = CreateLookup(resolver); + + Log.WriteLine("IsUnambiguousCall: Performing overload resolution for " + method); + Log.WriteCollection(" Arguments: ", arguments); + + argumentNames = firstOptionalArgumentIndex < 0 || argumentNames == null + ? argumentNames + : argumentNames.Take(firstOptionalArgumentIndex).ToArray(); + + var or = CreateOverloadResolution(resolver, arguments, argumentNames, typeArguments); + if (expectedTargetDetails.CallOpCode == OpCode.NewObj) + { + foreach (IMethod ctor in method.DeclaringType.GetConstructors()) + { + bool allowProtectedAccess = + resolver.CurrentTypeDefinition == method.DeclaringTypeDefinition; + if (lookup.IsAccessible(ctor, allowProtectedAccess)) + { + Log.Indent(); + OverloadResolutionErrors errors = or.AddCandidate(ctor); + Log.Unindent(); + or.LogCandidateAddingResult(" Candidate", ctor, errors); + } + } + } + else if (method.IsOperator) + { + IEnumerable operatorCandidates; + if (arguments.Length == 1) + { + IType argType = NullableType.GetUnderlyingType(arguments[0].Type); + operatorCandidates = resolver.GetUserDefinedOperatorCandidates(argType, method.Name); + if (method.Name == "op_Explicit") + { + // For casts, also consider candidates from the target type we are casting to. + var hashSet = new HashSet(operatorCandidates); + IType targetType = NullableType.GetUnderlyingType(method.ReturnType); + hashSet.UnionWith( + resolver.GetUserDefinedOperatorCandidates(targetType, method.Name) + ); + operatorCandidates = hashSet; + } + } + else if (arguments.Length == 2) + { + IType lhsType = NullableType.GetUnderlyingType(arguments[0].Type); + IType rhsType = NullableType.GetUnderlyingType(arguments[1].Type); + var hashSet = new HashSet(); + hashSet.UnionWith(resolver.GetUserDefinedOperatorCandidates(lhsType, method.Name)); + hashSet.UnionWith(resolver.GetUserDefinedOperatorCandidates(rhsType, method.Name)); + operatorCandidates = hashSet; + } + else + { + operatorCandidates = EmptyList.Instance; + } + foreach (var m in operatorCandidates) + { + or.AddCandidate(m); + } + } + else if (target == null) + { + var result = resolver.ResolveSimpleName(method.Name, typeArguments, isInvocationTarget: true) + as MethodGroupResolveResult; + if (result == null) + return OverloadResolutionErrors.AmbiguousMatch; + or.AddMethodLists(result.MethodsGroupedByDeclaringType.ToArray()); + } + else + { + var result = lookup.Lookup(target, method.Name, typeArguments, isInvocation: true) as MethodGroupResolveResult; + if (result == null) + return OverloadResolutionErrors.AmbiguousMatch; + or.AddMethodLists(result.MethodsGroupedByDeclaringType.ToArray()); + } + bestCandidateIsExpandedForm = or.BestCandidateIsExpandedForm; + if (or.BestCandidateErrors != OverloadResolutionErrors.None) + return or.BestCandidateErrors; + if (or.IsAmbiguous) + return OverloadResolutionErrors.AmbiguousMatch; + foundMember = or.GetBestCandidateWithSubstitutedTypeArguments(); + if (foundMember == null) + { + // Overload resolution reports no error for an empty candidate set - there is no + // best candidate to carry one - so a call that matched nothing has to be reported + // as unresolvable here. + return OverloadResolutionErrors.AmbiguousMatch; + } + if (!IsAppropriateCallTarget(expectedTargetDetails, method, foundMember)) + return OverloadResolutionErrors.AmbiguousMatch; + var map = or.GetArgumentToParameterMap(); + for (int i = 0; i < arguments.Length; i++) + { + ResolveResult arg = arguments[i]; + int parameterIndex = map[i]; + if (arg is OutVarResolveResult rr && parameterIndex >= 0) + { + var param = foundMember.Parameters[parameterIndex]; + var paramType = param.Type.UnwrapByRef(); + if (!paramType.Equals(rr.OriginalVariableType)) + return OverloadResolutionErrors.OutVarTypeMismatch; + } + } + + return OverloadResolutionErrors.None; + } + + bool IsUnambiguousAccess(ExpectedTargetDetails expectedTargetDetails, ResolveResult? target, IMethod method, + IList arguments, string[]? argumentNames, [NotNullWhen(true)] out IMember? foundMember) + { + Log.WriteLine("IsUnambiguousAccess: Performing overload resolution for " + method); + Log.WriteCollection(" Arguments: ", arguments.Select(a => a.ResolveResult)); + + foundMember = null; + if (target == null) + { + var result = resolver.ResolveSimpleName(method.AccessorOwner!.Name, + EmptyList.Instance, + isInvocationTarget: false) as MemberResolveResult; + if (result == null || result.IsError) + return false; + foundMember = result.Member; + } + else + { + var lookup = CreateLookup(resolver); + if (method.AccessorOwner!.SymbolKind == SymbolKind.Indexer) + { + var or = CreateOverloadResolution(resolver, arguments.SelectArray(a => a.ResolveResult), + argumentNames, Empty.Array); + or.AddMethodLists(lookup.LookupIndexers(target)); + if (or.BestCandidateErrors != OverloadResolutionErrors.None) + return false; + if (or.IsAmbiguous) + return false; + foundMember = or.GetBestCandidateWithSubstitutedTypeArguments(); + } + else + { + var result = lookup.Lookup(target, + method.AccessorOwner!.Name, + EmptyList.Instance, + isInvocation: false) as MemberResolveResult; + if (result == null || result.IsError) + return false; + foundMember = result.Member; + } + } + return foundMember != null && IsAppropriateCallTarget(expectedTargetDetails, method.AccessorOwner, foundMember); + } + + bool IsUnambiguousMethodReference(ExpectedTargetDetails expectedTargetDetails, IMethod method, ResolveResult? target, IReadOnlyList typeArguments, bool isExtensionMethodReference, [NotNullWhen(true)] out ResolveResult? result) + { + Log.WriteLine("IsUnambiguousMethodReference: Performing overload resolution for " + method); + + var lookup = CreateLookup(resolver); + OverloadResolution or; + + if (isExtensionMethodReference) + { + result = resolver.ResolveMemberAccess(target, method.Name, typeArguments, NameLookupMode.InvocationTarget) as MethodGroupResolveResult; + if (result == null) + return false; + or = ((MethodGroupResolveResult)result).PerformOverloadResolution(resolver.CurrentTypeResolveContext.Compilation, + method.Parameters.SelectReadOnlyArray(p => new TypeResolveResult(p.Type)), + argumentNames: null, allowExtensionMethods: true); + if (or == null || or.IsAmbiguous) + return false; + } + else + { + // There are no arguments; the parameter types stand in for them, and argument + // names are not possible. + or = CreateOverloadResolution(resolver, + method.Parameters.SelectReadOnlyArray(p => new TypeResolveResult(p.Type)), + argumentNames: null, typeArguments.ToArray()); + if (target == null) + { + result = resolver.ResolveSimpleName(method.Name, typeArguments, isInvocationTarget: false); + if (!(result is MethodGroupResolveResult mgrr)) + return false; + or.AddMethodLists(mgrr.MethodsGroupedByDeclaringType.ToArray()); + } + else + { + result = lookup.Lookup(target, method.Name, typeArguments, isInvocation: false); + if (!(result is MethodGroupResolveResult mgrr)) + return false; + or.AddMethodLists(mgrr.MethodsGroupedByDeclaringType.ToArray()); + } + } + + var foundMethod = or.GetBestCandidateWithSubstitutedTypeArguments(); + if (!IsAppropriateCallTarget(expectedTargetDetails, method, foundMethod)) + return false; + return result is MethodGroupResolveResult; + } + + /// + /// Checks whether calling `target.methodName()` will use `expected` as the method to invoke. + /// + internal static bool CheckSimpleCall(ExpressionBuilder expressionBuilder, ResolveResult target, + IMethod expected, OpCode expectedCallOpCode = OpCode.Call) + { + var details = new ExpectedTargetDetails { CallOpCode = expectedCallOpCode, NeedsBoxingConversion = false }; + CSharpResolver resolver = expressionBuilder.resolver; + if (resolver.ResolveMemberAccess(target, expected.Name, [], NameLookupMode.InvocationTarget) + is not MethodGroupResolveResult mgrr) + return false; + var or = mgrr.PerformOverloadResolution(resolver.Compilation, []); + if (or.BestCandidateErrors != OverloadResolutionErrors.None || or.IsAmbiguous) + return false; + return IsAppropriateCallTarget(details, expected, or.GetBestCandidateWithSubstitutedTypeArguments()!); + } + + internal static bool IsAppropriateCallTarget(ExpectedTargetDetails expectedTargetDetails, IMember expectedTarget, IMember actualTarget) + { + if (expectedTarget.Equals(actualTarget, NormalizeTypeVisitor.TypeErasure)) + return true; + + if (expectedTargetDetails.CallOpCode == OpCode.CallVirt && actualTarget.IsOverride) + { + if (expectedTargetDetails.NeedsBoxingConversion && actualTarget.DeclaringType.IsReferenceType != true) + return false; + foreach (var possibleTarget in InheritanceHelper.GetBaseMembers(actualTarget, false)) + { + if (expectedTarget.Equals(possibleTarget, NormalizeTypeVisitor.TypeErasure)) + return true; + if (!possibleTarget.IsOverride) + break; + } + } + return false; + } + + void ModifyReturnTypeOfLambda(LambdaExpression lambda) + { + var resolveResult = (DecompiledLambdaResolveResult)lambda.GetResolveResult(); + if (lambda.Body is Expression exprBody) + lambda.Body = new TranslatedExpression(exprBody.Detach()).ConvertTo(resolveResult.ReturnType, expressionBuilder); + else + ModifyReturnStatementInsideLambda(resolveResult.ReturnType, lambda); + resolveResult.InferredReturnType = resolveResult.ReturnType; + } + + void CastArguments(IList arguments, IList expectedParameters) + { + for (int i = 0; i < arguments.Count; i++) + { + if (expressionBuilder.settings.AnonymousTypes && expectedParameters[i].Type.ContainsAnonymousType()) + { + if (arguments[i].Expression is LambdaExpression lambda) + { + ModifyReturnTypeOfLambda(lambda); + } + } + else + { + IParameter parameter = expectedParameters[i]; + IType parameterType; + if (parameter.Type.Kind == TypeKind.Dynamic) + { + parameterType = expressionBuilder.compilation.FindType(KnownTypeCode.Object); + } + else + { + parameterType = parameter.Type; + } + + if (parameter.ReferenceKind == ReferenceKind.In && parameterType is ByReferenceType brt && arguments[i].Type is not ByReferenceType) + { + parameterType = brt.ElementType; + } + + arguments[i] = arguments[i].ConvertTo(parameterType, expressionBuilder, allowImplicitConversion: false); + } + } + } + + void EnforceExplicitIn(TranslatedExpression[] arguments, IParameter[] expectedParameters) + { + for (int i = 0; i < arguments.Length; i++) + { + if (expectedParameters[i].ReferenceKind != ReferenceKind.In) + continue; + if (arguments[i].Expression is DirectionExpression) + continue; + + arguments[i] = WrapInAsRefReadOnly(arguments[i]); + expressionBuilder.statementBuilder.EmitAsRefReadOnly = true; + } + } + + TranslatedExpression WrapInAsRefReadOnly(TranslatedExpression arg) + { + return new DirectionExpression( + FieldDirection.In, + new InvocationExpression { + Target = new IdentifierExpression("ILSpyHelper_AsRefReadOnly"), + Arguments = { arg.Expression } + } + ).WithRR(new ByReferenceResolveResult(arg.Type, ReferenceKind.In)) + .WithoutILInstruction(); + } + + void ModifyReturnStatementInsideLambda(IType returnType, AstNode parent) + { + foreach (var child in parent.Children) + { + if (child is LambdaExpression || child is AnonymousMethodExpression) + continue; + if (child is ReturnStatement ret) + { + if (ret.Expression is not null) + ret.Expression = new TranslatedExpression(ret.Expression.Detach()).ConvertTo(returnType, expressionBuilder); + continue; + } + ModifyReturnStatementInsideLambda(returnType, child); + } + } + + bool Apply(ReferenceTransformation step) + { + switch (step) + { + case ReferenceTransformation.RequireTarget: + return true; // recording it in Applied is the whole effect + case ReferenceTransformation.CastTarget: + if (skipTargetCast && RequireTarget != initiallyRequiredTarget) + { + // A protected member of a base type cannot be reached through a cast + // target, so the qualifier the previous step added is worse than useless: + // drop it again and let the remaining escalations do the work. It stays + // spent, so it is not offered a second time. + Applied &= ~ReferenceTransformation.RequireTarget; + return true; + } + Target = Target.ConvertTo(declaringType, expressionBuilder); + return true; + case ReferenceTransformation.RequireTypeArguments: + if (member is not IMethod method) + return false; + TypeArguments = method.TypeArguments.ToArray(); + return true; + case ReferenceTransformation.NoNamedArgsForPrettiness: + if (!Arguments.AddNamesToPrimitiveValues) + return false; + Arguments.AddNamesToPrimitiveValues = false; + return true; + case ReferenceTransformation.NoOptionalArgumentAllowed: + if (Arguments.FirstOptionalArgumentIndex < 0) + return false; + Arguments.FirstOptionalArgumentIndex = -1; + return true; + case ReferenceTransformation.CastArguments: + if (TypeArgumentsWereAppliedUpFront) + { + // The guess did not help, so take it back completely: a later step may + // still reach for it once the casts are in. + TypeArgumentsWereAppliedUpFront = false; + Applied &= ~ReferenceTransformation.RequireTypeArguments; + spent &= ~ReferenceTransformation.RequireTypeArguments; + TypeArguments = Empty.Array; + } + if (castingArgumentsDropsImplicitlyTypedOut) + { + Arguments.UseImplicitlyTypedOut = false; + } + CastArguments(Arguments.Arguments, Arguments.ExpectedParameters); + return true; + case ReferenceTransformation.EnforceExplicitIn: + EnforceExplicitIn(Arguments.Arguments, Arguments.ExpectedParameters); + return true; + default: + return false; + } + } + } +} diff --git a/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs b/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs index 600ca0b845a..b1ec27f577c 100644 --- a/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs +++ b/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs @@ -367,46 +367,13 @@ ExpressionWithResolveResult ConvertField(IField field, ILInstruction? targetInst { requireTarget = RequiresQualifier(field, target); } - bool targetCasted = false; - var targetResolveResult = requireTarget ? target.ResolveResult : null; - - bool IsAmbiguousAccess(out MemberResolveResult? result) - { - if (targetResolveResult == null) - { - result = resolver.ResolveSimpleName(field.Name, EmptyList.Instance, isInvocationTarget: false) as MemberResolveResult; - } - else - { - var lookup = new MemberLookup(resolver.CurrentTypeDefinition, resolver.CurrentTypeDefinition.ParentModule); - result = lookup.Lookup(target.ResolveResult, field.Name, EmptyList.Instance, isInvocation: false) as MemberResolveResult; - } - return result == null || result.IsError || !result.Member.Equals(field, NormalizeTypeVisitor.TypeErasure); - } - - MemberResolveResult? mrr; - while (IsAmbiguousAccess(out mrr)) - { - if (!requireTarget) - { - requireTarget = true; - targetResolveResult = target.ResolveResult; - } - else if (!targetCasted) - { - targetCasted = true; - target = target.ConvertTo(field.DeclaringType, this); - targetResolveResult = target.ResolveResult; - } - else - { - // the field reference is still ambiguous, however, mrr might refer to a different member, - // e.g., in the case of auto events, their backing fields have the same name. - // "this.Event" is ambiguous, but should refer to the field, not the event. - mrr = null; - break; - } - } + var disambiguator = Disambiguator.ForField(this, field, target, requireTarget); + // On giving up, the reference stays ambiguous, however the resolved member might be a + // different one, e.g., in the case of auto events, whose backing fields have the same + // name. "this.Event" is ambiguous, but should refer to the field, not the event. + MemberResolveResult? mrr = disambiguator.Resolved ? (MemberResolveResult?)disambiguator.Result : null; + requireTarget = disambiguator.RequireTarget; + target = disambiguator.Target; if (mrr == null || !requireTarget) { @@ -4580,7 +4547,7 @@ protected internal override TranslatedExpression VisitAwait(Await inst, Translat boxedOperand = boxCast.Expression; lookupTarget = boxing.Input; } - if (!callBuilder.CheckSimpleCall(lookupTarget, inst.GetAwaiterMethod, inst.GetAwaiterCallOpCode)) + if (!Disambiguator.CheckSimpleCall(this, lookupTarget, inst.GetAwaiterMethod, inst.GetAwaiterCallOpCode)) { value = value.ConvertTo(expectedType, this); } From 16196731d348362b6b2ee835d8edd9ef7fcd9812 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sat, 19 Sep 2026 08:42:52 +0200 Subject: [PATCH 3/4] Decide every qualifier with one rule Whether a member reference has to name its target was decided in three places: RequiresQualifier, and a copy each in the accessor and call paths. They disagreed in two ways that both look like oversights. Only the method group path failed to consult AlwaysQualifyMemberReferences. That setting is on for the WinForms InitializeComponent method, which is also decompiled with implicit method group conversion off, so every event handler there came out as new EventHandler(Foo) while everything around it was qualified. A base target was the other, and the copies answered it three ways: never for fields, always for accessors and method groups, and a virtual-only test for calls. Dropping "base." leaves the reference dispatching virtually, which reaches the same member unless the member can be overridden and the IL did not dispatch virtually - so IsOverridable decides it, not the kind of reference. Never is right only because a field access does not dispatch; always keeps qualifiers that change nothing; and virtual-only is too narrow, since an abstract or overriding member is dispatched virtually without carrying the keyword. That last one drops the qualifier on base.OverridingMember, which reaches this type's member instead of the base one the IL named, and the escalation cannot catch it: with nothing overridden here the shorter spelling binds to the same member and looks correct. QualifierTests already covered base references, but only where the calling type shadows or overrides the member, so the qualifier was always required and over-removal could not show up. It now also covers a member overridden further up the hierarchy. The spellings that change instead of round-tripping are Ugly test cases. Assisted-by: Claude:claude-opus-5[1m]:Claude Code --- .../ICSharpCode.Decompiler.Tests.csproj | 4 ++ .../TestCases/Pretty/QualifierTests.cs | 40 +++++++++++ .../TestCases/Ugly/BaseQualifier.Expected.cs | 65 ++++++++++++++++++ .../TestCases/Ugly/BaseQualifier.cs | 68 +++++++++++++++++++ .../Ugly/QualifiedMethodGroup.Expected.cs | 20 ++++++ .../TestCases/Ugly/QualifiedMethodGroup.cs | 21 ++++++ .../UglyTestRunner.cs | 18 +++++ ICSharpCode.Decompiler/CSharp/CallBuilder.cs | 36 +++++----- .../CSharp/ExpressionBuilder.cs | 16 ++++- 9 files changed, 266 insertions(+), 22 deletions(-) create mode 100644 ICSharpCode.Decompiler.Tests/TestCases/Ugly/BaseQualifier.Expected.cs create mode 100644 ICSharpCode.Decompiler.Tests/TestCases/Ugly/BaseQualifier.cs create mode 100644 ICSharpCode.Decompiler.Tests/TestCases/Ugly/QualifiedMethodGroup.Expected.cs create mode 100644 ICSharpCode.Decompiler.Tests/TestCases/Ugly/QualifiedMethodGroup.cs diff --git a/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj b/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj index 7bbdf3087d9..320d4badbf6 100644 --- a/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj +++ b/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj @@ -273,6 +273,10 @@ + + + + diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/QualifierTests.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/QualifierTests.cs index 2f5eb6f5fda..8f3a9a99b23 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/QualifierTests.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/QualifierTests.cs @@ -139,6 +139,46 @@ public void BaseQualifiers() } } + internal class OverridingParent : Parent + { +#if LEGACY_CSC + public virtual int Prop { + get { + return 1; + } + } +#else + public virtual int Prop => 1; +#endif + + public override void Virtual() + { + } + } + + internal class OverridingChild : OverridingParent + { +#if LEGACY_CSC + public override int Prop { + get { + return 2; + } + } +#else + public override int Prop => 2; +#endif + + // Neither member is overridden here, so the unqualified spelling would bind to the + // same one and the qualifier looks redundant - but Virtual is an override and Prop is + // overridden further down, so dropping it would dispatch to this type's member + // instead of the one the base call names. + public void BaseQualifiersOnOverriddenMembers() + { + base.Virtual(); + base.Prop.ToString(); + } + } + #pragma warning disable CS8981 private class i { diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Ugly/BaseQualifier.Expected.cs b/ICSharpCode.Decompiler.Tests/TestCases/Ugly/BaseQualifier.Expected.cs new file mode 100644 index 00000000000..c711ecab68d --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/TestCases/Ugly/BaseQualifier.Expected.cs @@ -0,0 +1,65 @@ +using System; + +namespace ICSharpCode.Decompiler.Tests.TestCases.Ugly; + +public class BaseQualifierBase : BaseQualifierRoot +{ + public int NonVirtualProperty { get; set; } + public virtual int VirtualProperty { get; set; } + public override int AbstractProperty { get; set; } + public override int OverriddenProperty { get; set; } + public sealed override int SealedProperty { get; set; } + + public void NonVirtualMethod() + { + } + + public virtual void VirtualMethod() + { + } +} + +public class BaseQualifierDerived : BaseQualifierBase +{ + public int ReadNonVirtualProperty() + { + return NonVirtualProperty; + } + + public int ReadSealedProperty() + { + return SealedProperty; + } + + public Action NonVirtualMethodGroup() + { + return NonVirtualMethod; + } + + public int ReadVirtualProperty() + { + return base.VirtualProperty; + } + + public int ReadAbstractProperty() + { + return base.AbstractProperty; + } + + public int ReadOverriddenProperty() + { + return base.OverriddenProperty; + } + + public Action VirtualMethodGroup() + { + return base.VirtualMethod; + } +} + +public abstract class BaseQualifierRoot +{ + public abstract int AbstractProperty { get; set; } + public virtual int OverriddenProperty { get; set; } + public virtual int SealedProperty { get; set; } +} diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Ugly/BaseQualifier.cs b/ICSharpCode.Decompiler.Tests/TestCases/Ugly/BaseQualifier.cs new file mode 100644 index 00000000000..8348dd0f34d --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/TestCases/Ugly/BaseQualifier.cs @@ -0,0 +1,68 @@ +using System; + +namespace ICSharpCode.Decompiler.Tests.TestCases.Ugly +{ + public abstract class BaseQualifierRoot + { + public abstract int AbstractProperty { get; set; } + public virtual int OverriddenProperty { get; set; } + public virtual int SealedProperty { get; set; } + } + + public class BaseQualifierBase : BaseQualifierRoot + { + public int NonVirtualProperty { get; set; } + public virtual int VirtualProperty { get; set; } + public override int AbstractProperty { get; set; } + public override int OverriddenProperty { get; set; } + public sealed override int SealedProperty { get; set; } + + public void NonVirtualMethod() + { + } + + public virtual void VirtualMethod() + { + } + } + + public class BaseQualifierDerived : BaseQualifierBase + { + // Dropping "base." leaves the reference dispatching virtually. That reaches the same + // member unless the member can be overridden, so only the overridable ones keep it. + public int ReadNonVirtualProperty() + { + return base.NonVirtualProperty; + } + + public int ReadSealedProperty() + { + return base.SealedProperty; + } + + public Action NonVirtualMethodGroup() + { + return base.NonVirtualMethod; + } + + public int ReadVirtualProperty() + { + return base.VirtualProperty; + } + + public int ReadAbstractProperty() + { + return base.AbstractProperty; + } + + public int ReadOverriddenProperty() + { + return base.OverriddenProperty; + } + + public Action VirtualMethodGroup() + { + return base.VirtualMethod; + } + } +} diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Ugly/QualifiedMethodGroup.Expected.cs b/ICSharpCode.Decompiler.Tests/TestCases/Ugly/QualifiedMethodGroup.Expected.cs new file mode 100644 index 00000000000..2a98fe2f4bc --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/TestCases/Ugly/QualifiedMethodGroup.Expected.cs @@ -0,0 +1,20 @@ +using System; + +namespace ICSharpCode.Decompiler.Tests.TestCases.Ugly; + +public class QualifiedMethodGroup +{ + public int Value; + + public event EventHandler Changed; + + public void Subscribe() + { + this.Changed += new EventHandler(this.OnChanged); + this.Value = 1; + } + + private void OnChanged(object sender, EventArgs e) + { + } +} diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Ugly/QualifiedMethodGroup.cs b/ICSharpCode.Decompiler.Tests/TestCases/Ugly/QualifiedMethodGroup.cs new file mode 100644 index 00000000000..b02a5bf06ff --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/TestCases/Ugly/QualifiedMethodGroup.cs @@ -0,0 +1,21 @@ +using System; + +namespace ICSharpCode.Decompiler.Tests.TestCases.Ugly +{ + public class QualifiedMethodGroup + { + public event EventHandler Changed; + + public int Value; + + public void Subscribe() + { + Changed += OnChanged; + Value = 1; + } + + private void OnChanged(object sender, EventArgs e) + { + } + } +} diff --git a/ICSharpCode.Decompiler.Tests/UglyTestRunner.cs b/ICSharpCode.Decompiler.Tests/UglyTestRunner.cs index 15127eb5c2b..b6a70a46b0c 100644 --- a/ICSharpCode.Decompiler.Tests/UglyTestRunner.cs +++ b/ICSharpCode.Decompiler.Tests/UglyTestRunner.cs @@ -218,6 +218,24 @@ public async Task TopLevelProgramAsync([ValueSource(nameof(topLevelProgramOption await Run(cscOptions: cscOptions); } + // AlwaysQualifyMemberReferences is turned on for the WinForms InitializeComponent method, + // which is also decompiled with implicit method group conversion off - so the method group + // has to be qualified like every other member reference. + [Test] + public async Task QualifiedMethodGroup([ValueSource(nameof(roslynOnlyOptions))] CompilerOptions cscOptions) + { + await RunForLibrary(cscOptions: cscOptions, decompilerSettings: new DecompilerSettings { + AlwaysQualifyMemberReferences = true, + UseImplicitMethodGroupConversion = false + }); + } + + [Test] + public async Task BaseQualifier([ValueSource(nameof(roslynOnlyOptions))] CompilerOptions cscOptions) + { + await RunForLibrary(cscOptions: cscOptions); + } + async Task RunForLibrary([CallerMemberName] string testName = null, CompilerOptions cscOptions = CompilerOptions.None, DecompilerSettings decompilerSettings = null) { await Run(testName, cscOptions | CompilerOptions.Library, decompilerSettings); diff --git a/ICSharpCode.Decompiler/CSharp/CallBuilder.cs b/ICSharpCode.Decompiler/CSharp/CallBuilder.cs index 2dd2aec84a2..b6e1dc244db 100644 --- a/ICSharpCode.Decompiler/CSharp/CallBuilder.cs +++ b/ICSharpCode.Decompiler/CSharp/CallBuilder.cs @@ -1242,22 +1242,21 @@ private ReferenceTransformation GetRequiredTransformationsForCall(ExpectedTarget bool requireTarget; if ((allowedTransforms & ReferenceTransformation.RequireTarget) != 0) { - if (settings.AlwaysQualifyMemberReferences || expressionBuilder.HidesVariableWithName(method.Name)) + if (method.IsLocalFunction) { + // A local function is never reached through a target. + requireTarget = settings.AlwaysQualifyMemberReferences + || expressionBuilder.HidesVariableWithName(method.Name); + } + else if (method.Name == ".ctor" || method.Name == ".cctor") + { + // Always use target for base/this-ctor-call, the constructor initializer pattern depends on this requireTarget = true; } else { - if (method.IsLocalFunction) - requireTarget = false; - else if (method.IsStatic) - requireTarget = !expressionBuilder.IsCurrentOrContainingType(method.DeclaringTypeDefinition) || method.Name == ".cctor"; - else if (method.Name == ".ctor") - requireTarget = true; // always use target for base/this-ctor-call, the constructor initializer pattern depends on this - else if (target.Expression is BaseReferenceExpression) - requireTarget = (expectedTargetDetails.CallOpCode != OpCode.CallVirt && method.IsVirtual); - else - requireTarget = target.Expression is not ThisReferenceExpression; + requireTarget = expressionBuilder.RequiresQualifier(method, target, + nonVirtualDispatch: expectedTargetDetails.CallOpCode != OpCode.CallVirt); } } else @@ -1347,13 +1346,10 @@ private ExpressionWithResolveResult HandleImplicitConversion(IMethod method, Tra ExpressionWithResolveResult HandleAccessorCall(ExpectedTargetDetails expectedTargetDetails, IMethod method, TranslatedExpression target, ArgumentList argumentList) { - bool requireTarget; - if (settings.AlwaysQualifyMemberReferences || method.AccessorOwner!.SymbolKind == SymbolKind.Indexer || expressionBuilder.HidesVariableWithName(method.AccessorOwner.Name)) - requireTarget = true; - else if (method.IsStatic) - requireTarget = !expressionBuilder.IsCurrentOrContainingType(method.DeclaringTypeDefinition); - else - requireTarget = !(target.Expression is ThisReferenceExpression); + // An indexer has no name of its own to write, so it can never drop its target. + bool requireTarget = method.AccessorOwner!.SymbolKind == SymbolKind.Indexer + || expressionBuilder.RequiresQualifier(method.AccessorOwner, target, + nonVirtualDispatch: expectedTargetDetails.CallOpCode != OpCode.CallVirt); bool isSetter = method.ReturnType.IsKnownType(KnownTypeCode.Void); TranslatedExpression value = default(TranslatedExpression); @@ -1641,8 +1637,8 @@ ExpressionWithResolveResult BuildDelegateReference(IMethod method, IMethod? invo memberStatic: method.IsStatic, memberDeclaringType: method.DeclaringType); // check if target is required - bool requireTarget = expressionBuilder.HidesVariableWithName(method.Name) - || (method.IsStatic ? !expressionBuilder.IsCurrentOrContainingType(method.DeclaringTypeDefinition) : !(target.Expression is ThisReferenceExpression)); + bool requireTarget = expressionBuilder.RequiresQualifier(method, target, + nonVirtualDispatch: expectedTargetDetails.CallOpCode != OpCode.CallVirt); var disambiguator = Disambiguator.ForMethodReference(expressionBuilder, method, targetType, target, requireTarget, expectedTargetDetails, isExtensionMethodReference: false); ResolveResult? result = disambiguator.Result; diff --git a/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs b/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs index b1ec27f577c..bfa376e8db8 100644 --- a/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs +++ b/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs @@ -290,13 +290,25 @@ bool HidesVariableOrNestedFunction(ILFunction function) return null; } - bool RequiresQualifier(IMember member, TranslatedExpression target) + /// + /// Whether a reference to has to name its target to reach it. + /// Dropping a "base." qualifier leaves the reference to dispatch virtually, which reaches + /// the same member unless the member can be overridden and the IL did not dispatch + /// virtually - so is what a base target turns on. A + /// reference that does not dispatch at all, such as a field access, never needs it. + /// Overridable, not virtual: an abstract or overriding member is dispatched virtually + /// without carrying the keyword, and a sealed override can no longer be overridden, so + /// dispatching it virtually reaches the same member anyway. + /// + internal bool RequiresQualifier(IMember member, TranslatedExpression target, bool nonVirtualDispatch = false) { if (settings.AlwaysQualifyMemberReferences || HidesVariableWithName(member.Name)) return true; if (member.IsStatic) return !IsCurrentOrContainingType(member.DeclaringTypeDefinition); - return !(target.Expression is ThisReferenceExpression || target.Expression is BaseReferenceExpression); + if (target.Expression is BaseReferenceExpression) + return nonVirtualDispatch && member.IsOverridable; + return target.Expression is not ThisReferenceExpression; } /// From 8faa1848d5aa975b6c4df7275f67b20fda343801 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sat, 19 Sep 2026 08:43:07 +0200 Subject: [PATCH 4/4] Close the test fixtures that hold an assembly open A PEFile keeps its assembly mapped until it is disposed, and three fixtures opened one and never did: DuplicateAssemblyReferenceTests both directly and for every reference its resolver hands back, TypeForwarderResolutionTests for everything the resolver probes, and LocationsInAstTests for the module behind each decompiler. Their teardowns then deleted the directory those files live in, which cannot work while the mapping is open, so a Windows run reported a failed teardown for a run that otherwise passed. They now dispose what they opened, the way WpfProjectExportTests already did. Deleting is cleanup either way, so it goes through RepeatOnIOError rather than straight at the file system, and that helper no longer rethrows: its last of five attempts was deliberately unguarded, and four retries ten milliseconds apart is a short window to wait for a handle that a virus scanner or the indexer still holds. It now backs off to about a second and a half and, failing that, reports and leaves the file behind, which costs nothing. Assisted-by: Claude:claude-opus-5[1m]:Claude Code --- .../Documentation/IdStringProviderTests.cs | 4 +-- .../Helpers/Tester.cs | 25 +++++++++++++------ .../Output/LocationsInAstTests.cs | 6 +++++ .../WpfProjectExportTests.cs | 3 ++- .../DuplicateAssemblyReferenceTests.cs | 21 +++++++++++++--- .../TypeForwarderResolutionTests.cs | 13 ++++++++-- .../UniversalAssemblyResolverTests.cs | 3 ++- 7 files changed, 57 insertions(+), 18 deletions(-) diff --git a/ICSharpCode.Decompiler.Tests/Documentation/IdStringProviderTests.cs b/ICSharpCode.Decompiler.Tests/Documentation/IdStringProviderTests.cs index 78c65be6c8e..3d4395334b2 100644 --- a/ICSharpCode.Decompiler.Tests/Documentation/IdStringProviderTests.cs +++ b/ICSharpCode.Decompiler.Tests/Documentation/IdStringProviderTests.cs @@ -553,9 +553,7 @@ public void TearDown() roslynIdMap = null; if (tempDllPath != null && File.Exists(tempDllPath)) { - try - { File.Delete(tempDllPath); } - catch { /* best effort cleanup */ } + Tester.RepeatOnIOError(() => File.Delete(tempDllPath)); } } diff --git a/ICSharpCode.Decompiler.Tests/Helpers/Tester.cs b/ICSharpCode.Decompiler.Tests/Helpers/Tester.cs index a56830f3fb0..fb608658ffe 100644 --- a/ICSharpCode.Decompiler.Tests/Helpers/Tester.cs +++ b/ICSharpCode.Decompiler.Tests/Helpers/Tester.cs @@ -1161,26 +1161,37 @@ public static async Task RunAndCompareOutput(string testFileName, string outputF } } - internal static void RepeatOnIOError(Action action, int numTries = 5) + /// + /// Retries an IO operation that a virus scanner, the indexer or a compiler that has only + /// just exited can still hold a handle on, backing off between attempts. Every caller is + /// deleting a temp file, so a failure is reported and swallowed: the file is left behind, + /// which costs nothing, where throwing out of a fixture teardown reports an error for a + /// run that otherwise passed. + /// + internal static void RepeatOnIOError(Action action, int numTries = 8) { - for (int i = 0; i < numTries - 1; i++) + Exception lastError = null; + int delay = 10; + for (int i = 0; i < numTries; i++) { try { action(); return; } - catch (IOException) + catch (IOException ex) { + lastError = ex; } - catch (UnauthorizedAccessException) + catch (UnauthorizedAccessException ex) { // potential virus scanner problem + lastError = ex; } - Thread.Sleep(10); + Thread.Sleep(delay); + delay = Math.Min(delay * 2, 500); } - // If the last try still fails, don't catch the exception - action(); + TestContext.Out.WriteLine($"Cleanup could not delete a temp file after {numTries} tries, leaving it behind: {lastError?.Message}"); } public static async Task SignAssembly(string assemblyPath, string keyFilePath) diff --git a/ICSharpCode.Decompiler.Tests/Output/LocationsInAstTests.cs b/ICSharpCode.Decompiler.Tests/Output/LocationsInAstTests.cs index 50eeee9373b..34db1dfb954 100644 --- a/ICSharpCode.Decompiler.Tests/Output/LocationsInAstTests.cs +++ b/ICSharpCode.Decompiler.Tests/Output/LocationsInAstTests.cs @@ -17,6 +17,7 @@ // DEALINGS IN THE SOFTWARE. using System; +using System.Collections.Generic; using System.IO; using System.Linq; @@ -55,6 +56,7 @@ public class LocationsInAstTests const string SequencePointSampleName = "ICSharpCode.Decompiler.Tests.TestCases.LocationsInAst.SequencePointSample"; static CompilerResults compiledSamples; + static readonly List openedModules = new List(); [OneTimeSetUp] public void CompileSamples() @@ -66,6 +68,9 @@ public void CompileSamples() [OneTimeTearDown] public void DeleteCompiledSamples() { + // The module keeps the assembly mapped, so it has to go before the file can. + foreach (var module in openedModules) + module.Dispose(); compiledSamples?.DeleteTempFiles(); } @@ -73,6 +78,7 @@ static CSharpDecompiler CreateDecompiler(out DecompilerSettings settings) { string assemblyPath = compiledSamples.PathToAssembly; var module = new PEFile(assemblyPath); + openedModules.Add(module); var resolver = new UniversalAssemblyResolver(assemblyPath, false, module.Metadata.DetectTargetFrameworkId()); settings = new DecompilerSettings(); return new CSharpDecompiler(module, resolver, settings); diff --git a/ICSharpCode.Decompiler.Tests/ProjectDecompiler/WpfProjectExportTests.cs b/ICSharpCode.Decompiler.Tests/ProjectDecompiler/WpfProjectExportTests.cs index 50b30d5561c..fe412204905 100644 --- a/ICSharpCode.Decompiler.Tests/ProjectDecompiler/WpfProjectExportTests.cs +++ b/ICSharpCode.Decompiler.Tests/ProjectDecompiler/WpfProjectExportTests.cs @@ -25,6 +25,7 @@ using ICSharpCode.Decompiler.CSharp.ProjectDecompiler; using ICSharpCode.Decompiler.Metadata; +using ICSharpCode.Decompiler.Tests.Helpers; using ICSharpCode.Decompiler.TypeSystem; using Microsoft.CodeAnalysis; @@ -107,7 +108,7 @@ public void TearDown() foreach (var module in openedModules) module.Dispose(); if (Directory.Exists(tempDirectory)) - Directory.Delete(tempDirectory, recursive: true); + Tester.RepeatOnIOError(() => Directory.Delete(tempDirectory, recursive: true)); } /// diff --git a/ICSharpCode.Decompiler.Tests/TypeSystem/DuplicateAssemblyReferenceTests.cs b/ICSharpCode.Decompiler.Tests/TypeSystem/DuplicateAssemblyReferenceTests.cs index d3def0338cb..b28954cdc07 100644 --- a/ICSharpCode.Decompiler.Tests/TypeSystem/DuplicateAssemblyReferenceTests.cs +++ b/ICSharpCode.Decompiler.Tests/TypeSystem/DuplicateAssemblyReferenceTests.cs @@ -17,6 +17,7 @@ // DEALINGS IN THE SOFTWARE. using System; +using System.Collections.Generic; using System.IO; using System.Linq; using System.Threading.Tasks; @@ -113,6 +114,7 @@ .method public hidebysig virtual instance void Run(class [Lib]Param p) cil manag string directory; string mainAssemblyPath; + readonly List openedModules = new List(); [OneTimeSetUp] public async Task SetUp() @@ -129,7 +131,11 @@ public async Task SetUp() [OneTimeTearDown] public void TearDown() { - Directory.Delete(directory, recursive: true); + // A PEFile keeps the assembly mapped, so the directory cannot be removed while any + // of the modules these tests opened is still around. + foreach (var module in openedModules) + module.Dispose(); + Tester.RepeatOnIOError(() => Directory.Delete(directory, recursive: true)); } Task AssembleAsync(string name, string il) @@ -142,7 +148,8 @@ Task AssembleAsync(string name, string il) DecompilerTypeSystem CreateTypeSystem() { var mainModule = new PEFile(mainAssemblyPath); - return new DecompilerTypeSystem(mainModule, new VersionedResolver(directory)); + openedModules.Add(mainModule); + return new DecompilerTypeSystem(mainModule, new VersionedResolver(directory, openedModules)); } [Test] @@ -179,10 +186,12 @@ class VersionedResolver : IAssemblyResolver static readonly string runtimeDirectory = Path.GetDirectoryName(typeof(object).Assembly.Location); readonly string directory; + readonly List openedModules; - public VersionedResolver(string directory) + public VersionedResolver(string directory, List openedModules) { this.directory = directory; + this.openedModules = openedModules; } public MetadataFile Resolve(IAssemblyReference reference) @@ -190,7 +199,11 @@ public MetadataFile Resolve(IAssemblyReference reference) string path = reference.Name == "Lib" ? Path.Combine(directory, $"Lib.v{reference.Version.Major}.dll") : Path.Combine(runtimeDirectory, reference.Name + ".dll"); - return File.Exists(path) ? new PEFile(path) : null; + if (!File.Exists(path)) + return null; + var module = new PEFile(path); + openedModules.Add(module); + return module; } public MetadataFile ResolveModule(MetadataFile mainModule, string moduleName) => null; diff --git a/ICSharpCode.Decompiler.Tests/TypeSystem/TypeForwarderResolutionTests.cs b/ICSharpCode.Decompiler.Tests/TypeSystem/TypeForwarderResolutionTests.cs index b6ebed6b3c7..c0e21b5b930 100644 --- a/ICSharpCode.Decompiler.Tests/TypeSystem/TypeForwarderResolutionTests.cs +++ b/ICSharpCode.Decompiler.Tests/TypeSystem/TypeForwarderResolutionTests.cs @@ -17,6 +17,7 @@ // DEALINGS IN THE SOFTWARE. using System; +using System.Collections.Generic; using System.IO; using System.Linq; using System.Threading.Tasks; @@ -173,6 +174,7 @@ .method public hidebysig instance void UseUndeclared(class [Shim2]Ns.U u) cil ma "; string inputDirectory; + readonly List openedModules = new List(); string frameworkDirectory; string mainAssemblyPath; @@ -202,7 +204,9 @@ public async Task SetUp() [OneTimeTearDown] public void TearDown() { - Directory.Delete(Path.GetDirectoryName(inputDirectory), recursive: true); + foreach (var module in openedModules) + module.Dispose(); + Tester.RepeatOnIOError(() => Directory.Delete(Path.GetDirectoryName(inputDirectory), recursive: true)); } static Task AssembleAsync(string directory, string name, string il) @@ -215,12 +219,17 @@ static Task AssembleAsync(string directory, string name, string il) DecompilerTypeSystem CreateTypeSystem() { var mainModule = new PEFile(mainAssemblyPath); + openedModules.Add(mainModule); // The target framework decides which probe runs first, and the bug only shows on the // .NET Core path finder, which the .NET Standard identifier selects. var resolver = new UniversalAssemblyResolver(mainAssemblyPath, throwOnError: false, ".NETStandard,Version=v2.0"); resolver.AddSearchDirectory(frameworkDirectory); - return new DecompilerTypeSystem(mainModule, resolver); + var typeSystem = new DecompilerTypeSystem(mainModule, resolver); + // The resolver opens the framework and implementation assemblies as it probes; every + // one of them keeps its file mapped until it is disposed. + openedModules.AddRange(typeSystem.Modules.Select(m => m.MetadataFile).Where(f => f != null && f != mainModule)); + return typeSystem; } IParameter GetParameterOf(DecompilerTypeSystem typeSystem, string methodName) diff --git a/ICSharpCode.Decompiler.Tests/UniversalAssemblyResolverTests.cs b/ICSharpCode.Decompiler.Tests/UniversalAssemblyResolverTests.cs index 2630cfdfd59..b73271fc4d0 100644 --- a/ICSharpCode.Decompiler.Tests/UniversalAssemblyResolverTests.cs +++ b/ICSharpCode.Decompiler.Tests/UniversalAssemblyResolverTests.cs @@ -19,6 +19,7 @@ using System.IO; using ICSharpCode.Decompiler.Metadata; +using ICSharpCode.Decompiler.Tests.Helpers; using NUnit.Framework; @@ -44,7 +45,7 @@ public void SetUp() public void TearDown() { if (Directory.Exists(gacDirectory)) - Directory.Delete(gacDirectory, recursive: true); + Tester.RepeatOnIOError(() => Directory.Delete(gacDirectory, recursive: true)); } string AddAssembly(string name, string version, string publicKeyToken, string culture = "")