Clean up property attributes (2/2)

This commit introduces a new AST node for property attributes (currently
required, default and readonly). That node is not integrated into our
visitors, as it is only meant as an internal implementaion detail of
UiPublicMember. All information stored in it is meant to be accessed by
UiPublicMember's methods (see also the note below).
We use an union of SourceLocation and a node pointer to only pay the
overhead of the new node for properties; signals simply store the signal
token.

The grammar is rewritten with a new UiPropertyAttributes rule, which
avoids quite a bit of duplication in various rules, which had to deal
with the combinatorial explosion of attributes. Some parse errors are
now turned into semantic errors instead (readonly without initializer,
required with initializer).

By centralizing the handling of attributes in the grammar, we now can
easily support e.g. default properties with a list initializer.

As part of this restructuring, UiPublicMember's firstSourceLocation is
fixed to ensure that we actually return the first source location,
independent of the order in which the attributes are written.

Note: In theory, we would not need to make UiPropertyAttributes an AST
node. It could be a simple data class. However, the parser currently
assumes that every pointer in its parser stack is an AST node, and
touching that part would be a larger undertaking. As we use a pool
allocator, the cost is not that high (though we use more memory from the
pool then we would strictly need to).

Change-Id: Ia1d9fd7a6553d443cc57bc3d773f5be0aebe0e0e
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

1 files changed, 74 insertions, 126 deletions

diff --git a/src/qml/parser/qqmljs.g b/src/qml/parser/qqmljs.g
index c3ceeb24b3..9bff45faf3 100644
--- a/src/qml/parser/qqmljs.g
+++ b/src/qml/parser/qqmljs.g
@@ -315,6 +315,7 @@ public:
       AST::UiPragma *UiPragma;
       AST::UiImport *UiImport;
       AST::UiParameterList *UiParameterList;
+      AST::UiPropertyAttributes *UiPropertyAttributes;
       AST::UiPublicMember *UiPublicMember;
       AST::UiObjectDefinition *UiObjectDefinition;
       AST::UiObjectInitializer *UiObjectInitializer;
@@ -1273,7 +1274,7 @@ UiObjectMember: T_SIGNAL T_IDENTIFIER T_LPAREN UiParameterListOpt T_RPAREN Semic
     case $rule_number: {
         AST::UiPublicMember *node = new (pool) AST::UiPublicMember(nullptr, stringRef(2));
         node->type = AST::UiPublicMember::Signal;
-        node->propertyToken = loc(1);
+        node->setPropertyToken(loc(1));
         node->typeToken = loc(2);
         node->identifierToken = loc(2);
         node->parameters = sym(4).UiParameterList;
@@ -1287,7 +1288,7 @@ UiObjectMember: T_SIGNAL T_IDENTIFIER Semicolon;
     case $rule_number: {
         AST::UiPublicMember *node = new (pool) AST::UiPublicMember(nullptr, stringRef(2));
         node->type = AST::UiPublicMember::Signal;
-        node->propertyToken = loc(1);
+        node->setPropertyToken(loc(1));
         node->typeToken = loc(2);
         node->identifierToken = loc(2);
         node->semicolonToken = loc(3);
@@ -1295,114 +1296,97 @@ UiObjectMember: T_SIGNAL T_IDENTIFIER Semicolon;
     } break;
 ./
 
-UiObjectMemberListPropertyNoInitialiser: T_PROPERTY T_IDENTIFIER T_LT UiPropertyType T_GT QmlIdentifier Semicolon;
-/.
-    case $rule_number: {
-        AST::UiPublicMember *node = new (pool) AST::UiPublicMember(sym(4).UiQualifiedId->finish(), stringRef(6));
-        node->typeModifier = stringRef(2);
-        node->propertyToken = loc(1);
-        node->typeModifierToken = loc(2);
-        node->typeToken = loc(4);
-        node->identifierToken = loc(6);
-        node->semicolonToken = loc(7);
-        sym(1).Node = node;
-    } break;
-./
-
-UiObjectMember: UiObjectMemberListPropertyNoInitialiser;
+-------------------------------------------------------------------------------
+-- There is some ambiguity in whether required default property should be parsed
+-- as required (default (property)) or as ((required (default)) property)
+-- by reducing after each attribute modifier, we ensure that T_PROPERTY (which
+-- is always available is used as the base case (so we only have to allocate the
+-- node in the T_PROPERY case, and all other rules can assume that the node is
+-- already available).
+--------------------------------------------------------------------------------
 
-UiObjectMember: T_READONLY UiObjectMemberListPropertyNoInitialiser;
-/.
-    case $rule_number: {
-        AST::UiPublicMember *node = sym(2).UiPublicMember;
-        node->m_readonlyToken = loc(1);
-        sym(1).Node = node;
-    } break;
-./
+AttrRequired:  T_REQUIRED %prec REDUCE_HERE;
+AttrReadonly:  T_READONLY %prec REDUCE_HERE;
+AttrDefault:  T_DEFAULT %prec REDUCE_HERE;
 
-UiObjectMemberPropertyNoInitialiser: T_PROPERTY UiPropertyType QmlIdentifier Semicolon;
+UiPropertyAttributes: AttrRequired UiPropertyAttributes;
 /.
     case $rule_number: {
-        AST::UiPublicMember *node = new (pool) AST::UiPublicMember(sym(2).UiQualifiedId->finish(), stringRef(3));
-        node->propertyToken = loc(1);
-        node->typeToken = loc(2);
-        node->identifierToken = loc(3);
-        node->semicolonToken = loc(4);
-        sym(1).Node = node;
+        AST::UiPropertyAttributes *node = sym(2).UiPropertyAttributes;
+        if (node->isRequired())
+            diagnostic_messages.append(compileError(node->requiredToken(), QLatin1String("Duplicated 'required' attribute is not allowed."), QtCriticalMsg));
+        node->m_requiredToken = loc(1);
+        sym(1).UiPropertyAttributes = node;
     } break;
 ./
 
-
-UiObjectMember: UiObjectMemberPropertyNoInitialiser;
-
-UiObjectMember: T_DEFAULT UiObjectMemberPropertyNoInitialiser;
+UiPropertyAttributes: AttrDefault UiPropertyAttributes;
 /.
     case $rule_number: {
-        AST::UiPublicMember *node = sym(2).UiPublicMember;
+        AST::UiPropertyAttributes *node = sym(2).UiPropertyAttributes;
+        if (node->isDefaultMember())
+            diagnostic_messages.append(compileError(node->requiredToken(), QLatin1String("Duplicated 'default' attribute is not allowed."), QtCriticalMsg));
         node->m_defaultToken = loc(1);
-        sym(1).Node = node;
+        sym(1).UiPropertyAttributes = node;
     } break;
 ./
 
-
-UiObjectMember: T_REQUIRED UiObjectMemberListPropertyNoInitialiser;
+UiPropertyAttributes: AttrReadonly UiPropertyAttributes;
 /.
     case $rule_number: {
-        AST::UiPublicMember *node = sym(2).UiPublicMember;
-        node->m_requiredToken = loc(1);
-        sym(1).Node = node;
+        AST::UiPropertyAttributes *node = sym(2).UiPropertyAttributes;
+        if (node->isReadonly())
+            diagnostic_messages.append(compileError(node->requiredToken(), QLatin1String("Duplicated 'readonly' attribute is not allowed."), QtCriticalMsg));
+        node->m_readonlyToken = loc(1);
+        sym(1).UiPropertyAttributes = node;
     } break;
 ./
 
-UiObjectMember: T_DEFAULT T_REQUIRED UiObjectMemberListPropertyNoInitialiser;
+UiPropertyAttributes: T_PROPERTY;
 /.
     case $rule_number: {
-        AST::UiPublicMember *node = sym(3).UiPublicMember;
-        node->m_requiredToken = loc(2);
-        node->m_defaultToken = loc(1);
-        sym(1).Node = node;
+        AST::UiPropertyAttributes *node = new (pool) AST::UiPropertyAttributes();
+        node->m_propertyToken = loc(1);
+        sym(1).UiPropertyAttributes = node;
     } break;
 ./
 
-UiObjectMember: T_REQUIRED T_DEFAULT UiObjectMemberListPropertyNoInitialiser;
+UiObjectMemberListPropertyNoInitialiser: UiPropertyAttributes T_IDENTIFIER T_LT UiPropertyType T_GT QmlIdentifier Semicolon;
 /.
     case $rule_number: {
-        AST::UiPublicMember *node = sym(3).UiPublicMember;
-        node->m_requiredToken = loc(1);
-        node->m_defaultToken = loc(2);
+        AST::UiPublicMember *node = new (pool) AST::UiPublicMember(sym(4).UiQualifiedId->finish(), stringRef(6));
+        auto attributes = sym(1).UiPropertyAttributes;
+        node->setAttributes(attributes);
+        if (attributes->isReadonly())
+            diagnostic_messages.append(compileError(attributes->readonlyToken(), QLatin1String("Read-only properties require an initializer."), QtWarningMsg));
+        node->typeModifier = stringRef(2);
+        node->typeModifierToken = loc(2);
+        node->typeToken = loc(4);
+        node->identifierToken = loc(6);
+        node->semicolonToken = loc(7);
         sym(1).Node = node;
     } break;
 ./
 
-UiObjectMember: T_DEFAULT UiObjectMemberListPropertyNoInitialiser;
-/.
-    case $rule_number: {
-        AST::UiPublicMember *node = sym(2).UiPublicMember;
-        node->m_defaultToken = loc(1);
-        sym(1).Node = node;
-    } break;
-./
+UiObjectMember: UiObjectMemberListPropertyNoInitialiser;
 
-UiObjectMember: T_DEFAULT T_REQUIRED UiObjectMemberPropertyNoInitialiser;
+UiObjectMemberPropertyNoInitialiser: UiPropertyAttributes UiPropertyType QmlIdentifier Semicolon;
 /.
     case $rule_number: {
-        AST::UiPublicMember *node = sym(3).UiPublicMember;
-        node->m_defaultToken = loc(1);
-        node->m_requiredToken = loc(2);
+        AST::UiPublicMember *node = new (pool) AST::UiPublicMember(sym(2).UiQualifiedId->finish(), stringRef(3));
+        auto attributes = sym(1).UiPropertyAttributes;
+        if (attributes->isReadonly())
+            diagnostic_messages.append(compileError(attributes->readonlyToken(), QLatin1String("Read-only properties require an initializer."), QtCriticalMsg));
+        node->setAttributes(attributes);
+        node->typeToken = loc(2);
+        node->identifierToken = loc(3);
+        node->semicolonToken = loc(4);
         sym(1).Node = node;
     } break;
 ./
 
 
-UiObjectMember: T_REQUIRED T_DEFAULT UiObjectMemberPropertyNoInitialiser;
-/.
-    case $rule_number: {
-        AST::UiPublicMember *node = sym(3).UiPublicMember;
-        node->m_defaultToken = loc(2);
-        node->m_requiredToken = loc(1);
-        sym(1).Node = node;
-    } break;
-./
+UiObjectMember: UiObjectMemberPropertyNoInitialiser;
 
 OptionalSemicolon: | Semicolon;
 /.
@@ -1422,20 +1406,14 @@ UiRequired: T_REQUIRED QmlIdentifier Semicolon;
 
 UiObjectMember: UiRequired;
 
-UiObjectMember: T_REQUIRED UiObjectMemberPropertyNoInitialiser;
-/.
-    case $rule_number: {
-        AST::UiPublicMember *node = sym(2).UiPublicMember;
-        node->m_requiredToken = loc(1);
-        sym(1).Node = node;
-    } break;
-./
-
-UiObjectMemberWithScriptStatement: T_PROPERTY UiPropertyType QmlIdentifier T_COLON UiScriptStatement OptionalSemicolon;
+UiObjectMemberWithScriptStatement: UiPropertyAttributes UiPropertyType QmlIdentifier T_COLON UiScriptStatement OptionalSemicolon;
 /.
     case $rule_number: {
         AST::UiPublicMember *node = new (pool) AST::UiPublicMember(sym(2).UiQualifiedId->finish(), stringRef(3), sym(5).Statement);
-        node->propertyToken = loc(1);
+        auto attributes = sym(1).UiPropertyAttributes;
+        if (attributes->isRequired())
+            diagnostic_messages.append(compileError(attributes->requiredToken(), QLatin1String("Required properties with initializer do not make sense."), QtCriticalMsg));
+        node->setAttributes(attributes);
         node->typeToken = loc(2);
         node->identifierToken = loc(3);
         node->colonToken = loc(4);
@@ -1445,30 +1423,15 @@ UiObjectMemberWithScriptStatement: T_PROPERTY UiPropertyType QmlIdentifier T_COL
 
 UiObjectMember: UiObjectMemberWithScriptStatement;
 
-UiObjectMember: T_READONLY UiObjectMemberWithScriptStatement;
-/.
-    case $rule_number: {
-        AST::UiPublicMember *node = sym(2).UiPublicMember;
-        node->m_readonlyToken = loc(1);
-        sym(1).Node = node;
-    } break;
-./
-
-UiObjectMember: T_DEFAULT UiObjectMemberWithScriptStatement;
-/.
-    case $rule_number: {
-        AST::UiPublicMember *node = sym(2).UiPublicMember;
-        node->m_defaultToken = loc(1);
-        sym(1).Node = node;
-    } break;
-./
-
-UiObjectMemberWithArray: T_PROPERTY T_IDENTIFIER T_LT UiPropertyType T_GT QmlIdentifier T_COLON T_LBRACKET UiArrayMemberList T_RBRACKET Semicolon;
+UiObjectMemberWithArray: UiPropertyAttributes T_IDENTIFIER T_LT UiPropertyType T_GT QmlIdentifier T_COLON T_LBRACKET UiArrayMemberList T_RBRACKET Semicolon;
 /.
     case $rule_number: {
         AST::UiPublicMember *node = new (pool) AST::UiPublicMember(sym(4).UiQualifiedId->finish(), stringRef(6));
+        auto attributes = sym(1).UiPropertyAttributes;
+        if (attributes->isRequired())
+            diagnostic_messages.append(compileError(attributes->requiredToken(), QLatin1String("Required properties with initializer do not make sense."), QtCriticalMsg));
+        node->setAttributes(attributes);
         node->typeModifier = stringRef(2);
-        node->propertyToken = loc(1);
         node->typeModifierToken = loc(2);
         node->typeToken = loc(4);
         node->identifierToken = loc(6);
@@ -1476,7 +1439,7 @@ UiObjectMemberWithArray: T_PROPERTY T_IDENTIFIER T_LT UiPropertyType T_GT QmlIde
 
         AST::UiQualifiedId *propertyName = new (pool) AST::UiQualifiedId(stringRef(6));
         propertyName->identifierToken = loc(6);
-        propertyName->next = 0;
+        propertyName->next = nullptr;
 
         AST::UiArrayBinding *binding = new (pool) AST::UiArrayBinding(propertyName, sym(9).UiArrayMemberList->finish());
         binding->colonToken = loc(7);
@@ -1491,27 +1454,21 @@ UiObjectMemberWithArray: T_PROPERTY T_IDENTIFIER T_LT UiPropertyType T_GT QmlIde
 
 UiObjectMember: UiObjectMemberWithArray;
 
-UiObjectMember: T_READONLY UiObjectMemberWithArray;
-/.
-    case $rule_number: {
-        AST::UiPublicMember *node = sym(2).UiPublicMember;
-        node->m_readonlyToken = loc(1);
-        sym(1).Node = node;
-    } break;
-./
-
-UiObjectMemberExpressionStatementLookahead: T_PROPERTY UiPropertyType QmlIdentifier T_COLON ExpressionStatementLookahead UiQualifiedId UiObjectInitializer Semicolon;
+UiObjectMemberExpressionStatementLookahead: UiPropertyAttributes UiPropertyType QmlIdentifier T_COLON ExpressionStatementLookahead UiQualifiedId UiObjectInitializer Semicolon;
 /.
     case $rule_number: {
         AST::UiPublicMember *node = new (pool) AST::UiPublicMember(sym(2).UiQualifiedId->finish(), stringRef(3));
-        node->propertyToken = loc(1);
+        auto attributes = sym(1).UiPropertyAttributes;
+        if (attributes->isRequired())
+            diagnostic_messages.append(compileError(attributes->requiredToken(), QLatin1String("Required properties with initializer do not make sense."), QtWarningMsg));
+        node->setAttributes(attributes);
         node->typeToken = loc(2);
         node->identifierToken = loc(3);
         node->semicolonToken = loc(4); // insert a fake ';' before ':'
 
         AST::UiQualifiedId *propertyName = new (pool) AST::UiQualifiedId(stringRef(3));
         propertyName->identifierToken = loc(3);
-        propertyName->next = 0;
+        propertyName->next = nullptr;
 
         AST::UiObjectBinding *binding = new (pool) AST::UiObjectBinding(
           propertyName, sym(6).UiQualifiedId, sym(7).UiObjectInitializer);
@@ -1525,15 +1482,6 @@ UiObjectMemberExpressionStatementLookahead: T_PROPERTY UiPropertyType QmlIdentif
 
 UiObjectMember: UiObjectMemberExpressionStatementLookahead;
 
-UiObjectMember: T_READONLY UiObjectMemberExpressionStatementLookahead;
-/.
-    case $rule_number: {
-        AST::UiPublicMember *node = sym(2).UiPublicMember;
-        node->m_readonlyToken = loc(1);
-        sym(1).Node = node;
-    } break;
-./
-
 UiObjectMember: GeneratorDeclaration;
 /.
     case $rule_number: {