Skip to content

Commit

Permalink
More PRs for 17.10P2 snap (#72299)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jjonescz
jjonescz authored Feb 29, 2024
2 parents 1f4017d + c3b4361 commit bf8791d
Show file tree
Hide file tree
Showing 11 changed files with 362 additions and 102 deletions.
3 changes: 1 addition & 2 deletions src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1349,7 +1349,6 @@ internal void BindDefaultArgumentsAndParamsArray(
bool expanded,
bool enableCallerInfo,
BindingDiagnosticBag diagnostics,
bool assertMissingParametersAreOptional = true,
Symbol? attributedMember = null)
{
int firstParamArrayArgument = -1;
Expand Down Expand Up @@ -1485,7 +1484,7 @@ internal void BindDefaultArgumentsAndParamsArray(
{
if (!visitedParameters[parameter.Ordinal])
{
Debug.Assert(parameter.IsOptional || !assertMissingParametersAreOptional);
Debug.Assert(parameter.IsOptional);

defaultArguments[argumentsBuilder.Count] = true;
argumentsBuilder.Add(bindDefaultArgument(node, parameter, containingMember, enableCallerInfo, diagnostics, argumentsBuilder, argumentsCount, argsToParamsOpt));
Expand Down
8 changes: 4 additions & 4 deletions src/Compilers/CSharp/Portable/Binder/ForEachLoopBinder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1199,7 +1199,8 @@ private void GetDisposalInfoForEnumerator(SyntaxNode syntax, ref ForEachEnumerat
if (patternDisposeMethod is object)
{
Debug.Assert(!patternDisposeMethod.IsExtensionMethod);
Debug.Assert(patternDisposeMethod.ParameterRefKinds.IsDefaultOrEmpty);
Debug.Assert(patternDisposeMethod.ParameterRefKinds.IsDefaultOrEmpty ||
patternDisposeMethod.ParameterRefKinds.All(static refKind => refKind is RefKind.None or RefKind.In or RefKind.RefReadOnlyParameter));

var argsBuilder = ArrayBuilder<BoundExpression>.GetInstance(patternDisposeMethod.ParameterCount);
var argsToParams = default(ImmutableArray<int>);
Expand Down Expand Up @@ -1865,7 +1866,7 @@ private MethodArgumentInfo GetParameterlessSpecialTypeMemberInfo(SpecialMember m
}

/// <param name="extensionReceiverOpt">If method is an extension method, this must be non-null.</param>
private MethodArgumentInfo BindDefaultArguments(MethodSymbol method, BoundExpression extensionReceiverOpt, bool expanded, SyntaxNode syntax, BindingDiagnosticBag diagnostics, bool assertMissingParametersAreOptional = true)
private MethodArgumentInfo BindDefaultArguments(MethodSymbol method, BoundExpression extensionReceiverOpt, bool expanded, SyntaxNode syntax, BindingDiagnosticBag diagnostics)
{
Debug.Assert((extensionReceiverOpt != null) == method.IsExtensionMethod);

Expand All @@ -1892,8 +1893,7 @@ private MethodArgumentInfo BindDefaultArguments(MethodSymbol method, BoundExpres
defaultArguments: out BitVector defaultArguments,
expanded,
enableCallerInfo: true,
diagnostics,
assertMissingParametersAreOptional);
diagnostics);

Debug.Assert(argsToParams.IsDefault);
return new MethodArgumentInfo(method, argsBuilder.ToImmutableAndFree(), defaultArguments, expanded);
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_ForEachStatement.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -534,12 +534,12 @@ private BoundExpression ConvertReceiverForInvocation(CSharpSyntaxNode syntax, Bo
return receiver;
}

private BoundExpression SynthesizeCall(MethodArgumentInfo methodArgumentInfo, CSharpSyntaxNode syntax, BoundExpression? receiver, bool allowExtensionAndOptionalParameters, bool assertParametersAreOptional = true)
private BoundExpression SynthesizeCall(MethodArgumentInfo methodArgumentInfo, CSharpSyntaxNode syntax, BoundExpression? receiver, bool allowExtensionAndOptionalParameters)
{
if (allowExtensionAndOptionalParameters)
{
// Generate a call with zero explicit arguments, but with implicit arguments for optional and params parameters.
return MakeCallWithNoExplicitArgument(methodArgumentInfo, syntax, receiver, assertParametersAreOptional);
return MakeCallWithNoExplicitArgument(methodArgumentInfo, syntax, receiver);
}

// Generate a call with literally zero arguments
Expand Down
9 changes: 4 additions & 5 deletions src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_UsingStatement.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -489,23 +489,22 @@ private BoundExpression GenerateDisposeCall(
/// Synthesize a call `expression.Method()`, but with some extra smarts to handle extension methods, and to fill-in optional and params parameters. This call expects that the
/// receiver parameter has already been visited.
/// </summary>
private BoundExpression MakeCallWithNoExplicitArgument(MethodArgumentInfo methodArgumentInfo, SyntaxNode syntax, BoundExpression? expression, bool assertParametersAreOptional = true)
private BoundExpression MakeCallWithNoExplicitArgument(MethodArgumentInfo methodArgumentInfo, SyntaxNode syntax, BoundExpression? expression)
{
MethodSymbol method = methodArgumentInfo.Method;

#if DEBUG
if (method.IsExtensionMethod)
{
Debug.Assert(expression == null);
Debug.Assert(method.Parameters.AsSpan()[1..].All(assertParametersAreOptional, (p, assertOptional) => (p.IsOptional || p.IsParams || !assertOptional) && p.RefKind == RefKind.None));
Debug.Assert(method.ParameterRefKinds.IsDefaultOrEmpty || method.ParameterRefKinds[0] is RefKind.In or RefKind.RefReadOnlyParameter or RefKind.None);
Debug.Assert(method.Parameters.AsSpan()[1..].All(static (p) => (p.IsOptional || p.IsParams) && p.RefKind is RefKind.None or RefKind.In or RefKind.RefReadOnlyParameter));
}
else
{
Debug.Assert(!assertParametersAreOptional || method.Parameters.All(p => p.IsOptional || p.IsParams));
Debug.Assert(method.ParameterRefKinds.IsDefaultOrEmpty);
Debug.Assert(method.Parameters.All(p => p.IsOptional || p.IsParams));
}

Debug.Assert(method.ParameterRefKinds.IsDefaultOrEmpty || method.ParameterRefKinds.All(static refKind => refKind is RefKind.In or RefKind.RefReadOnlyParameter or RefKind.None));
Debug.Assert(methodArgumentInfo.Arguments.All(arg => arg is not BoundConversion { ConversionKind: ConversionKind.InterpolatedStringHandler }));
#endif

Expand Down
113 changes: 113 additions & 0 deletions src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenForEachTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4306,6 +4306,119 @@ public static class Extensions
CompileAndVerify(source, parseOptions: TestOptions.Regular9, expectedOutput: "123");
}

[Theory, CombinatorialData]
public void TestGetEnumeratorPatternViaInExtensionOnAssignableVariable_OptionalParameter(
[CombinatorialValues("ref", "in", "ref readonly", "")] string modifier)
{
var source = $$"""
using System;
public struct C
{
public static void Main()
{
var c = new C();
foreach (var i in c)
{
Console.Write(i);
}
}
public struct Enumerator
{
public int Current { get; private set; }
public bool MoveNext() => Current++ != 3;
}
}
public static class Extensions
{
public static C.Enumerator GetEnumerator(this in C self, {{modifier}} int x = 9)
{
Console.Write(x);
return new C.Enumerator();
}
}
""";
if (modifier == "ref")
{
CreateCompilation(source).VerifyDiagnostics(
// (7,27): error CS7036: There is no argument given that corresponds to the required parameter 'x' of 'Extensions.GetEnumerator(in C, ref int)'
// foreach (var i in c)
Diagnostic(ErrorCode.ERR_NoCorrespondingArgument, "c").WithArguments("x", "Extensions.GetEnumerator(in C, ref int)").WithLocation(7, 27),
// (7,27): error CS1579: foreach statement cannot operate on variables of type 'C' because 'C' does not contain a public instance or extension definition for 'GetEnumerator'
// foreach (var i in c)
Diagnostic(ErrorCode.ERR_ForEachMissingMember, "c").WithArguments("C", "GetEnumerator").WithLocation(7, 27),
// (20,62): error CS1741: A ref or out parameter cannot have a default value
// public static C.Enumerator GetEnumerator(this in C self, ref int x = 9)
Diagnostic(ErrorCode.ERR_RefOutDefaultValue, "ref").WithLocation(20, 62));
}
else
{
var verifier = CompileAndVerify(source, expectedOutput: "9123");
if (modifier == "ref readonly")
{
verifier.VerifyDiagnostics(
// (20,83): warning CS9200: A default value is specified for 'ref readonly' parameter 'x', but 'ref readonly' should be used only for references. Consider declaring the parameter as 'in'.
// public static C.Enumerator GetEnumerator(this in C self, ref readonly int x = 9)
Diagnostic(ErrorCode.WRN_RefReadonlyParameterDefaultValue, "9").WithArguments("x").WithLocation(20, 83));
}
else
{
verifier.VerifyDiagnostics();
}
}
}

[Theory, CombinatorialData]
public void TestDisposePattern_OptionalParameter(
[CombinatorialValues("ref", "in", "ref readonly", "")] string modifier)
{
var source = $$"""
using System;
public struct C
{
public static void Main()
{
var c = new C();
foreach (var i in c)
{
Console.Write(i);
}
}
public Enumerator GetEnumerator()
{
return new Enumerator();
}
public ref struct Enumerator
{
public int Current { get; private set; }
public bool MoveNext() => Current++ != 3;
public void Dispose({{modifier}} int x = 5) { Console.Write(x); }
}
}
""";
if (modifier == "ref")
{
CreateCompilation(source).VerifyDiagnostics(
// (20,29): error CS1741: A ref or out parameter cannot have a default value
// public void Dispose(ref int x = 5) { Console.Write(x); }
Diagnostic(ErrorCode.ERR_RefOutDefaultValue, "ref").WithLocation(20, 29));
}
else
{
var verifier = CompileAndVerify(source, expectedOutput: "1235", verify: Verification.FailsILVerify);
if (modifier == "ref readonly")
{
verifier.VerifyDiagnostics(
// (20,50): warning CS9200: A default value is specified for 'ref readonly' parameter 'x', but 'ref readonly' should be used only for references. Consider declaring the parameter as 'in'.
// public void Dispose(ref readonly int x = 5) { Console.Write(x); }
Diagnostic(ErrorCode.WRN_RefReadonlyParameterDefaultValue, "5").WithArguments("x").WithLocation(20, 50));
}
else
{
verifier.VerifyDiagnostics();
}
}
}

[Fact]
public void TestGetEnumeratorPatternViaExtensionsCSharp8()
{
Expand Down
44 changes: 44 additions & 0 deletions src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenUsingStatementTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3095,5 +3095,49 @@ static void Main()

CompileAndVerify(source, expectedOutput: "0");
}

[Theory, CombinatorialData]
public void OptionalParameter([CombinatorialValues("ref", "in", "ref readonly", "")] string modifier)
{
var source = $$"""
using System;
using (var s = new S())
{
Console.Write("1");
}
ref struct S
{
public void Dispose({{modifier}} int x = 2) => Console.Write(x);
}
""";
if (modifier == "ref")
{
CreateCompilation(source).VerifyDiagnostics(
// (2,8): error CS7036: There is no argument given that corresponds to the required parameter 'x' of 'S.Dispose(ref int)'
// using (var s = new S())
Diagnostic(ErrorCode.ERR_NoCorrespondingArgument, "var s = new S()").WithArguments("x", "S.Dispose(ref int)").WithLocation(2, 8),
// (2,8): error CS1674: 'S': type used in a using statement must be implicitly convertible to 'System.IDisposable'.
// using (var s = new S())
Diagnostic(ErrorCode.ERR_NoConvToIDisp, "var s = new S()").WithArguments("S").WithLocation(2, 8),
// (8,25): error CS1741: A ref or out parameter cannot have a default value
// public void Dispose(ref int x = 2) => Console.Write(x);
Diagnostic(ErrorCode.ERR_RefOutDefaultValue, "ref").WithLocation(8, 25));
}
else
{
var verifier = CompileAndVerify(source, expectedOutput: "12");
if (modifier == "ref readonly")
{
verifier.VerifyDiagnostics(
// (8,46): warning CS9200: A default value is specified for 'ref readonly' parameter 'x', but 'ref readonly' should be used only for references. Consider declaring the parameter as 'in'.
// public void Dispose(ref readonly int x = 2) => Console.Write(x);
Diagnostic(ErrorCode.WRN_RefReadonlyParameterDefaultValue, "2").WithArguments("x").WithLocation(8, 46));
}
else
{
verifier.VerifyDiagnostics();
}
}
}
}
}
19 changes: 11 additions & 8 deletions ...res/Core/Portable/IntroduceParameter/AbstractIntroduceParameterCodeRefactoringProvider.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ private static bool IsValidExpression(SyntaxNode expression, ISyntaxFactsService
using var actionsBuilder = TemporaryArray<CodeAction>.Empty;
using var actionsBuilderAllOccurrences = TemporaryArray<CodeAction>.Empty;
var syntaxFacts = document.GetRequiredLanguageService<ISyntaxFactsService>();
var methodCallSites = await FindCallSitesAsync(document, methodSymbol, cancellationToken).ConfigureAwait(false);

if (!containsClassExpression)
{
Expand All @@ -157,10 +158,14 @@ private static bool IsValidExpression(SyntaxNode expression, ISyntaxFactsService

if (methodSymbol.MethodKind is not MethodKind.Constructor)
{
actionsBuilder.Add(CreateNewCodeAction(
FeaturesResources.into_extracted_method_to_invoke_at_call_sites, allOccurrences: false, IntroduceParameterCodeActionKind.Trampoline));
actionsBuilderAllOccurrences.Add(CreateNewCodeAction(
FeaturesResources.into_extracted_method_to_invoke_at_call_sites, allOccurrences: true, IntroduceParameterCodeActionKind.Trampoline));
var containsObjectCreationReferences = methodCallSites.Values.Flatten().OfType<TObjectCreationExpressionSyntax>().Any();
if (!containsObjectCreationReferences)
{
actionsBuilder.Add(CreateNewCodeAction(
FeaturesResources.into_extracted_method_to_invoke_at_call_sites, allOccurrences: false, IntroduceParameterCodeActionKind.Trampoline));
actionsBuilderAllOccurrences.Add(CreateNewCodeAction(
FeaturesResources.into_extracted_method_to_invoke_at_call_sites, allOccurrences: true, IntroduceParameterCodeActionKind.Trampoline));
}

if (methodSymbol.MethodKind is not MethodKind.LocalFunction)
{
Expand All @@ -178,7 +183,7 @@ CodeAction CreateNewCodeAction(string actionName, bool allOccurrences, Introduce
{
return CodeAction.Create(
actionName,
cancellationToken => IntroduceParameterAsync(document, expression, methodSymbol, containingMethod, allOccurrences, selectedCodeAction, fallbackOptions, cancellationToken),
cancellationToken => IntroduceParameterAsync(document, expression, methodSymbol, containingMethod, methodCallSites, allOccurrences, selectedCodeAction, fallbackOptions, cancellationToken),
actionName);
}
}
Expand Down Expand Up @@ -229,11 +234,9 @@ CodeAction CreateNewCodeAction(string actionName, bool allOccurrences, Introduce
/// Introduces a new parameter and refactors all the call sites based on the selected code action.
/// </summary>
private async Task<Solution> IntroduceParameterAsync(Document originalDocument, TExpressionSyntax expression,
IMethodSymbol methodSymbol, SyntaxNode containingMethod, bool allOccurrences, IntroduceParameterCodeActionKind selectedCodeAction,
IMethodSymbol methodSymbol, SyntaxNode containingMethod, Dictionary<Document, List<TExpressionSyntax>> methodCallSites, bool allOccurrences, IntroduceParameterCodeActionKind selectedCodeAction,
CodeGenerationOptionsProvider fallbackOptions, CancellationToken cancellationToken)
{
var methodCallSites = await FindCallSitesAsync(originalDocument, methodSymbol, cancellationToken).ConfigureAwait(false);

var modifiedSolution = originalDocument.Project.Solution;
var rewriter = new IntroduceParameterDocumentRewriter(this, originalDocument,
expression, methodSymbol, containingMethod, selectedCodeAction, fallbackOptions, allOccurrences);
Expand Down
Loading

0 comments on commit bf8791d

Please sign in to comment.