parser error if identifier is also a token (CxxKeyword)

[Bertk]

Parser fails if a keyword is used as identifier - the following code is correct and uses "typeid" which is defined as token CxxKeyword.TYPEID.

static IInterface^ Connect()
{
    for each(App::Feature^ feature in App::Current->Features)
    {
        if(feature->GetType() == IInterface::typeid)
            return (IInterface^)feature;
    }
    return nullptr;
}

This problem was detected while C++/CLI support was introduced (#582 )

[guwirth]

For my understanding typeid is a keyword in C++ and C++\CLI:

http://en.cppreference.com/w/cpp/language/typeid
https://msdn.microsoft.com/en-us/library/xey702bw.aspx
https://msdn.microsoft.com/en-us/library/kwd9abya.aspx

General question is why do we need keywords? For syntax highlighting only?

[Bertk]

@guwirth This is correct for typeid but C++/CLI uses new keywords which created problems with C++ only sources and therefore I removed them from CxxKeyword. The drawbacks are the missing highlighting and the tags in AST which can be used by checks etc.
I tried also to extend the postfixExpression rule and add the CLI typeid extention but I noticed side-effects and a some refactoring might be required to make it work.

    b.rule(postfixExpression).is(
      b.firstOf(
            b.sequence(simpleTypeSpecifier, "(", b.optional(expressionList), ")"),
            b.sequence(simpleTypeSpecifier, bracedInitList),
            b.sequence(simpleTypeSpecifier, "::", CxxKeyword.TYPEID),
            b.sequence(typenameSpecifier, "(", b.optional(expressionList), ")"),
            b.sequence(typenameSpecifier, bracedInitList),
            b.sequence(typenameSpecifier, "::", CxxKeyword.TYPEID),
            primaryExpression,
            b.sequence(CxxKeyword.DYNAMIC_CAST, typeIdEnclosed, "(", expression, ")"),
            b.sequence(CxxKeyword.STATIC_CAST, typeIdEnclosed, "(", expression, ")"),
            b.sequence(CxxKeyword.REINTERPRET_CAST, typeIdEnclosed, "(", expression, ")"),
            b.sequence(CxxKeyword.CONST_CAST, typeIdEnclosed, "(", expression, ")"),
            b.sequence(CxxKeyword.TYPEID, "(", expression, ")"),
            b.sequence(CxxKeyword.TYPEID, "(", typeId, ")")
        ),
        b.zeroOrMore(
        b.firstOf(
            b.sequence("[", expressionList, "]"),
            b.sequence("[", bracedInitList, "]"),
            b.sequence("(", b.optional(expressionList), ")"),
            b.sequence(b.firstOf(".", "->"),
                b.firstOf(b.sequence(b.optional(CxxKeyword.TEMPLATE), idExpression),
                    pseudoDestructorName)),
            "++",
            "--"
        )
        )
        ).skipIfOneChild();

The minimal but not optimal solution is to remove the CxxKeyword.TYPEID to avoid the parser error. Should I create a PR for this?
By the way, the sequence of the statements within postfixExpression is not correct and the definition for postfix-expression [ braced-init-list ]is missing. New parser errors occurred after moving primary-expression to the binginning of the rule and I stopped this activity.
Excerpt from N4527 Annex A Grammar summary:

postfix-expression:
primary-expression
postfix-expression [ expression ]
postfix-expression [ braced-init-list ]
postfix-expression ( expression-listopt)
simple-type-specifier ( expression-listopt)
typename-specifier ( expression-listopt)
simple-type-specifier braced-init-list
typename-specifier braced-init-list
postfix-expression . templateopt id-expression
postfix-expression -> templateopt id-expression
postfix-expression . pseudo-destructor-name
postfix-expression -> pseudo-destructor-name
postfix-expression ++
postfix-expression --
dynamic_cast < type-id > ( expression )
static_cast < type-id > ( expression )
reinterpret_cast < type-id > ( expression )
const_cast < type-id > ( expression )
typeid ( expression )
typeid ( type-id )

[guwirth]

@Bertk My initial idea and question was to remove all keywords from the grammar and replace them with strings. Question is how to deal otherwise with a mixture of C, C++ and C++/CLI. I'm only not sure with other side effects this could have? E.g. How does Sonar calculate the complexity?

The drawbacks are the missing highlighting and the tags in AST which can be used by checks etc.

Keeping it in https://github.com/wenns/sonar-cxx/blob/master/cxx-squid/src/main/java/org/sonar/cxx/api/CxxKeyword.java should also keep the syntax highlighting?

tags in AST...

Someone using this?

he minimal but not optimal solution is to remove the CxxKeyword.TYPEID

You can give it a try.

By the way, the sequence of the statements within postfixExpression...

During my work on C++11 and C++14 extensions I also realized that there are still differences in the grammar. I think that is an extra point we should keep in mind and fix.

[guwirth]

• Keyword parsing is in: https://github.com/wenns/sonar-cxx/blob/master/cxx-squid/src/main/java/org/sonar/cxx/lexer/CxxLexer.java
• Keyword highlighting is in https://github.com/wenns/sonar-cxx/blob/master/sonar-cxx-plugin/src/main/java/org/sonar/plugins/cxx/highlighter/CxxHighlighter.java isKeyword

[guwirth]

AstScanner is using keywords for e.g. ComplexityVisitor. Still not sure how many places has to be adapted in case we remove CxxKeyword from lexer/grammar?

[guwirth]

Close this use #597 for further discussions.