Skip to content

Commit

Permalink
Add a number of PROTOTYPE comments and fix formatting of throw expres…
Browse files Browse the repository at this point in the history 
…sions in diagnostics.
  • Loading branch information
@gafter
gafter committed Mar 21, 2016
1 parent 737cde5 commit 2772c43
Show file tree
Hide file tree
Showing 9 changed files with 82 additions and 63 deletions.
14 changes: 1 addition & 13 deletions src/Compilers/CSharp/Portable/Binder/Binder_AnonymousTypes.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,19 +192,7 @@ private TypeSymbol GetAnonymousTypeFieldType(BoundExpression expression, CSharpS
}
else
{
if (expression.Kind == BoundKind.UnboundLambda)
{
errorArg = ((UnboundLambda)expression).MessageID.Localize();
}
else if (expression.Kind == BoundKind.MethodGroup)
{
errorArg = MessageID.IDS_MethodGroup.Localize();
}
else
{
Debug.Assert(expression.IsLiteralNull(), "How did we successfully bind an expression without a type?");
errorArg = MessageID.IDS_NULL.Localize();
}
errorArg = expression.Display;
}
}

Expand Down
56 changes: 36 additions & 20 deletions src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ internal BoundPattern BindPattern(
return new BoundWildcardPattern(node);

default:
// PROTOTYPE: Can this occur due to parser error recovery? If so, how to handle?
throw ExceptionUtilities.UnexpectedValue(node.Kind());
}
}
Expand All @@ -64,11 +65,10 @@ private BoundPattern BindRecursivePattern(
var type = (NamedTypeSymbol)this.BindType(node.Type, diagnostics);
hasErrors = hasErrors || CheckValidPatternType(node.Type, operand, operandType, type, false, diagnostics);

// We intend that (positional) recursive pattern-matching should be defined in terms of
// a pattern of user-defined methods or operators. Tentatively, perhaps a method called
// GetValues that has an out parameter for each position of the recursive pattern. But
// for now we try to *infer* a positional pattern-matching operation from the presence of
// an accessible constructor.
// PROTOTYPE: We intend that (positional) recursive pattern-matching should be defined in terms of
// PROTOTYPE: a pattern of user-defined methods or operators. As currently specified it is `operator is`.
// PROTOTYPE: But for now we try to *infer* a positional pattern-matching operation from the presence of
// PROTOTYPE: an accessible constructor.
var correspondingMembers = default(ImmutableArray<Symbol>);
HashSet<DiagnosticInfo> useSiteDiagnostics = null;
var memberNames = type.MemberNames;
Expand Down Expand Up @@ -112,6 +112,7 @@ private BoundPattern BindRecursivePattern(
if (!correspondingMembersForCtor.SequenceEqual(correspondingMembers, (s1, s2) => s1 == s2))
{
correspondingMembersForCtor.Free();
// PROTOTYPE: If we decide to support this, we need a properly i18n'd diagnostic.
Error(diagnostics, ErrorCode.ERR_FeatureIsUnimplemented, node,
"cannot infer a positional pattern from conflicting constructors");
diagnostics.Add(node, useSiteDiagnostics);
Expand All @@ -124,6 +125,7 @@ private BoundPattern BindRecursivePattern(

if (correspondingMembers == null)
{
// PROTOTYPE: If we decide to support this, we need a properly i18n'd diagnostic.
Error(diagnostics, ErrorCode.ERR_FeatureIsUnimplemented, node,
"cannot infer a positional pattern from any accessible constructor");
diagnostics.Add(node, useSiteDiagnostics);
Expand Down Expand Up @@ -260,6 +262,7 @@ private BoundExpression BindPropertyPatternMember(

bool hasErrors = boundMember.HasAnyErrors || implicitReceiver.HasAnyErrors;

// PROTOTYPE: the following may be needed if we end up supporting an indexed property.
//ImmutableArray<BoundExpression> arguments = ImmutableArray<BoundExpression>.Empty;
//ImmutableArray<string> argumentNamesOpt = default(ImmutableArray<string>);
//ImmutableArray<int> argsToParamsOpt = default(ImmutableArray<int>);
Expand All @@ -273,8 +276,9 @@ private BoundExpression BindPropertyPatternMember(
break;

case BoundKind.IndexerAccess:
// TODO: Should a property pattern be capable of referencing an indexed property?
// https://github.com/dotnet/roslyn/issues/9375
// PROTOTYPE: Should a property pattern be capable of referencing an indexed property?
// PROTOTYPE: This is an open issue in the language specification
// PROTOTYPE: See https://github.com/dotnet/roslyn/issues/9375
//{
// var indexer = (BoundIndexerAccess)boundMember;
// arguments = indexer.Arguments;
Expand All @@ -286,8 +290,9 @@ private BoundExpression BindPropertyPatternMember(
//}

case BoundKind.DynamicIndexerAccess:
// TODO: Should a property pattern be capable of referencing a dynamic indexer?
// https://github.com/dotnet/roslyn/issues/9375
// PROTOTYPE: Should a property pattern be capable of referencing a dynamic indexer?
// PROTOTYPE: This is an open issue in the language specification
// PROTOTYPE: See https://github.com/dotnet/roslyn/issues/9375
//{
// var indexer = (BoundDynamicIndexerAccess)boundMember;
// arguments = indexer.Arguments;
Expand All @@ -297,8 +302,9 @@ private BoundExpression BindPropertyPatternMember(
//}

case BoundKind.EventAccess:
// TODO: Should a property pattern be capable of referencing an event?
// https://github.com/dotnet/roslyn/issues/9515
// PROTOTYPE: Should a property pattern be capable of referencing an event?
// PROTOTYPE: This is an open issue in the language specification
// PROTOTYPE: See https://github.com/dotnet/roslyn/issues/9515

default:
return BadSubpatternMemberAccess(boundMember, implicitReceiver, memberName, diagnostics, hasErrors);
Expand Down Expand Up @@ -345,14 +351,14 @@ private BoundExpression BadSubpatternMemberAccess(
break;

default:
// TODO: review and test this code path.
// PROTOTYPE: We need to review and test this code path.
// https://github.com/dotnet/roslyn/issues/9542
Error(diagnostics, ErrorCode.ERR_PropertyLacksGet, memberName, member);
break;
}
}

// TODO: review and test this code path. Is LookupResultKind.Inaccessible appropriate?
// PROTOTYPE: review and test this code path. Is LookupResultKind.Inaccessible appropriate?
// https://github.com/dotnet/roslyn/issues/9542
return ToBadExpression(boundMember, LookupResultKind.Inaccessible);
}
Expand All @@ -371,7 +377,7 @@ private BoundPattern BindConstantPattern(
hasErrors = true;
}

// TODO: check that the constant is valid for the given operand or operandType.
// PROTOTYPE: we still need to check that the constant is valid for the given operand or operandType.
return new BoundConstantPattern(node, expression, hasErrors);
}

Expand Down Expand Up @@ -411,7 +417,7 @@ private bool CheckValidPatternType(
{
case ConversionKind.Boxing:
case ConversionKind.ExplicitNullable:
case ConversionKind.ExplicitNumeric: // TODO: we should constrain this to integral?
case ConversionKind.ExplicitNumeric: // PROTOTYPE: we should constrain this to integral? Need LDM decision
case ConversionKind.ExplicitReference:
case ConversionKind.Identity:
case ConversionKind.ImplicitReference:
Expand Down Expand Up @@ -548,7 +554,7 @@ private BoundExpression BindMatchExpression(
DiagnosticBag diagnostics)
{
var expression = BindValue(node.Left, diagnostics, BindValueKind.RValue);
// TODO: any constraints on a switch expression must be enforced here. For example,
// PROTOTYPE: any constraints on a switch expression must be enforced here. For example,
// it must have a type (not be target-typed, lambda, null, etc)

var sectionBuilder = ArrayBuilder<BoundMatchCase>.GetInstance();
Expand All @@ -572,30 +578,40 @@ private BoundExpression BindThrowExpression(ThrowExpressionSyntax node, Diagnost
{
switch (node.Parent.Kind())
{
case SyntaxKind.ConditionalExpression:
case SyntaxKind.ConditionalExpression: // ?:
{
var papa = (ConditionalExpressionSyntax)node.Parent;
if (node == papa.WhenTrue || node == papa.WhenFalse) goto syntaxOk;
break;
}
case SyntaxKind.CoalesceExpression:
case SyntaxKind.CoalesceExpression: // ??
{
var papa = (BinaryExpressionSyntax)node.Parent;
if (node == papa.Right) goto syntaxOk;
break;
}
case SyntaxKind.MatchSection:
case SyntaxKind.MatchSection: // match
{
var papa = (MatchSectionSyntax)node.Parent;
if (node == papa.Expression) goto syntaxOk;
break;
}
case SyntaxKind.ArrowExpressionClause:
case SyntaxKind.ArrowExpressionClause: // =>
{
var papa = (ArrowExpressionClauseSyntax)node.Parent;
if (node == papa.Expression) goto syntaxOk;
break;
}
// PROTOTYPE: It has been suggested that we allow throw on the right of && and ||, but
// PROTOTYPE: due to the relative precedence the parser already rejects that.
//case SyntaxKind.LogicalAndExpression: // &&
//case SyntaxKind.LogicalOrExpression: // ||
// {
// // PROTOTYPE: it isn't clear what the semantics should be
// var papa = (BinaryExpressionSyntax)node.Parent;
// if (node == papa.Right) goto syntaxOk;
// break;
// }
default:
break;
}
Expand Down
16 changes: 1 addition & 15 deletions src/Compilers/CSharp/Portable/Binder/Binder_Query.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -568,21 +568,7 @@ private void ReduceLet(LetClauseSyntax let, QueryTranslationState state, Diagnos
SourceLocation errorLocation = new SourceLocation(let.SyntaxTree, new TextSpan(let.Identifier.SpanStart, let.Expression.Span.End - let.Identifier.SpanStart));
if (!yExpression.HasAnyErrors && !yExpression.HasExpressionType())
{
MessageID id = MessageID.IDS_NULL;
if (yExpression.Kind == BoundKind.UnboundLambda)
{
id = ((UnboundLambda)yExpression).MessageID;
}
else if (yExpression.Kind == BoundKind.MethodGroup)
{
id = MessageID.IDS_MethodGroup;
}
else
{
Debug.Assert(yExpression.IsLiteralNull(), "How did we successfully bind an expression without a type?");
}
Error(d, ErrorCode.ERR_QueryRangeVariableAssignedBadValue, errorLocation, id.Localize());
Error(d, ErrorCode.ERR_QueryRangeVariableAssignedBadValue, errorLocation, yExpression.Display);
yExpression = new BoundBadExpression(yExpression.Syntax, LookupResultKind.Empty, ImmutableArray<Symbol>.Empty, ImmutableArray.Create<BoundNode>(yExpression), CreateErrorType());
}
else if (!yExpression.HasAnyErrors && yExpression.Type.SpecialType == SpecialType.System_Void)
Expand Down
16 changes: 1 addition & 15 deletions src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -692,22 +692,8 @@ protected BoundExpression BindInferredVariableInitializer(DiagnosticBag diagnost
// their own and therefore cannot be the initializer of an implicitly typed local.
if (!expression.HasAnyErrors && !expression.HasExpressionType())
{
MessageID id = MessageID.IDS_NULL;
if (expression.Kind == BoundKind.UnboundLambda)
{
id = ((UnboundLambda)expression).MessageID;
}
else if (expression.Kind == BoundKind.MethodGroup)
{
id = MessageID.IDS_MethodGroup;
}
else
{
Debug.Assert(expression.IsLiteralNull(), "How did we successfully bind an expression without a type?");
}

// Cannot assign {0} to an implicitly-typed local variable
Error(diagnostics, ErrorCode.ERR_ImplicitlyTypedVariableAssignedBadValue, errorSyntax, id.Localize());
Error(diagnostics, ErrorCode.ERR_ImplicitlyTypedVariableAssignedBadValue, errorSyntax, expression.Display);
}

return expression;
Expand Down
8 changes: 8 additions & 0 deletions src/Compilers/CSharp/Portable/BoundTree/Formatting.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,4 +65,12 @@ public override object Display
get { throw ExceptionUtilities.Unreachable; }
}
}

internal sealed partial class BoundThrowExpression
{
public override object Display
{
get { return MessageID.IDS_ThrowExpression.Localize(); }
}
}
}
9 changes: 9 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.Designer.cs
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4164,6 +4164,9 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="IDS_FeatureRefLocalsReturns" xml:space="preserve">
<value>byref locals and returns</value>
</data>
<data name="IDS_ThrowExpression" xml:space="preserve">
<value>&lt;throw&gt;</value>
</data>
<data name="CompilationC" xml:space="preserve">
<value>Compilation (C#): </value>
</data>
Expand Down
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/Errors/MessageID.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ internal enum MessageID
IDS_FeatureLocalFunctions = MessageBase + 12708,

IDS_FeatureRefLocalsReturns = MessageBase + 12710,

IDS_ThrowExpression = MessageBase + 12711,
}

// Message IDs may refer to strings that need to be localized.
Expand Down
21 changes: 21 additions & 0 deletions src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7785,5 +7785,26 @@ static bool Dummy(bool x, object y, object z)
1
2");
}

[Fact]
public void ThrowName()
{
var source =
@"
class Program
{
static void Main(string[] args)
{
var y = null ?? throw null;
}
}
";
var compilation = CreateCompilationWithMscorlib45(source, options: TestOptions.DebugExe, parseOptions: patternParseOptions);
compilation.VerifyDiagnostics(
// (6,17): error CS0019: Operator '??' cannot be applied to operands of type '<null>' and '<throw>'
// var y = null ?? throw null;
Diagnostic(ErrorCode.ERR_BadBinaryOps, "null ?? throw null").WithArguments("??", "<null>", "<throw>").WithLocation(6, 17)
);
}
}
}

0 comments on commit 2772c43

Please sign in to comment.