Skip to content

Commit

Permalink
Revert "Add support for parsing simple nullable types"
Browse files Browse the repository at this point in the history 
This reverts commit c5fd11b.

Reason for revert: Conditional expression lookahead may modify the token stream

Original change's description:
> Add support for parsing simple nullable types
> 
> ... as part of adding NNBD as outlined in
> dart-lang/language#110
> 
> This only supports parsing simple nullable types
> such as int? and List<int>? while subsequent CLs
> will add support for parsing more complex types.
> 
> Change-Id: I144858946cb115755af437299899c2631105bf8c
> Reviewed-on: https://dart-review.googlesource.com/c/87501
> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
> Commit-Queue: Dan Rubel <danrubel@google.com>

TBR=brianwilkerson@google.com,danrubel@google.com

Change-Id: Ie1f47e7384ff51159aa2ebeb21561455b8e6287f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/87620
Reviewed-by: Dan Rubel <danrubel@google.com>
Commit-Queue: Dan Rubel <danrubel@google.com>
  • Loading branch information
Dan Rubel authored and commit-bot@chromium.org committed Dec 18, 2018
1 parent c7e7cbd commit 48093f5
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 288 deletions.
28 changes: 10 additions & 18 deletions pkg/front_end/lib/src/fasta/parser/parser.dart
View file
Expand Up @@ -4489,7 +4489,11 @@ class Parser {
Token name = beforeName.next;
if (name.isIdentifier) {
TypeParamOrArgInfo typeParam = computeTypeParamOrArg(name);
Token next = typeParam.skip(name).next;
Token next = name;
if (typeParam != noTypeParamOrArg) {
next = typeParam.skip(next);
}
next = next.next;
if (optional('(', next)) {
if (looksLikeFunctionBody(next.endGroup.next)) {
return parseFunctionLiteral(
Expand Down Expand Up @@ -4869,10 +4873,8 @@ class Parser {
if (optional('!', token.next)) {
not = token = token.next;
}
TypeInfo typeInfo = computeType(token, true);
if (typeInfo.isConditionalExpressionStart(token, this)) {
typeInfo = typeInfo.asNonNullable;
}
// Ignore trailing `?` if there is one as it may be part of an expression
TypeInfo typeInfo = computeType(token, true).asNonNullableType();
token = typeInfo.ensureTypeNotVoid(token, this);
listener.handleIsOperator(operator, not);
return skipChainedAsIsOperators(token);
Expand All @@ -4886,10 +4888,8 @@ class Parser {
Token parseAsOperatorRest(Token token) {
Token operator = token = token.next;
assert(optional('as', operator));
TypeInfo typeInfo = computeType(token, true);
if (typeInfo.isConditionalExpressionStart(token, this)) {
typeInfo = typeInfo.asNonNullable;
}
// Ignore trailing `?` if there is one as it may be part of an expression
TypeInfo typeInfo = computeType(token, true).asNonNullableType();
token = typeInfo.ensureTypeNotVoid(token, this);
listener.handleAsOperator(operator);
return skipChainedAsIsOperators(token);
Expand All @@ -4908,11 +4908,7 @@ class Parser {
if (optional('!', next.next)) {
next = next.next;
}
TypeInfo typeInfo = computeType(next, true);
if (typeInfo.isConditionalExpressionStart(next, this)) {
typeInfo = typeInfo.asNonNullable;
}
token = typeInfo.skipType(next);
token = computeType(next, true).skipType(next);
next = token.next;
value = next.stringValue;
}
Expand Down Expand Up @@ -5033,10 +5029,6 @@ class Parser {
TypeInfo typeInfo,
bool onlyParseVariableDeclarationStart = false]) {
typeInfo ??= computeType(beforeType, false);
if (typeInfo.isConditionalExpressionStart(beforeType, this)) {
typeInfo = noType;
}

Token token = typeInfo.skipType(beforeType);
Token next = token.next;

Expand Down
45 changes: 9 additions & 36 deletions pkg/front_end/lib/src/fasta/parser/type_info.dart
View file
Expand Up @@ -19,12 +19,12 @@ import 'util.dart' show isOneOf, optional;
abstract class TypeInfo {
/// Return type info representing the receiver without the trailing `?`
/// or the receiver if the receiver does not represent a nullable type.
TypeInfo get asNonNullable;
TypeInfo asNonNullableType();

/// Return `true` if the tokens comprising the type represented by the
/// receiver could be interpreted as a valid standalone expression.
/// For example, `A` or `A.b` could be interpreted as type references
/// or expressions, while `A<T>` only looks like a type reference.
/// For example, `A` or `A.b` could be interpreted as a type references
/// or as expressions, while `A<T>` only looks like a type reference.
bool get couldBeExpression;

/// Call this function when the token after [token] must be a type (not void).
Expand All @@ -41,13 +41,6 @@ abstract class TypeInfo {
/// in valid code or during recovery.
Token ensureTypeOrVoid(Token token, Parser parser);

/// Return `true` if the tokens comprising the type represented by the
/// receiver are the start of a conditional expression.
/// For example, `A?` or `A.b?` could be the start of a conditional expression
/// and require arbitrary look ahead to determine if it is,
/// while `A<T>?` cannot be the start of a conditional expression.
bool isConditionalExpressionStart(Token token, Parser parser);

/// Call this function to parse an optional type (not void) after [token].
/// This function will call the appropriate event methods on the [Parser]'s
/// listener to handle the type. This may modify the token stream
Expand Down Expand Up @@ -206,19 +199,13 @@ TypeInfo computeType(final Token token, bool required,
// We've seen identifier `<` identifier `>`
next = typeParamOrArg.skip(next).next;
if (!isGeneralizedFunctionType(next)) {
if (optional('?', next) && typeParamOrArg == simpleTypeArgument1) {
if (required || looksLikeName(next.next)) {
// identifier `<` identifier `>` `?` identifier
return simpleNullableTypeWith1Argument;
}
if (required || looksLikeName(next)) {
// identifier `<` identifier `>` identifier
return typeParamOrArg.typeInfo;
} else {
if (required || looksLikeName(next)) {
// identifier `<` identifier `>` identifier
return typeParamOrArg.typeInfo;
}
// identifier `<` identifier `>` non-identifier
return noType;
}
// identifier `<` identifier `>` non-identifier
return noType;
}
}
// TODO(danrubel): Consider adding a const for
Expand Down Expand Up @@ -268,21 +255,7 @@ TypeInfo computeType(final Token token, bool required,
.computeIdentifierGFT(required);
}

if (optional('?', next)) {
if (required) {
// identifier `?`
return simpleNullableType;
} else {
next = next.next;
if (isGeneralizedFunctionType(next)) {
// identifier `?` Function `(`
return simpleNullableType;
} else if (looksLikeName(next)) {
// identifier `?` identifier `=`
return simpleNullableType;
}
}
} else if (required || looksLikeName(next)) {
if (required || looksLikeName(next)) {
// identifier identifier
return simpleType;
}
Expand Down
124 changes: 14 additions & 110 deletions pkg/front_end/lib/src/fasta/parser/type_info_impl.dart
View file
Expand Up @@ -38,10 +38,6 @@ import 'util.dart'
/// when there is a single identifier as the type reference.
const TypeInfo simpleType = const SimpleType();

/// [SimpleNullableType] is a specialized [TypeInfo] returned by [computeType]
/// when there is a single identifier followed by `?` as the type reference.
const TypeInfo simpleNullableType = const SimpleNullableType();

/// [PrefixedType] is a specialized [TypeInfo] returned by [computeType]
/// when the type reference is of the form: identifier `.` identifier.
const TypeInfo prefixedType = const PrefixedType();
Expand All @@ -64,12 +60,6 @@ const TypeInfo simpleTypeWith1ArgumentGtEq =
const TypeInfo simpleTypeWith1ArgumentGtGt =
const SimpleTypeWith1Argument(simpleTypeArgument1GtGt);

/// [SimpleNullableTypeWith1Argument] is a specialized [TypeInfo] returned by
/// [computeType] when the type reference is of the form:
/// identifier `<` identifier `>` `?`.
const TypeInfo simpleNullableTypeWith1Argument =
const SimpleNullableTypeWith1Argument();

/// [SimpleTypeArgument1] is a specialized [TypeParamOrArgInfo] returned by
/// [computeTypeParamOrArg] when the type reference is of the form:
/// `<` identifier `>`.
Expand All @@ -92,10 +82,10 @@ class NoType implements TypeInfo {
const NoType();

@override
TypeInfo get asNonNullable => this;
bool get couldBeExpression => false;

@override
bool get couldBeExpression => false;
TypeInfo asNonNullableType() => this;

@override
Token ensureTypeNotVoid(Token token, Parser parser) {
Expand All @@ -109,9 +99,6 @@ class NoType implements TypeInfo {
Token ensureTypeOrVoid(Token token, Parser parser) =>
ensureTypeNotVoid(token, parser);

@override
bool isConditionalExpressionStart(Token token, Parser parser) => false;

@override
Token parseTypeNotVoid(Token token, Parser parser) =>
parseType(token, parser);
Expand All @@ -133,10 +120,10 @@ class PrefixedType implements TypeInfo {
const PrefixedType();

@override
TypeInfo get asNonNullable => this;
bool get couldBeExpression => true;

@override
bool get couldBeExpression => true;
TypeInfo asNonNullableType() => this;

@override
Token ensureTypeNotVoid(Token token, Parser parser) =>
Expand All @@ -146,9 +133,6 @@ class PrefixedType implements TypeInfo {
Token ensureTypeOrVoid(Token token, Parser parser) =>
parseType(token, parser);

@override
bool isConditionalExpressionStart(Token token, Parser parser) => false;

@override
Token parseTypeNotVoid(Token token, Parser parser) =>
parseType(token, parser);
Expand Down Expand Up @@ -180,44 +164,17 @@ class PrefixedType implements TypeInfo {
}
}

/// See documentation on the [simpleNullableTypeWith1Argument] const.
class SimpleNullableTypeWith1Argument extends SimpleTypeWith1Argument {
const SimpleNullableTypeWith1Argument() : super(simpleTypeArgument1);

@override
TypeInfo get asNonNullable => simpleTypeWith1Argument;

@override
bool isConditionalExpressionStart(Token token, Parser parser) =>
isConditionalThenExpression(skipType(token), parser);

@override
Token parseTypeRest(Token start, Token token, Parser parser) {
token = token.next;
assert(optional('?', token));
parser.listener.handleType(start, token);
return token;
}

@override
Token skipType(Token token) {
token = super.skipType(token).next;
assert(optional('?', token));
return token;
}
}

/// See documentation on the [simpleTypeWith1Argument] const.
class SimpleTypeWith1Argument implements TypeInfo {
final TypeParamOrArgInfo typeArg;

const SimpleTypeWith1Argument(this.typeArg);

@override
TypeInfo get asNonNullable => this;
bool get couldBeExpression => false;

@override
bool get couldBeExpression => false;
TypeInfo asNonNullableType() => this;

@override
Token ensureTypeNotVoid(Token token, Parser parser) =>
Expand All @@ -227,9 +184,6 @@ class SimpleTypeWith1Argument implements TypeInfo {
Token ensureTypeOrVoid(Token token, Parser parser) =>
parseType(token, parser);

@override
bool isConditionalExpressionStart(Token token, Parser parser) => false;

@override
Token parseTypeNotVoid(Token token, Parser parser) =>
parseType(token, parser);
Expand All @@ -256,40 +210,15 @@ class SimpleTypeWith1Argument implements TypeInfo {
}
}

/// See documentation on the [simpleNullableType] const.
class SimpleNullableType extends SimpleType {
const SimpleNullableType();

@override
TypeInfo get asNonNullable => simpleType;

@override
bool isConditionalExpressionStart(Token token, Parser parser) =>
isConditionalThenExpression(skipType(token), parser);

@override
Token parseTypeRest(Token start, Parser parser) {
Token token = start.next;
assert(optional('?', token));
parser.listener.handleType(start, token);
return token;
}

@override
Token skipType(Token token) {
return token.next.next;
}
}

/// See documentation on the [simpleType] const.
class SimpleType implements TypeInfo {
const SimpleType();

@override
TypeInfo get asNonNullable => this;
bool get couldBeExpression => true;

@override
bool get couldBeExpression => true;
TypeInfo asNonNullableType() => this;

@override
Token ensureTypeNotVoid(Token token, Parser parser) =>
Expand All @@ -299,9 +228,6 @@ class SimpleType implements TypeInfo {
Token ensureTypeOrVoid(Token token, Parser parser) =>
parseType(token, parser);

@override
bool isConditionalExpressionStart(Token token, Parser parser) => false;

@override
Token parseTypeNotVoid(Token token, Parser parser) =>
parseType(token, parser);
Expand Down Expand Up @@ -331,10 +257,10 @@ class VoidType implements TypeInfo {
const VoidType();

@override
TypeInfo get asNonNullable => this;
bool get couldBeExpression => false;

@override
bool get couldBeExpression => false;
TypeInfo asNonNullableType() => this;

@override
Token ensureTypeNotVoid(Token token, Parser parser) {
Expand All @@ -347,9 +273,6 @@ class VoidType implements TypeInfo {
Token ensureTypeOrVoid(Token token, Parser parser) =>
parseType(token, parser);

@override
bool isConditionalExpressionStart(Token token, Parser parser) => false;

@override
Token parseTypeNotVoid(Token token, Parser parser) =>
ensureTypeNotVoid(token, parser);
Expand Down Expand Up @@ -414,12 +337,12 @@ class ComplexTypeInfo implements TypeInfo {
: this.start = beforeStart.next;

@override
TypeInfo get asNonNullable {
return this;
}
bool get couldBeExpression => false;

@override
bool get couldBeExpression => false;
TypeInfo asNonNullableType() {
return this;
}

@override
Token ensureTypeNotVoid(Token token, Parser parser) =>
Expand All @@ -429,12 +352,6 @@ class ComplexTypeInfo implements TypeInfo {
Token ensureTypeOrVoid(Token token, Parser parser) =>
parseType(token, parser);

@override
bool isConditionalExpressionStart(Token token, Parser parser) {
//return isConditionalThenExpression(token.next.next, parser);
return false;
}

@override
Token parseTypeNotVoid(Token token, Parser parser) =>
parseType(token, parser);
Expand Down Expand Up @@ -1144,16 +1061,3 @@ Token splitCloser(Token closer) {
}
return null;
}

/// Return `true` if the tokens after [token]
/// represent a valid expression followed by a `:`.
bool isConditionalThenExpression(Token token, Parser parser) {
// TODO(danrubel): Consider adding checks for simple situations
// before resorting to heavy weight lookahead.

final originalListener = parser.listener;
parser.listener = new ForwardingListener();
token = parser.parseExpression(token);
parser.listener = originalListener;
return optional(':', token.next);
}

0 comments on commit 48093f5

Please sign in to comment.