diff --git a/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs b/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs index 2c5d439ab23ea..00f409a334fb3 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs @@ -1486,6 +1486,10 @@ internal uint GetInvocationEscapeScope( bool isRefEscape ) { +#if DEBUG + Debug.Assert(AllParametersConsideredInEscapeAnalysisHaveArguments(argsOpt, parameters, argsToParamsOpt)); +#endif + // SPEC: (also applies to the CheckInvocationEscape counterpart) // // An lvalue resulting from a ref-returning method invocation e1.M(e2, ...) is ref-safe - to - escape the smallest of the following scopes: @@ -1507,8 +1511,6 @@ bool isRefEscape //by default it is safe to escape uint escapeScope = Binder.ExternalScope; - ArrayBuilder? inParametersMatchedWithArgs = null; - if (!argsOpt.IsDefault) { moreArguments: @@ -1531,7 +1533,7 @@ bool isRefEscape goto moreArguments; } - RefKind effectiveRefKind = GetEffectiveRefKindAndMarkMatchedInParameter(argIndex, argRefKindsOpt, parameters, argsToParamsOpt, ref inParametersMatchedWithArgs, out DeclarationScope scope); + RefKind effectiveRefKind = GetEffectiveRefKind(argIndex, argRefKindsOpt, parameters, argsToParamsOpt, out DeclarationScope scope); // ref escape scope is the narrowest of // - ref escape of all byref arguments @@ -1554,25 +1556,12 @@ bool isRefEscape if (escapeScope >= scopeOfTheContainingExpression) { - // no longer needed - inParametersMatchedWithArgs?.Free(); - // can't get any worse return escapeScope; } } } - // handle omitted optional "in" parameters if there are any - ParameterSymbol? unmatchedInParameter = TryGetUnmatchedInParameterAndFreeMatchedArgs(parameters, ref inParametersMatchedWithArgs); - - // unmatched "in" parameter is the same as a literal, its ref escape is scopeOfTheContainingExpression (can't get any worse) - // its val escape is ExternalScope (does not affect overall result) - if (unmatchedInParameter != null && isRefEscape) - { - return scopeOfTheContainingExpression; - } - // check receiver if ref-like if (receiver?.Type?.IsRefLikeType == true) { @@ -1605,6 +1594,10 @@ private bool CheckInvocationEscape( bool isRefEscape ) { +#if DEBUG + Debug.Assert(AllParametersConsideredInEscapeAnalysisHaveArguments(argsOpt, parameters, argsToParamsOpt)); +#endif + // SPEC: // In a method invocation, the following constraints apply: //• If there is a ref or out argument to a ref struct type (including the receiver), with safe-to-escape E1, then @@ -1617,8 +1610,6 @@ bool isRefEscape receiver = null; } - ArrayBuilder? inParametersMatchedWithArgs = null; - if (!argsOpt.IsDefault) { moreArguments: @@ -1641,7 +1632,7 @@ bool isRefEscape goto moreArguments; } - RefKind effectiveRefKind = GetEffectiveRefKindAndMarkMatchedInParameter(argIndex, argRefKindsOpt, parameters, argsToParamsOpt, ref inParametersMatchedWithArgs, out DeclarationScope scope); + RefKind effectiveRefKind = GetEffectiveRefKind(argIndex, argRefKindsOpt, parameters, argsToParamsOpt, out DeclarationScope scope); // ref escape scope is the narrowest of // - ref escape of all byref arguments @@ -1662,9 +1653,6 @@ bool isRefEscape if (!valid) { - // no longer needed - inParametersMatchedWithArgs?.Free(); - ErrorCode errorCode = GetStandardCallEscapeError(checkingReceiver); string parameterName; @@ -1689,22 +1677,6 @@ bool isRefEscape } } - // handle omitted optional "in" parameters if there are any - ParameterSymbol? unmatchedInParameter = TryGetUnmatchedInParameterAndFreeMatchedArgs(parameters, ref inParametersMatchedWithArgs); - - // unmatched "in" parameter is the same as a literal, its ref escape is scopeOfTheContainingExpression (can't get any worse) - // its val escape is ExternalScope (does not affect overall result) - if (unmatchedInParameter != null && isRefEscape) - { - var parameterName = unmatchedInParameter.Name; - if (string.IsNullOrEmpty(parameterName)) - { - parameterName = unmatchedInParameter.Ordinal.ToString(); - } - Error(diagnostics, GetStandardCallEscapeError(checkingReceiver), syntax, symbol, parameterName); - return false; - } - // check receiver if ref-like if (receiver?.Type?.IsRefLikeType == true) { @@ -1828,9 +1800,6 @@ private bool CheckInvocationArgMixing( } } - //NB: we do not care about unmatched "in" parameters here. - // They have "outer" val escape, so cannot be worse than escapeTo. - // check val escape of receiver if ref-like if (receiverOpt?.Type?.IsRefLikeType == true) { @@ -1881,21 +1850,49 @@ void updateEscapeTo(BoundExpression argument, RefKind refKind, uint scopeOfTheCo }; } +#if DEBUG + private static bool AllParametersConsideredInEscapeAnalysisHaveArguments( + ImmutableArray argsOpt, + ImmutableArray parameters, + ImmutableArray argsToParamsOpt) + { + if (parameters.IsDefaultOrEmpty) return true; + + var paramsMatched = BitVector.Create(parameters.Length); + for (int argIndex = 0; argIndex < argsOpt.Length; argIndex++) + { + int paramIndex = argsToParamsOpt.IsDefault ? argIndex : argsToParamsOpt[argIndex]; + paramsMatched[paramIndex] = true; + } + for (int paramIndex = 0; paramIndex < parameters.Length; paramIndex++) + { + if (!paramsMatched[paramIndex]) + { + // Default arguments for params arrays are not created during + // binding (see https://github.com/dotnet/roslyn/issues/49602), + // but a params array cannot contain references or ref structs. + if (parameters[paramIndex] is not { IsParams: true, Type.TypeKind: TypeKind.Array }) + { + return false; + } + } + } + return true; + } +#endif + /// - /// Gets "effective" ref kind of an argument. - /// If the ref kind is 'in', marks that that corresponding parameter was matched with a value - /// We need that to detect when there were optional 'in' parameters for which values were not supplied. + /// Gets "effective" ref kind of an argument. /// /// NOTE: Generally we know if a formal argument is passed as ref/out/in by looking at the call site. /// However, 'in' may also be passed as an ordinary val argument so we need to take a look at corresponding parameter, if such exists. /// There are cases like params/vararg, when a corresponding parameter may not exist, then val cannot become 'in'. /// - private static RefKind GetEffectiveRefKindAndMarkMatchedInParameter( + private static RefKind GetEffectiveRefKind( int argIndex, ImmutableArray argRefKindsOpt, ImmutableArray parameters, ImmutableArray argsToParamsOpt, - ref ArrayBuilder? inParametersMatchedWithArgs, out DeclarationScope scope) { int paramIndex = argIndex < parameters.Length ? @@ -1909,8 +1906,6 @@ private static RefKind GetEffectiveRefKindAndMarkMatchedInParameter( if (parameters[paramIndex].RefKind == RefKind.In) { effectiveRefKind = RefKind.In; - inParametersMatchedWithArgs = inParametersMatchedWithArgs ?? ArrayBuilder.GetInstance(parameters.Length, fillWithValue: false); - inParametersMatchedWithArgs[paramIndex] = true; } } @@ -1918,44 +1913,6 @@ private static RefKind GetEffectiveRefKindAndMarkMatchedInParameter( return effectiveRefKind; } - /// - /// Gets a "in" parameter for which there is no argument supplied, if such exists. - /// That indicates an optional "in" parameter. We treat it as an RValue passed by reference via a temporary. - /// The effective scope of such variable is the immediately containing scope. - /// - private static ParameterSymbol? TryGetUnmatchedInParameterAndFreeMatchedArgs(ImmutableArray parameters, ref ArrayBuilder? inParametersMatchedWithArgs) - { - try - { - if (!parameters.IsDefault) - { - for (int i = 0; i < parameters.Length; i++) - { - var parameter = parameters[i]; - if (parameter.IsParams) - { - break; - } - - if (parameter.RefKind == RefKind.In && - inParametersMatchedWithArgs?[i] != true && - parameter.Type.IsRefLikeType == false) - { - return parameter; - } - } - } - - return null; - } - finally - { - inParametersMatchedWithArgs?.Free(); - // make sure noone uses it after. - inParametersMatchedWithArgs = null; - } - } - #nullable disable private static ErrorCode GetStandardCallEscapeError(bool checkingReceiver) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/RefFieldTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/RefFieldTests.cs index e555a0fbcb75b..7e0d4384a781e 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/RefFieldTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/RefFieldTests.cs @@ -2336,6 +2336,49 @@ class Program Diagnostic(ErrorCode.ERR_EscapeVariable, "x").WithArguments("R").WithLocation(16, 71)); } + [Fact] + public void MethodInvocation_Params() + { + var source = +@"ref struct R +{ + public R(ref int i) { } +} +class Program +{ + static R M1(R input, params object[] array) + { + return input; + } + static R M2() + { + int i = 42; + var r = new R(ref i); + return M1(r); + } + static R M3() + { + int i = 42; + var r = new R(ref i); + return M1(r, 0, 1, 2); + } +}"; + var comp = CreateCompilation(source); + comp.VerifyEmitDiagnostics( + // (15,16): error CS8347: Cannot use a result of 'Program.M1(R, params object[])' in this context because it may expose variables referenced by parameter 'input' outside of their declaration scope + // return M1(r); + Diagnostic(ErrorCode.ERR_EscapeCall, "M1(r)").WithArguments("Program.M1(R, params object[])", "input").WithLocation(15, 16), + // (15,19): error CS8352: Cannot use variable 'r' in this context because it may expose referenced variables outside of their declaration scope + // return M1(r); + Diagnostic(ErrorCode.ERR_EscapeVariable, "r").WithArguments("r").WithLocation(15, 19), + // (21,16): error CS8347: Cannot use a result of 'Program.M1(R, params object[])' in this context because it may expose variables referenced by parameter 'input' outside of their declaration scope + // return M1(r, 0, 1, 2); + Diagnostic(ErrorCode.ERR_EscapeCall, "M1(r, 0, 1, 2)").WithArguments("Program.M1(R, params object[])", "input").WithLocation(21, 16), + // (21,19): error CS8352: Cannot use variable 'r' in this context because it may expose referenced variables outside of their declaration scope + // return M1(r, 0, 1, 2); + Diagnostic(ErrorCode.ERR_EscapeVariable, "r").WithArguments("r").WithLocation(21, 19)); + } + [Fact] public void MethodArgumentsMustMatch_01() {