Skip to content

Commit

Permalink
Merge remote-tracking branch 'dotnet/master' into delegate-creation
Browse files Browse the repository at this point in the history 
* dotnet/master:
  Report binding error for typed LINQ query on a type expression (dotnet#22412)
  clean up
  more refactoring
  Fixed tests
  fix test after rebase
  one more test
  CR feedback
  Allow taking unmanaged/pinned addresses of readonly variables.
  Fix typo in INotifyCompletion API in doc (dotnet#22417)
  Fix serialization of attribute data to account for named params argument (dotnet#22231)
  Create tests to cover test plans dotnet#19216 and dotnet#20127 (dotnet#22284)
  Checked uses of RefKind.RefReadOnly
  String rename
  Check for null before registering incremental analyzer in work coordinator.
  clean up
  Create tests to cover test plans dotnet#19216 and dotnet#20127
  • Loading branch information
@333fred
333fred committed Sep 29, 2017
2 parents 9175bf1 + 1fd9a04 commit 3460d12
Show file tree
Hide file tree
Showing 90 changed files with 2,922 additions and 1,560 deletions.
2 changes: 1 addition & 1 deletion docs/features/task-types.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class Awaiter<T> : INotifyCompletion
{
public bool IsCompleted { get; }
public T GetResult();
public void OnCompletion(Action completion);
public void OnCompleted(Action completion);
}
```
## Builder Type
Expand Down
50 changes: 23 additions & 27 deletions src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,27 +99,27 @@ internal enum BindValueKind : byte
ReadonlyRef = RefersToLocation | RValue,

/// <summary>
/// Expression is passed as a ref or out parameter or assigned to a byref variable.
/// Expression can be the operand of an address-of operation (&amp;).
/// Same as ReadonlyRef. The difference is just for error reporting.
/// </summary>
RefOrOut = RefersToLocation | RValue | Assignable,
AddressOf = ReadonlyRef + 1,

/// <summary>
/// Expression can be the operand of an address-of operation (&amp;).
/// Same as RefOrOut. The difference is just for error reporting.
/// Expression is the receiver of a fixed buffer field access
/// Same as ReadonlyRef. The difference is just for error reporting.
/// </summary>
AddressOf = RefOrOut + 1,
FixedReceiver = ReadonlyRef + 2,

/// <summary>
/// Expression is the receiver of a fixed buffer field access
/// Same as RefOrOut. The difference is just for error reporting.
/// Expression is passed as a ref or out parameter or assigned to a byref variable.
/// </summary>
FixedReceiver = RefOrOut + 2,
RefOrOut = RefersToLocation | RValue | Assignable,

/// <summary>
/// Expression is returned by an ordinary r/w reference.
/// Same as RefOrOut. The difference is just for error reporting.
/// </summary>
RefReturn = RefOrOut + 3,
RefReturn = RefOrOut + 1,
}

private static bool RequiresRValueOnly(BindValueKind kind)
Expand Down Expand Up @@ -395,7 +395,9 @@ internal bool CheckValueKind(SyntaxNode node, BoundExpression expr, BindValueKin
// Note: RValueOnly is checked at the beginning of this method. Since we are here we need more than readable.
//"this" is readonly in members of readonly structs, unless we are in a constructor.
if (!thisref.Type.IsValueType ||
(thisref.Type.IsReadOnly && (this.ContainingMemberOrLambda as MethodSymbol)?.MethodKind != MethodKind.Constructor))
(RequiresAssignableVariable(valueKind) &&
thisref.Type.IsReadOnly &&
(this.ContainingMemberOrLambda as MethodSymbol)?.MethodKind != MethodKind.Constructor))
{
// CONSIDER: the Dev10 name has angle brackets (i.e. "<this>")
Error(diagnostics, GetThisLvalueError(valueKind), node, ThisParameterSymbol.SymbolName);
Expand Down Expand Up @@ -518,7 +520,7 @@ private bool CheckParameterValueKind(SyntaxNode node, BoundParameter parameter,

// all parameters can be passed by ref/out or assigned to
// except "in" parameters, which are readonly
if (parameterSymbol.RefKind == RefKind.RefReadOnly && RequiresAssignableVariable(valueKind))
if (parameterSymbol.RefKind == RefKind.In && RequiresAssignableVariable(valueKind))
{
ReportReadOnlyError(parameterSymbol, node, valueKind, checkingReceiver, diagnostics);
return false;
Expand Down Expand Up @@ -1010,7 +1012,7 @@ bool isRefEscape
}

// handle omitted optional "in" parameters if there are any
ParameterSymbol unmatchedInParameter = TryGetUnmatchedInParameterAndFreeMatchedArgs(parameters, ref inParametersMatchedWithArgs);
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 overal result)
Expand Down Expand Up @@ -1125,7 +1127,7 @@ bool isRefEscape
}

// handle omitted optional "in" parameters if there are any
ParameterSymbol unmatchedInParameter = TryGetUnmatchedInParameterAndFreeMatchedArgs(parameters, ref inParametersMatchedWithArgs);
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 overal result)
Expand Down Expand Up @@ -1261,7 +1263,7 @@ private static bool CheckInvocationArgMixing(
}
}

//NB: we do not care about unmatched "ref readonly" parameters here.
//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
Expand All @@ -1277,7 +1279,7 @@ private static bool CheckInvocationArgMixing(
/// Gets "effective" ref kind of an argument.
/// Generally we know if a formal argument is passed as ref/out by looking at the call site.
/// However, to distinguish "in" and regular "val" parameters we need to take a look at corresponding parameter, if such exists.
/// NOTE: there are cases like params/vararg, when a corresponding parameter may not exist, then it cannot be an "in".
/// NOTE: there are cases like params/vararg, when a corresponding parameter may not exist, then it cannot be "in".
/// </summary>
private static RefKind GetEffectiveRefKind(
int argIndex,
Expand All @@ -1291,9 +1293,9 @@ private static RefKind GetEffectiveRefKind(
{
var paramIndex = argsToParamsOpt.IsDefault ? argIndex : argsToParamsOpt[argIndex];

if (parameters[paramIndex].RefKind == RefKind.RefReadOnly)
if (parameters[paramIndex].RefKind == RefKind.In)
{
effectiveRefKind = RefKind.RefReadOnly;
effectiveRefKind = RefKind.In;
inParametersMatchedWithArgs = inParametersMatchedWithArgs ?? ArrayBuilder<bool>.GetInstance(parameters.Length, fillWithValue: false);
inParametersMatchedWithArgs[paramIndex] = true;
}
Expand All @@ -1303,11 +1305,11 @@ private static RefKind GetEffectiveRefKind(
}

/// <summary>
/// Gets an "in" parameter for which there is no argument supplied, if such exists.
/// 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.
/// </summary>
private static ParameterSymbol TryGetUnmatchedInParameterAndFreeMatchedArgs(ImmutableArray<ParameterSymbol> parameters, ref ArrayBuilder<bool> inParametersMatchedWithArgs)
private static ParameterSymbol TryGetunmatchedInParameterAndFreeMatchedArgs(ImmutableArray<ParameterSymbol> parameters, ref ArrayBuilder<bool> inParametersMatchedWithArgs)
{
try
{
Expand All @@ -1321,7 +1323,7 @@ private static ParameterSymbol TryGetUnmatchedInParameterAndFreeMatchedArgs(Immu
break;
}

if (parameter.RefKind == RefKind.RefReadOnly &&
if (parameter.RefKind == RefKind.In &&
inParametersMatchedWithArgs?[i] != true &&
parameter.Type.IsByRefLikeType == false)
{
Expand Down Expand Up @@ -1369,12 +1371,6 @@ private static void ReportReadonlyLocalError(SyntaxNode node, LocalSymbol local,
return;
}

if (kind == BindValueKind.AddressOf)
{
Error(diagnostics, ErrorCode.ERR_AddrOnReadOnlyLocal, node);
return;
}

ErrorCode[] ReadOnlyLocalErrors =
{
ErrorCode.ERR_RefReadonlyLocalCause,
Expand All @@ -1401,7 +1397,7 @@ static private ErrorCode GetThisLvalueError(BindValueKind kind)
return ErrorCode.ERR_RefReadonlyLocal;

case BindValueKind.AddressOf:
return ErrorCode.ERR_AddrOnReadOnlyLocal;
return ErrorCode.ERR_InvalidAddrOp;

case BindValueKind.IncrementDecrement:
return ErrorCode.ERR_IncrementLvalueExpected;
Expand Down
88 changes: 66 additions & 22 deletions src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -578,8 +578,12 @@ private ImmutableArray<TypedConstant> GetRewrittenAttributeConstructorArguments(

if (parameter.IsParams && parameter.Type.IsSZArray() && i + 1 == parameterCount)
{
reorderedArgument = GetParamArrayArgument(parameter, constructorArgsArray, argumentsCount, argsConsumedCount, this.Conversions);
sourceIndices = sourceIndices ?? CreateSourceIndicesArray(i, parameterCount);
reorderedArgument = GetParamArrayArgument(parameter, constructorArgsArray, constructorArgumentNamesOpt, argumentsCount,
argsConsumedCount, this.Conversions, out bool foundNamed);
if (!foundNamed)
{
sourceIndices = sourceIndices ?? CreateSourceIndicesArray(i, parameterCount);
}
}
else if (argsConsumedCount < argumentsCount)
{
Expand Down Expand Up @@ -820,41 +824,48 @@ private TypedConstant GetDefaultValueArgument(ParameterSymbol parameter, Attribu
}
}

private static TypedConstant GetParamArrayArgument(ParameterSymbol parameter, ImmutableArray<TypedConstant> constructorArgsArray, int argumentsCount, int argsConsumedCount, Conversions conversions)
private static TypedConstant GetParamArrayArgument(ParameterSymbol parameter, ImmutableArray<TypedConstant> constructorArgsArray,
ImmutableArray<string> constructorArgumentNamesOpt, int argumentsCount, int argsConsumedCount, Conversions conversions, out bool foundNamed)
{
Debug.Assert(argsConsumedCount <= argumentsCount);

// If there's a named argument, we'll use that
if (!constructorArgumentNamesOpt.IsDefault)
{
int argIndex = constructorArgumentNamesOpt.IndexOf(parameter.Name);
if (argIndex >= 0)
{
foundNamed = true;
if (TryGetNormalParamValue(parameter, constructorArgsArray, argIndex, conversions, out var namedValue))
{
return namedValue;
}

// A named argument for a params parameter is necessarily the only one for that parameter
return new TypedConstant(parameter.Type, ImmutableArray.Create(constructorArgsArray[argIndex]));
}
}

int paramArrayArgCount = argumentsCount - argsConsumedCount;
foundNamed = false;

// If there are zero arguments left
if (paramArrayArgCount == 0)
{
return new TypedConstant(parameter.Type, ImmutableArray<TypedConstant>.Empty);
}

// If there's exactly one argument and it's an array of an appropriate type, then just return it.
if (paramArrayArgCount == 1 && constructorArgsArray[argsConsumedCount].Kind == TypedConstantKind.Array)
// If there's exactly one argument left, we'll try to use it in normal form
if (paramArrayArgCount == 1 &&
TryGetNormalParamValue(parameter, constructorArgsArray, argsConsumedCount, conversions, out var lastValue))
{
TypeSymbol argumentType = (TypeSymbol)constructorArgsArray[argsConsumedCount].Type;

// Easy out (i.e. don't both classifying conversion).
if (argumentType == parameter.Type)
{
return constructorArgsArray[argsConsumedCount];
}

HashSet<DiagnosticInfo> useSiteDiagnostics = null; // ignoring, since already bound argument and parameter
Conversion conversion = conversions.ClassifyBuiltInConversion(argumentType, parameter.Type, ref useSiteDiagnostics);

// NOTE: Won't always succeed, even though we've performed overload resolution.
// For example, passing int[] to params object[] actually treats the int[] as an element of the object[].
if (conversion.IsValid && conversion.Kind == ConversionKind.ImplicitReference)
{
return constructorArgsArray[argsConsumedCount];
}
return lastValue;
}

Debug.Assert(!constructorArgsArray.IsDefault);
Debug.Assert(argsConsumedCount <= constructorArgsArray.Length);

// Take the trailing arguments as an array for expanded form
var values = new TypedConstant[paramArrayArgCount];

for (int i = 0; i < paramArrayArgCount; i++)
Expand All @@ -865,6 +876,39 @@ private static TypedConstant GetParamArrayArgument(ParameterSymbol parameter, Im
return new TypedConstant(parameter.Type, values.AsImmutableOrNull());
}

private static bool TryGetNormalParamValue(ParameterSymbol parameter, ImmutableArray<TypedConstant> constructorArgsArray,
int argIndex, Conversions conversions, out TypedConstant result)
{
TypedConstant argument = constructorArgsArray[argIndex];
if (argument.Kind != TypedConstantKind.Array)
{
result = default;
return false;
}

TypeSymbol argumentType = (TypeSymbol)argument.Type;
// Easy out (i.e. don't bother classifying conversion).
if (argumentType == parameter.Type)
{
result = argument;
return true;
}

HashSet<DiagnosticInfo> useSiteDiagnostics = null; // ignoring, since already bound argument and parameter
Conversion conversion = conversions.ClassifyBuiltInConversion(argumentType, parameter.Type, ref useSiteDiagnostics);

// NOTE: Won't always succeed, even though we've performed overload resolution.
// For example, passing int[] to params object[] actually treats the int[] as an element of the object[].
if (conversion.IsValid && conversion.Kind == ConversionKind.ImplicitReference)
{
result = argument;
return true;
}

result = default;
return false;
}

#endregion

#region AttributeExpressionVisitor
Expand Down
14 changes: 11 additions & 3 deletions src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -584,10 +584,18 @@ private bool MemberGroupFinalValidationAccessibilityChecks(BoundExpression recei

if (invokedAsExtensionMethod)
{
if (receiverOpt?.Kind == BoundKind.QueryClause && IsMemberAccessedThroughType(receiverOpt))
if (IsMemberAccessedThroughType(receiverOpt))
{
// Could not find an implementation of the query pattern for source type '{0}'. '{1}' not found.
diagnostics.Add(ErrorCode.ERR_QueryNoProvider, node.Location, receiverOpt.Type, memberSymbol.Name);
if (receiverOpt.Kind == BoundKind.QueryClause)
{
// Could not find an implementation of the query pattern for source type '{0}'. '{1}' not found.
diagnostics.Add(ErrorCode.ERR_QueryNoProvider, node.Location, receiverOpt.Type, memberSymbol.Name);
}
else
{
// An object reference is required for the non-static field, method, or property '{0}'
diagnostics.Add(ErrorCode.ERR_ObjectRequired, node.Location, memberSymbol);
}
return true;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -980,7 +980,7 @@ private BoundCall BindInvocationExpressionContinued(

CheckFeatureAvailability(receiverArgument.Syntax, MessageID.IDS_FeatureRefExtensionMethods, diagnostics);
}
else if (receiverParameter.RefKind == RefKind.RefReadOnly)
else if (receiverParameter.RefKind == RefKind.In)
{
CheckFeatureAvailability(receiverArgument.Syntax, MessageID.IDS_FeatureRefExtensionMethods, diagnostics);
}
Expand Down
7 changes: 2 additions & 5 deletions src/Compilers/CSharp/Portable/Binder/Binder_Lambda.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,6 @@ private Tuple<ImmutableArray<RefKind>, ImmutableArray<TypeSymbol>, ImmutableArra
type = BindType(typeSyntax, diagnostics);
foreach (var modifier in p.Modifiers)
{
var modKind = modifier.Kind();

switch (modifier.Kind())
{
case SyntaxKind.RefKeyword:
Expand All @@ -139,9 +137,8 @@ private Tuple<ImmutableArray<RefKind>, ImmutableArray<TypeSymbol>, ImmutableArra
allValue = false;
break;

case SyntaxKind.ReadOnlyKeyword:
Debug.Assert(refKind == RefKind.Ref || syntax.HasErrors);
refKind = RefKind.RefReadOnly;
case SyntaxKind.InKeyword:
refKind = RefKind.In;
allValue = false;
break;

Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1710,7 +1710,7 @@ internal void GenerateAnonymousFunctionConversionError(DiagnosticBag diagnostics

// Parameter {0} is declared as type '{1}{2}' but should be '{3}{4}'
Error(diagnostics, ErrorCode.ERR_BadParamType, lambdaParameterLocation,
i + 1, lambdaRefKind.ToPrefix(), distinguisher.First, delegateRefKind.ToPrefix(), distinguisher.Second);
i + 1, lambdaRefKind.ToParameterPrefix(), distinguisher.First, delegateRefKind.ToParameterPrefix(), distinguisher.Second);
}
else if (lambdaRefKind != delegateRefKind)
{
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2575,7 +2575,7 @@ private RefKind GetEffectiveParameterRefKind(ParameterSymbol parameter, RefKind
{
var paramRefKind = parameter.RefKind;

if (paramRefKind == RefKind.RefReadOnly)
if (paramRefKind == RefKind.In)
{
// "in" parameters are effectively None for the purpose of overload resolution.
paramRefKind = RefKind.None;
Expand Down
Loading

0 comments on commit 3460d12

Please sign in to comment.