Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parse array type in conditional operator #31050

Merged
merged 2 commits into from
Nov 13, 2018
Merged

Parse array type in conditional operator #31050

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
56b952e
Allow nullable array type in conditional operators
cston Nov 8, 2018
f1a012f
Test arrays of arrays
cston Nov 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 39 additions & 26 deletions src/Compilers/CSharp/Portable/Parser/LanguageParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4485,7 +4485,7 @@ private VariableDeclaratorSyntax ParseVariableDeclarator(
case SyntaxKind.OpenBracketToken:
bool sawNonOmittedSize;
_termState |= TerminatorState.IsPossibleEndOfVariableDeclaration;
var specifier = this.ParseArrayRankSpecifier(isArrayCreation: false, expectSizes: flags == VariableFlags.Fixed, allowQuestionToken: false, sawNonOmittedSize: out sawNonOmittedSize);
var specifier = this.ParseArrayRankSpecifier(isArrayCreation: false, expectSizes: flags == VariableFlags.Fixed, questionTokenModeOpt: null, sawNonOmittedSize: out sawNonOmittedSize);
_termState = saveTerm;
var open = specifier.OpenBracketToken;
var sizes = specifier.Sizes;
Expand Down Expand Up @@ -6175,7 +6175,6 @@ private TypeSyntax ParseTypeCore(
ParseTypeMode mode,
bool expectSizes)
{
var isOrAs = mode == ParseTypeMode.AsExpression || mode == ParseTypeMode.AfterIs;
NameOptions nameOptions;
switch (mode)
{
Expand Down Expand Up @@ -6205,31 +6204,15 @@ private TypeSyntax ParseTypeCore(
}

var type = this.ParseUnderlyingType(parentIsParameter: mode == ParseTypeMode.Parameter, options: nameOptions);
Debug.Assert(type != null);

if (this.CurrentToken.Kind == SyntaxKind.QuestionToken &&
// we do not permit nullable types in a declaration pattern
(mode != ParseTypeMode.AfterIs && mode != ParseTypeMode.AfterCase || !IsTrueIdentifier(this.PeekToken(1))))
if (this.CurrentToken.Kind == SyntaxKind.QuestionToken)
{
var resetPoint = this.GetResetPoint();
try
var question = EatNullableQualifierIfApplicable(mode);
if (question != null)
{
var question = this.EatToken();

if (isOrAs && (IsTerm() || IsPredefinedType(this.CurrentToken.Kind) || SyntaxFacts.IsAnyUnaryExpression(this.CurrentToken.Kind)))
{
this.Reset(ref resetPoint);

Debug.Assert(type != null);
return type;
}

question = CheckFeatureAvailability(question, MessageID.IDS_FeatureNullable);
type = _syntaxFactory.NullableType(type, question);
}
finally
{
this.Release(ref resetPoint);
}
}

switch (mode)
Expand Down Expand Up @@ -6262,7 +6245,7 @@ private TypeSyntax ParseTypeCore(
while (this.IsPossibleRankAndDimensionSpecifier())
{
bool unused;
var rank = this.ParseArrayRankSpecifier(mode == ParseTypeMode.ArrayCreation, expectSizes, allowQuestionToken: true, out unused);
var rank = this.ParseArrayRankSpecifier(mode == ParseTypeMode.ArrayCreation, expectSizes, questionTokenModeOpt: mode, out unused);
ranks.Add(rank);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to apply to all levels of a multidimensional array. Shouldn't it apply only to the last? That is, shouldn't we allow var z = e is int[]?[] ? x : y; (existing code)

Copy link
Member Author

@cston cston Nov 8, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We allow e is int[]?[] ? x : y and e is int[]?[]? ? x : y. The issue was we were incorrectly parsing the first case by treating the ? from the conditional as a nullable qualifier on the array type.

I've added tests for arrays of arrays. Thanks.

expectSizes = false;
}
Expand All @@ -6279,6 +6262,36 @@ private TypeSyntax ParseTypeCore(
return type;
}

private SyntaxToken EatNullableQualifierIfApplicable(ParseTypeMode mode)
{
Debug.Assert(this.CurrentToken.Kind == SyntaxKind.QuestionToken);

// we do not permit nullable types in a declaration pattern
if (mode != ParseTypeMode.AfterIs && mode != ParseTypeMode.AfterCase || !IsTrueIdentifier(this.PeekToken(1)))
{
var resetPoint = this.GetResetPoint();
try
{
var question = this.EatToken();

var isOrAs = mode == ParseTypeMode.AsExpression || mode == ParseTypeMode.AfterIs;
if (isOrAs && (IsTerm() || IsPredefinedType(this.CurrentToken.Kind) || SyntaxFacts.IsAnyUnaryExpression(this.CurrentToken.Kind)))
{
this.Reset(ref resetPoint);
return null;
}

return CheckFeatureAvailability(question, MessageID.IDS_FeatureNullable);
}
finally
{
this.Release(ref resetPoint);
}
}

return null;
}

private bool PointerTypeModsFollowedByRankAndDimensionSpecifier()
{
// Are pointer specifiers (if any) followed by an array specifier?
Expand All @@ -6301,7 +6314,7 @@ private bool IsPossibleRankAndDimensionSpecifier()
return this.CurrentToken.Kind == SyntaxKind.OpenBracketToken;
}

private ArrayRankSpecifierSyntax ParseArrayRankSpecifier(bool isArrayCreation, bool expectSizes, bool allowQuestionToken, out bool sawNonOmittedSize)
private ArrayRankSpecifierSyntax ParseArrayRankSpecifier(bool isArrayCreation, bool expectSizes, ParseTypeMode? questionTokenModeOpt, out bool sawNonOmittedSize)
{
sawNonOmittedSize = false;
bool sawOmittedSize = false;
Expand Down Expand Up @@ -6368,9 +6381,9 @@ private ArrayRankSpecifierSyntax ParseArrayRankSpecifier(bool isArrayCreation, b
var close = this.EatToken(SyntaxKind.CloseBracketToken);

SyntaxToken questionToken = null;
if (allowQuestionToken && this.CurrentToken.Kind == SyntaxKind.QuestionToken)
if (questionTokenModeOpt != null && this.CurrentToken.Kind == SyntaxKind.QuestionToken)
{
questionToken = this.EatToken();
questionToken = EatNullableQualifierIfApplicable(questionTokenModeOpt.GetValueOrDefault());
}

return _syntaxFactory.ArrayRankSpecifier(open, list, close, questionToken);
Expand Down
66 changes: 66 additions & 0 deletions src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,72 @@ static void F2(object? w)
Diagnostic(ErrorCode.WRN_MissingNonNullTypesContextForAnnotation, "?").WithLocation(14, 26));
}

[Fact]
public void NullableAndConditionalOperators()
{
var source =
@"class Program
{
static void F1(object x)
{
_ = x is string? 1 : 2;
_ = x is string? ? 1 : 2;
_ = x is string ? ? 1 : 2;
_ = x as string?? x;
_ = x as string ? ?? x;
}
static void F2(object y)
{
_ = y is object[]? 1 : 2;
_ = y is object[]? ? 1 : 2;
_ = y is object[] ? ? 1 : 2;
_ = y as object[]?? y;
_ = y as object[] ? ?? y;
}
static void F3<T>(object z)
{
_ = z is T[][]? 1 : 2;
_ = z is T[]?[] ? 1 : 2;
_ = z is T[] ? [] ? 1 : 2;
_ = z as T[][]?? z;
_ = z as T[] ? [] ?? z;
}
}";

var comp = CreateCompilation(source, parseOptions: TestOptions.Regular7);
comp.VerifyDiagnostics(
// (6,24): error CS8107: Feature 'nullable reference types' is not available in C# 7.0. Please use language version 8.0 or greater.
// _ = x is string? ? 1 : 2;
Diagnostic(ErrorCode.ERR_FeatureNotAvailableInVersion7, "?").WithArguments("nullable reference types", "8.0").WithLocation(6, 24),
// (7,25): error CS8107: Feature 'nullable reference types' is not available in C# 7.0. Please use language version 8.0 or greater.
// _ = x is string ? ? 1 : 2;
Diagnostic(ErrorCode.ERR_FeatureNotAvailableInVersion7, "?").WithArguments("nullable reference types", "8.0").WithLocation(7, 25),
// (9,25): error CS8107: Feature 'nullable reference types' is not available in C# 7.0. Please use language version 8.0 or greater.
// _ = x as string ? ?? x;
Diagnostic(ErrorCode.ERR_FeatureNotAvailableInVersion7, "?").WithArguments("nullable reference types", "8.0").WithLocation(9, 25),
// (14,26): error CS8107: Feature 'nullable reference types' is not available in C# 7.0. Please use language version 8.0 or greater.
// _ = y is object[]? ? 1 : 2;
Diagnostic(ErrorCode.ERR_FeatureNotAvailableInVersion7, "?").WithArguments("nullable reference types", "8.0").WithLocation(14, 26),
// (15,27): error CS8107: Feature 'nullable reference types' is not available in C# 7.0. Please use language version 8.0 or greater.
// _ = y is object[] ? ? 1 : 2;
Diagnostic(ErrorCode.ERR_FeatureNotAvailableInVersion7, "?").WithArguments("nullable reference types", "8.0").WithLocation(15, 27),
// (17,27): error CS8107: Feature 'nullable reference types' is not available in C# 7.0. Please use language version 8.0 or greater.
// _ = y as object[] ? ?? y;
Diagnostic(ErrorCode.ERR_FeatureNotAvailableInVersion7, "?").WithArguments("nullable reference types", "8.0").WithLocation(17, 27),
// (22,21): error CS8107: Feature 'nullable reference types' is not available in C# 7.0. Please use language version 8.0 or greater.
// _ = z is T[]?[] ? 1 : 2;
Diagnostic(ErrorCode.ERR_FeatureNotAvailableInVersion7, "?").WithArguments("nullable reference types", "8.0").WithLocation(22, 21),
// (23,22): error CS8107: Feature 'nullable reference types' is not available in C# 7.0. Please use language version 8.0 or greater.
// _ = z is T[] ? [] ? 1 : 2;
Diagnostic(ErrorCode.ERR_FeatureNotAvailableInVersion7, "?").WithArguments("nullable reference types", "8.0").WithLocation(23, 22),
// (25,22): error CS8107: Feature 'nullable reference types' is not available in C# 7.0. Please use language version 8.0 or greater.
// _ = z as T[] ? [] ?? z;
Diagnostic(ErrorCode.ERR_FeatureNotAvailableInVersion7, "?").WithArguments("nullable reference types", "8.0").WithLocation(25, 22));

comp = CreateCompilation(source, options: WithNonNullTypesTrue());
comp.VerifyDiagnostics();
}

[Fact, WorkItem(29318, "https://github.com/dotnet/roslyn/issues/29318")]
public void IsOperatorOnNonNullExpression()
{
Expand Down
Loading