Skip to content

Commit

Permalink
Fix a crash caused by an improper reuse of lambdas from return infere…
Browse files Browse the repository at this point in the history
  • Loading branch information
AlekseyTs committed Dec 27, 2016
1 parent 27339fd commit 752d181
Show file tree
Hide file tree
Showing 8 changed files with 188 additions and 114 deletions.
172 changes: 113 additions & 59 deletions src/Compilers/CSharp/Portable/BoundTree/UnboundLambda.cs

Large diffs are not rendered by default.

Expand Up @@ -274,9 +274,6 @@ private static Binder GetEnclosingBinder(SyntaxNode node, int position, Binder r
{
binder = new ExecutableCodeBinder(unexpectedAnonymousFunction,
new LambdaSymbol(binder.ContainingMemberOrLambda,
ImmutableArray<ParameterSymbol>.Empty,
RefKind.None,
ErrorTypeSymbol.UnknownResultType,
unexpectedAnonymousFunction.Kind() == SyntaxKind.AnonymousMethodExpression ? MessageID.IDS_AnonMethod : MessageID.IDS_Lambda,
unexpectedAnonymousFunction,
isSynthesized: false),
Expand Down
Expand Up @@ -963,12 +963,6 @@ public BoundExpression Null(TypeSymbol type)

public BoundTypeExpression Type(TypeSymbol type)
{
// This is an attempt to get a repro for https://devdiv.visualstudio.com/DevDiv/_workitems?id=278481
if ((object)type == null)
{
throw ExceptionUtilities.Unreachable;
}

return new BoundTypeExpression(Syntax, null, type) { WasCompilerGenerated = true };
}

Expand Down
15 changes: 0 additions & 15 deletions src/Compilers/CSharp/Portable/Symbols/MemberSignatureComparer.cs
Expand Up @@ -262,21 +262,6 @@ internal class MemberSignatureComparer : IEqualityComparer<Symbol>
considerRefOutDifference: true,
considerCustomModifiers: false);

/// <summary>
/// This instance is used as a key in the lambda return type inference.
/// We basically only interested in parameters since inference will set the return type to null.
/// </summary>
public static readonly MemberSignatureComparer LambdaReturnInferenceCacheComparer = new MemberSignatureComparer(
considerName: false, // valid invoke is always called "Invoke"
considerExplicitlyImplementedInterfaces: false,
considerReturnType: true, // to differentiate Task types
considerTypeConstraints: false, // valid invoke is never generic
considerCallingConvention: false, // valid invoke is never static
considerRefOutDifference: true,
considerCustomModifiers: true,
ignoreDynamic: false,
ignoreTupleNames: false);

// Compare the "unqualified" part of the member name (no explicit part)
private readonly bool _considerName;

Expand Down
50 changes: 26 additions & 24 deletions src/Compilers/CSharp/Portable/Symbols/Source/LambdaSymbol.cs
Expand Up @@ -18,11 +18,17 @@ internal sealed class LambdaSymbol : MethodSymbol
private readonly bool _isSynthesized;
private readonly bool _isAsync;

/// <summary>
/// This symbol is used as the return type of a LambdaSymbol when we failed to infer its return type.
/// </summary>
internal static readonly TypeSymbol InferenceFailureReturnType = new UnsupportedMetadataTypeSymbol();

public LambdaSymbol(
CSharpCompilation compilation,
Symbol containingSymbol,
UnboundLambda unboundLambda,
ImmutableArray<ParameterSymbol> delegateParameters,
ImmutableArray<TypeSymbol> parameterTypes,
ImmutableArray<RefKind> parameterRefKinds,
RefKind refKind,
TypeSymbol returnType)
{
Expand All @@ -34,25 +40,22 @@ internal sealed class LambdaSymbol : MethodSymbol
_isSynthesized = unboundLambda.WasCompilerGenerated;
_isAsync = unboundLambda.IsAsync;
// No point in making this lazy. We are always going to need these soon after creation of the symbol.
_parameters = MakeParameters(compilation, unboundLambda, delegateParameters);
_parameters = MakeParameters(compilation, unboundLambda, parameterTypes, parameterRefKinds);
}

public LambdaSymbol(
Symbol containingSymbol,
ImmutableArray<ParameterSymbol> parameters,
RefKind refKind,
TypeSymbol returnType,
MessageID messageID,
SyntaxNode syntax,
bool isSynthesized)
{
_containingSymbol = containingSymbol;
_messageID = messageID;
_syntax = syntax;
_refKind = refKind;
_returnType = returnType;
_refKind = RefKind.None;
_returnType = ErrorTypeSymbol.UnknownResultType;
_isSynthesized = isSynthesized;
_parameters = parameters.SelectAsArray(CopyParameter, this);
_parameters = ImmutableArray<ParameterSymbol>.Empty;
}

public MessageID MessageID { get { return _messageID; } }
Expand Down Expand Up @@ -287,17 +290,27 @@ public override bool HidesBaseMethodsByName
private ImmutableArray<ParameterSymbol> MakeParameters(
CSharpCompilation compilation,
UnboundLambda unboundLambda,
ImmutableArray<ParameterSymbol> delegateParameters)
ImmutableArray<TypeSymbol> parameterTypes,
ImmutableArray<RefKind> parameterRefKinds)
{
Debug.Assert(parameterTypes.Length == parameterRefKinds.Length);

if (!unboundLambda.HasSignature || unboundLambda.ParameterCount == 0)
{
// The parameters may be omitted in source, but they are still present on the symbol.
return delegateParameters.SelectAsArray(CopyParameter, this);
return parameterTypes.SelectAsArray((type, ordinal, arg) =>
SynthesizedParameterSymbol.Create(
arg.owner,
type,
ordinal,
arg.refKinds[ordinal],
GeneratedNames.LambdaCopyParameterName(ordinal)), // Make sure nothing binds to this.
(owner: this, refKinds: parameterRefKinds));
}

var builder = ArrayBuilder<ParameterSymbol>.GetInstance();
var hasExplicitlyTypedParameterList = unboundLambda.HasExplicitlyTypedParameterList;
var numDelegateParameters = delegateParameters.IsDefault ? 0 : delegateParameters.Length;
var numDelegateParameters = parameterTypes.Length;

for (int p = 0; p < unboundLambda.ParameterCount; ++p)
{
Expand All @@ -316,9 +329,8 @@ public override bool HidesBaseMethodsByName
}
else if (p < numDelegateParameters)
{
ParameterSymbol delegateParameter = delegateParameters[p];
type = delegateParameter.Type;
refKind = delegateParameter.RefKind;
type = parameterTypes[p];
refKind = parameterRefKinds[p];
}
else
{
Expand All @@ -339,16 +351,6 @@ public override bool HidesBaseMethodsByName
return result;
}

private static ParameterSymbol CopyParameter(ParameterSymbol parameter, MethodSymbol owner)
{
return SynthesizedParameterSymbol.Create(
owner,
parameter.Type,
parameter.Ordinal,
parameter.RefKind,
GeneratedNames.LambdaCopyParameterName(parameter)); // Make sure nothing binds to this.
}

public sealed override bool Equals(object symbol)
{
if ((object)this == symbol) return true;
Expand Down
Expand Up @@ -510,9 +510,9 @@ internal static string ReusableHoistedLocalFieldName(int number)
return "<>7__wrap" + StringExtensions.GetNumeral(number);
}

internal static string LambdaCopyParameterName(ParameterSymbol sourceParameter)
internal static string LambdaCopyParameterName(int ordinal)
{
return "<" + sourceParameter.Name + ">";
return "<p" + StringExtensions.GetNumeral(ordinal) + ">";
}
}
}
Expand Up @@ -1594,15 +1594,15 @@ public static void Main()
Assert.Null(GetSymbolNamesJoined(analysis.AlwaysAssigned));
Assert.Equal("p", GetSymbolNamesJoined(analysis.Captured));
Assert.Equal("i", GetSymbolNamesJoined(analysis.UnsafeAddressTaken));
Assert.Equal("<p>", GetSymbolNamesJoined(analysis.VariablesDeclared));
Assert.Equal("<p0>", GetSymbolNamesJoined(analysis.VariablesDeclared));

Assert.Equal("p", GetSymbolNamesJoined(analysis.DataFlowsIn));
Assert.Null(GetSymbolNamesJoined(analysis.DataFlowsOut));

Assert.Equal("p", GetSymbolNamesJoined(analysis.ReadInside));
Assert.Equal("i", GetSymbolNamesJoined(analysis.ReadOutside));

Assert.Equal("<p>", GetSymbolNamesJoined(analysis.WrittenInside));
Assert.Equal("<p0>", GetSymbolNamesJoined(analysis.WrittenInside));
Assert.Equal("i, p, d", GetSymbolNamesJoined(analysis.WrittenOutside));
}

Expand Down
48 changes: 45 additions & 3 deletions src/Compilers/CSharp/Test/Semantic/Semantics/LambdaTests.cs
Expand Up @@ -1638,8 +1638,8 @@ static void Main(string[] args)

var lambdaParameters = ((MethodSymbol)(model.GetSymbolInfo(node1)).Symbol).Parameters;

Assert.Equal("System.Object <sender>", lambdaParameters[0].ToTestDisplayString());
Assert.Equal("System.EventArgs <e>", lambdaParameters[1].ToTestDisplayString());
Assert.Equal("System.Object <p0>", lambdaParameters[0].ToTestDisplayString());
Assert.Equal("System.EventArgs <p1>", lambdaParameters[1].ToTestDisplayString());

CompileAndVerify(compilation);
}
Expand Down Expand Up @@ -2292,7 +2292,7 @@ public static class C
}

[Fact, WorkItem(278481, "https://devdiv.visualstudio.com/DevDiv/_workitems?id=278481")]
public void LambdaReturningNull()
public void LambdaReturningNull_1()
{
var src = @"
public static class ExtensionMethods
Expand All @@ -2319,6 +2319,18 @@ public static class ExtensionMethods
return null;
}
public static System.Collections.Generic.IEnumerable<TResult> LeftOuterJoin<TOuter, TInner, TKey, TResult>(
this System.Collections.Generic.IEnumerable<TOuter> outerValues,
System.Linq.IQueryable<TInner> innerValues,
System.Func<TOuter, TKey> outerKeySelector,
System.Func<TInner, TKey> innerKeySelector,
System.Func<TOuter, TInner, TResult> fullResultSelector,
System.Func<TOuter, TResult> partialResultSelector)
{
System.Console.WriteLine(""2"");
return null;
}
public static System.Collections.Generic.IEnumerable<TResult> LeftOuterJoin<TOuter, TInner, TKey, TResult>(
this System.Collections.Generic.IEnumerable<TOuter> outerQueryable,
System.Collections.Generic.IEnumerable<TInner> innerQueryable,
Expand Down Expand Up @@ -2356,6 +2368,36 @@ class B
CompileAndVerify(comp, expectedOutput: "1");
}

[Fact, WorkItem(296550, "https://devdiv.visualstudio.com/DevDiv/_workitems?id=296550")]
public void LambdaReturningNull_2()
{
var src = @"
class Test1<T>
{
public void M1(System.Func<T> x) {}
public void M1<S>(System.Func<S> x) {}
public void M2<S>(System.Func<S> x) {}
public void M2(System.Func<T> x) {}
}
class Test2 : Test1<System.>
{
void Main()
{
M1(()=> null);
M2(()=> null);
}
}
";
var comp = CreateCompilationWithMscorlib(src, options: TestOptions.DebugDll);

comp.VerifyDiagnostics(
// (10,32): error CS1001: Identifier expected
// class Test2 : Test1<System.>
Diagnostic(ErrorCode.ERR_IdentifierExpected, ">").WithLocation(10, 32)
);
}

[Fact]
public void ThrowExpression_Lambda()
{
Expand Down

0 comments on commit 752d181

Please sign in to comment.