 Chromium Code Reviews
 Chromium Code Reviews Issue 23606010:
  Fix various parser bugs related to modifiers of top-level and class members.  (Closed) 
  Base URL: https://dart.googlecode.com/svn/branches/bleeding_edge
    
  
    Issue 23606010:
  Fix various parser bugs related to modifiers of top-level and class members.  (Closed) 
  Base URL: https://dart.googlecode.com/svn/branches/bleeding_edge| Index: dart/sdk/lib/_internal/compiler/implementation/scanner/parser.dart | 
| diff --git a/dart/sdk/lib/_internal/compiler/implementation/scanner/parser.dart b/dart/sdk/lib/_internal/compiler/implementation/scanner/parser.dart | 
| index 90b5a0bde0545a6ae9ec52087b64f3cbedb13066..13ffe9bd606593f39a91ae689df5054939ca6667 100644 | 
| --- a/dart/sdk/lib/_internal/compiler/implementation/scanner/parser.dart | 
| +++ b/dart/sdk/lib/_internal/compiler/implementation/scanner/parser.dart | 
| @@ -48,7 +48,8 @@ class Parser { | 
| Token parseTopLevelDeclaration(Token token) { | 
| token = parseMetadataStar(token); | 
| final String value = token.stringValue; | 
| - if ((identical(value, 'abstract')) || (identical(value, 'class'))) { | 
| + if ((identical(value, 'abstract') && optional('class', token.next)) | 
| + || identical(value, 'class')) { | 
| return parseClass(token); | 
| } else if (identical(value, 'typedef')) { | 
| return parseTypedef(token); | 
| @@ -644,26 +645,114 @@ class Parser { | 
| } | 
| var modifiers = identifiers.reverse(); | 
| return isField | 
| - ? parseTopLevelFields(start, modifiers, type, getOrSet, name) | 
| + ? parseFields(start, modifiers, type, getOrSet, name, true) | 
| : parseTopLevelMethod(start, modifiers, type, getOrSet, name); | 
| } | 
| - Token parseTopLevelFields(Token start, | 
| - Link<Token> modifiers, | 
| - Token type, | 
| - Token getOrSet, | 
| - Token name) { | 
| + bool isVarFinalOrConst(Token token) { | 
| + String value = token.stringValue; | 
| + return identical('var', value) | 
| + || identical('final', value) | 
| + || identical('const', value); | 
| + } | 
| + | 
| + Token expectVarFinalOrConst(Link<Token> modifiers, | 
| + bool hasType, | 
| + bool allowStatic) { | 
| + int modifierCount = 0; | 
| + Token staticModifier; | 
| + if (allowStatic && !modifiers.isEmpty | 
| + && optional('static', modifiers.head)) { | 
| + staticModifier = modifiers.head; | 
| + modifierCount++; | 
| + parseModifier(staticModifier); | 
| + modifiers = modifiers.tail; | 
| + } | 
| + if (modifiers.isEmpty) { | 
| + listener.handleModifiers(modifierCount); | 
| + return null; | 
| + } | 
| + if (modifiers.tail.isEmpty) { | 
| + Token modifier = modifiers.head; | 
| + if (isVarFinalOrConst(modifier)) { | 
| + modifierCount++; | 
| + parseModifier(modifier); | 
| + listener.handleModifiers(modifierCount); | 
| + return modifier; | 
| 
Johnni Winther
2013/09/02 11:02:08
Add a comment that the caller checks for 'var Type
 
ahe
2013/09/02 17:49:59
Done.
 | 
| + } | 
| + } | 
| + | 
| + // Slow case to report errors. | 
| + List<Token> modifierList = modifiers.toList(); | 
| + Token varFinalOrConst = | 
| + modifierList.firstWhere(isVarFinalOrConst, orElse: () => null); | 
| + if (allowStatic && staticModifier == null) { | 
| + staticModifier = | 
| + modifierList.firstWhere( | 
| + (modifier) => optional('static', modifier), orElse: () => null); | 
| + if (staticModifier != null) { | 
| + modifierCount++; | 
| + parseModifier(staticModifier); | 
| + modifierList.remove(staticModifier); | 
| + } | 
| + } | 
| + bool hasTypeOrModifier = hasType; | 
| + if (varFinalOrConst != null) { | 
| + parseModifier(varFinalOrConst); | 
| + modifierCount++; | 
| + hasTypeOrModifier = true; | 
| + modifierList.remove(varFinalOrConst); | 
| + } | 
| + listener.handleModifiers(modifierCount); | 
| + var kind = hasTypeOrModifier | 
| + ? MessageKind.EXTRANEOUS_MODIFIER | 
| + : MessageKind.EXTRANEOUS_MODIFIER_REPLACE; | 
| + for (Token modifier in modifierList) { | 
| + listener.reportError(modifier, kind, {'modifier': modifier}); | 
| + } | 
| + return null; | 
| + } | 
| + | 
| + Token parseFields(Token start, | 
| + Link<Token> modifiers, | 
| + Token type, | 
| + Token getOrSet, | 
| + Token name, | 
| + bool isTopLevel) { | 
| + bool hasType = type != null; | 
| + Token varFinalOrConst = | 
| + expectVarFinalOrConst(modifiers, hasType, !isTopLevel); | 
| + bool isVar = false; | 
| + bool hasModifier = false; | 
| + if (varFinalOrConst != null) { | 
| + hasModifier = true; | 
| + isVar = optional('var', varFinalOrConst); | 
| + } | 
| + | 
| if (getOrSet != null) { | 
| - // TODO(ahe): Enable this error: | 
| - // listener.recoverableError("unexpected", token: getOrSet); | 
| + var kind = (hasModifier || hasType) | 
| + ? MessageKind.EXTRANEOUS_MODIFIER | 
| + : MessageKind.EXTRANEOUS_MODIFIER_REPLACE; | 
| + listener.reportError(getOrSet, kind, {'modifier': getOrSet}); | 
| } | 
| - parseModifierList(modifiers); | 
| - if (type == null) { | 
| + | 
| + if (!hasType) { | 
| listener.handleNoType(name); | 
| + } else if (optional('void', type)) { | 
| + listener.handleNoType(name); | 
| + // TODO(ahe): This error is reported twice, second time is from | 
| + // [parseVariablesDeclarationMaybeSemicolon] via | 
| + // [PartialFieldListElement.parseNode]. | 
| + listener.reportError(type, MessageKind.VOID_NOT_ALLOWED); | 
| } else { | 
| - // TODO(ahe): Report recoverable error on 'void'. | 
| - parseReturnTypeOpt(type); | 
| + parseType(type); | 
| + if (isVar) { | 
| + listener.reportError( | 
| + modifiers.head, MessageKind.EXTRANEOUS_MODIFIER, | 
| + {'modifier': modifiers.head}); | 
| + } | 
| } | 
| + | 
| Token token = parseIdentifier(name); | 
| int fieldCount = 1; | 
| @@ -674,7 +763,11 @@ class Parser { | 
| ++fieldCount; | 
| } | 
| expectSemicolon(token); | 
| - listener.endTopLevelFields(fieldCount, start, token); | 
| + if (isTopLevel) { | 
| + listener.endTopLevelFields(fieldCount, start, token); | 
| + } else { | 
| + listener.endFields(fieldCount, start, token); | 
| + } | 
| return token.next; | 
| } | 
| @@ -683,7 +776,22 @@ class Parser { | 
| Token type, | 
| Token getOrSet, | 
| Token name) { | 
| - parseModifierList(modifiers); | 
| + Token externalModifier; | 
| + for (Token modifier in modifiers) { | 
| + if (externalModifier == null && optional('external', modifier)) { | 
| + externalModifier = modifier; | 
| + } else { | 
| + listener.reportError( | 
| + modifier, MessageKind.EXTRANEOUS_MODIFIER, {'modifier': modifier}); | 
| + } | 
| + } | 
| + if (externalModifier != null) { | 
| + parseModifier(externalModifier); | 
| + listener.handleModifiers(1); | 
| + } else { | 
| + listener.handleModifiers(0); | 
| + } | 
| + | 
| if (type == null) { | 
| listener.handleNoType(name); | 
| } else { | 
| @@ -692,7 +800,7 @@ class Parser { | 
| Token token = parseIdentifier(name); | 
| token = parseFormalParametersOpt(token); | 
| - token = parseFunctionBody(token, false); | 
| + token = parseFunctionBody(token, false, externalModifier != null); | 
| listener.endTopLevelMethod(start, getOrSet, token); | 
| return token.next; | 
| } | 
| @@ -961,47 +1069,54 @@ class Parser { | 
| var modifiers = identifiers.reverse(); | 
| return isField | 
| - ? parseFields(start, modifiers, type, getOrSet, name) | 
| + ? parseFields(start, modifiers, type, getOrSet, name, false) | 
| : parseMethod(start, modifiers, type, getOrSet, name); | 
| } | 
| - Token parseFields(Token start, | 
| - Link<Token> modifiers, | 
| - Token type, | 
| - Token getOrSet, | 
| - Token name) { | 
| - parseModifierList(modifiers); | 
| - if (type == null) { | 
| - listener.handleNoType(name); | 
| - } else { | 
| - parseReturnTypeOpt(type); | 
| - } | 
| - | 
| - Token token = parseIdentifier(name); | 
| - | 
| - int fieldCount = 1; | 
| - token = parseVariableInitializerOpt(token); | 
| - if (getOrSet != null) { | 
| - listener.recoverableError("unexpected", token: getOrSet); | 
| - } | 
| - while (optional(',', token)) { | 
| - // TODO(ahe): Count these. | 
| - token = parseIdentifier(token.next); | 
| - token = parseVariableInitializerOpt(token); | 
| - ++fieldCount; | 
| - } | 
| - expectSemicolon(token); | 
| - listener.endFields(fieldCount, start, token); | 
| - return token.next; | 
| - } | 
| - | 
| Token parseMethod(Token start, | 
| Link<Token> modifiers, | 
| Token type, | 
| Token getOrSet, | 
| Token name) { | 
| + Token externalModifier; | 
| + Token staticModifier; | 
| + Token constModifier; | 
| + int modifierCount = 0; | 
| + int allowedModifierCount = 1; | 
| + for (Token modifier in modifiers) { | 
| + if (externalModifier == null && optional('external', modifier)) { | 
| + modifierCount++; | 
| + externalModifier = modifier; | 
| + if (modifierCount != allowedModifierCount) { | 
| + listener.reportError( | 
| + modifier, | 
| + MessageKind.EXTRANEOUS_MODIFIER, {'modifier': modifier}); | 
| + } | 
| + allowedModifierCount++; | 
| + } else if (staticModifier == null && optional('static', modifier)) { | 
| + modifierCount++; | 
| + staticModifier = modifier; | 
| + if (modifierCount != allowedModifierCount) { | 
| + listener.reportError( | 
| + modifier, | 
| + MessageKind.EXTRANEOUS_MODIFIER, {'modifier': modifier}); | 
| + } | 
| + } else if (constModifier == null && optional('const', modifier)) { | 
| + modifierCount++; | 
| + constModifier = modifier; | 
| + if (modifierCount != allowedModifierCount) { | 
| + listener.reportError( | 
| + modifier, | 
| + MessageKind.EXTRANEOUS_MODIFIER, {'modifier': modifier}); | 
| + } | 
| + } else { | 
| + listener.reportError( | 
| + modifier, MessageKind.EXTRANEOUS_MODIFIER, {'modifier': modifier}); | 
| + } | 
| + } | 
| parseModifierList(modifiers); | 
| + | 
| if (type == null) { | 
| listener.handleNoType(name); | 
| } else { | 
| @@ -1010,6 +1125,12 @@ class Parser { | 
| Token token; | 
| if (optional('operator', name)) { | 
| token = parseOperatorName(name); | 
| + if (staticModifier != null) { | 
| + // TODO(ahe): Consider a more specific error message. | 
| + listener.reportError( | 
| + staticModifier, MessageKind.EXTRANEOUS_MODIFIER, | 
| + {'modifier': staticModifier}); | 
| + } | 
| } else { | 
| token = parseIdentifier(name); | 
| } | 
| @@ -1020,7 +1141,8 @@ class Parser { | 
| if (optional('=', token)) { | 
| token = parseRedirectingFactoryBody(token); | 
| } else { | 
| - token = parseFunctionBody(token, false); | 
| + token = parseFunctionBody( | 
| + token, false, staticModifier == null || externalModifier != null); | 
| } | 
| listener.endMethod(getOrSet, start, token); | 
| return token.next; | 
| @@ -1029,7 +1151,11 @@ class Parser { | 
| Token parseFactoryMethod(Token token) { | 
| assert(isFactoryDeclaration(token)); | 
| Token start = token; | 
| - if (identical(token.stringValue, 'external')) token = token.next; | 
| + Token externalModifier; | 
| + if (identical(token.stringValue, 'external')) { | 
| + externalModifier = token; | 
| + token = token.next; | 
| + } | 
| Token constKeyword = null; | 
| if (optional('const', token)) { | 
| constKeyword = token; | 
| @@ -1043,7 +1169,7 @@ class Parser { | 
| if (optional('=', token)) { | 
| token = parseRedirectingFactoryBody(token); | 
| } else { | 
| - token = parseFunctionBody(token, false); | 
| + token = parseFunctionBody(token, false, externalModifier != null); | 
| } | 
| listener.endFactoryMethod(start, token); | 
| return token.next; | 
| @@ -1086,7 +1212,7 @@ class Parser { | 
| if (optional('=', token)) { | 
| token = parseRedirectingFactoryBody(token); | 
| } else { | 
| - token = parseFunctionBody(token, false); | 
| + token = parseFunctionBody(token, false, true); | 
| } | 
| listener.endFunction(getOrSet, token); | 
| return token.next; | 
| @@ -1096,7 +1222,7 @@ class Parser { | 
| listener.beginUnamedFunction(token); | 
| token = parseFormalParameters(token); | 
| bool isBlock = optional('{', token); | 
| - token = parseFunctionBody(token, true); | 
| + token = parseFunctionBody(token, true, false); | 
| listener.endUnamedFunction(token); | 
| return isBlock ? token.next : token; | 
| } | 
| @@ -1118,7 +1244,7 @@ class Parser { | 
| token = parseFormalParameters(token); | 
| listener.handleNoInitializers(); | 
| bool isBlock = optional('{', token); | 
| - token = parseFunctionBody(token, true); | 
| + token = parseFunctionBody(token, true, false); | 
| listener.endFunction(null, token); | 
| return isBlock ? token.next : token; | 
| } | 
| @@ -1149,8 +1275,11 @@ class Parser { | 
| return token; | 
| } | 
| - Token parseFunctionBody(Token token, bool isExpression) { | 
| + Token parseFunctionBody(Token token, bool isExpression, bool allowAbstract) { | 
| if (optional(';', token)) { | 
| + if (!allowAbstract) { | 
| + listener.reportError(token, MessageKind.BODY_EXPECTED); | 
| + } | 
| listener.endFunctionBody(0, null, token); | 
| return token; | 
| } else if (optional('=>', token)) { | 
| @@ -1882,6 +2011,7 @@ class Parser { | 
| } | 
| Token parseVariablesDeclarationNoSemicolon(Token token) { | 
| + // Only called when parsing a for loop, so this is for parsing locals. | 
| return parseVariablesDeclarationMaybeSemicolon(token, false); | 
| } |