[BUG] the grammar comments are not always correct

[neumannt]

Out of curiosity I have an implemented an alternative parser for cppfront / cpp2, which uses a PEG grammar as input for a parser generator. During that experiment, I noticed that the grammar rules embedded as //G comments are not always correct. I will list errors that I noticed below.

One preliminary note: The cppfront compiler has a rather relaxed concept of keywords. In most cases it will accept a keyword were an identifier is expected, for example it will happily compile if: () -> void = { }. I don't think that is a good idea, my grammar explicitly distinguishes between keywords and identifiers. (Modulo the few context specific soft-keywords like in/out etc.). For some grammar rules that requires changes were the parser previously worked by accident (i.e, by not recognizing a certain keyword).

a) id_expression

    //G id-expression
    //G     unqualified-id
    //G     qualified-id
    //G

here the order is wrong, it should be

    //G id-expression
    //G     qualified-id
    //G     unqualified-id    
    //G

b) primary_expression

    //G primary-expression:
    //G     literal
    //G     ( expression-list )
    //G     id-expression
    //G     unnamed-declaration
    //G     inspect-expression
    //G

this does not correspond to the source code order. Furthermore, the expression-list is optional. And if we distinguish keywords from literals we potentially need some extra rules to handle keywords that are currently silently eaten as identifier. I would suggest

    //G primary-expression:
    //G     inspect-expression
    //G     id-expression
    //G     literal
    //G     '(' expression-list? ')'
    //G     unnamed-declaration
    //G     'nullptr'
    //G     'true'
    //G     'false'
    //G     'typeid' '(' expression ')'
    //G     'new' < id-expression > '(' expression-list? ')'

c) nested-name-specifier

    //G nested-name-specifier:
    //G     ::
    //G     unqualified-id ::

this has to support nested scopes. I would suggest

    //G nested-name-specifier:
    //G     :: (unqualified-id ::)*
    //G     (unqualified-id ::)+

d) template-argument

    //G template-argument:
    //G     expression
    //G     id-expression

There should be a comment here that we disable '<'/'>'/'<<'/'>>' in the expressions until a new parentheses is opened. In fact that causes some of the expression rules to be cloned until we reach the level below these operators. (In my implementation these are the rules with suffix _no_cmp).

e) id-expression from fundamental types

We want to accept builtin types like int as type ids. Currently this works by accident because the parser does not even recognize these as keywords. When enforcing that keywords are not identifiers we need rules for these, too. I have added a fundamental-type alternative at the end of id-expression, and have defines that as follows:

fundamental-type
  'void'
  fundamental-type-modifier_list? 'char'
  'char8_t'
  'char16_t'
  'char32_t'
  'wchar_t'
  fundamental-type-modifier-list? 'int'
  'bool'
  'float'
  'double'
  'long' 'double'
  fundamental-type-modifier-list

fundamental-type-modifier-list
  fundamental-type-modifier+

fundamental-type-modifier
  'unsigned'
  'signed'
  'long'
  'short'

[filipsajdak]

Excellent experiment. It brings even more insights.

[jcanizales]

We want to accept builtin types like int as type ids. Currently this works by accident because the parser does not even recognize these as keywords.

Isn't it desirable to leave the builtin types outside of the reserved words of the grammar? They are identifiers after all, just ones that the user didn't define somewhere else.

[neumannt]

Officially they are keywords, we should not treat them as regular identifiers. Otherwise you can define a function called "int". And they behave a bit strange anyway, for example "unsigned long int" and "long unsigned int" are both valid (and in fact identical). I think that has to be done explicitly, we cannot pretend that these behave like regular identifiers.

[jcanizales]

for example "unsigned long int" and "long unsigned int" are both valid (and in fact identical)

Ok yeah this sold it to me. Even if only one of those two combinations were allowed, unsigned long UserType is senseless.

Unless one were to remove all those keywords anyway and offer only one-word builtins, uint8_t, int32_t, etc.

Giving a function the same name as a type is equally undesirable for builtin or user-defined types. It's a problem of name collision / shadowing, which in my mind is a whole different category from if: () -> void = { }.

[Urfoex]

Even if only one of those two combinations were allowed, unsigned long UserType is senseless.

Unless one were to remove all those keywords anyway and offer only one-word builtins, uint8_t, int32_t, etc.

I would vote for that. The same rule for all types. No special cases.
So either unsigned long int is a type and therefore unsigned long UserType is too.
Or we have, like you said, just uint32_t and no unsigned long int.

Or we treat unsigned and long as some form of concepts or template parameter with which you can constrain or widen the type. For an example I can currently just think of Strings, which often need to be constrained, like having a size from 10 to 12, or having not white-spaces, or being trimmed, or allowing just letters from A-F. Which then could be molded into in more strict type, e.g. using hex_string = no_whitespace trimmed size<4> allowed_chars<HEX> string;
(Just a spontaneous thought.)

[neumannt]

I would vote for that. The same rule for all types. No special cases. So either unsigned long int is a type and therefore unsigned long UserType is too. Or we have, like you said, just uint32_t and no unsigned long int.

But that breaks compatibility with existing C++ code. Note that, e.g., long and long long are two distinct types, with distinct overloads, even if both map to 64bit integers. Silly, I know, but if we only support int64_t we cannot seamlessly interact with existing code bases.

[jcanizales]

It wouldn't be hard to convince me that leaving this craziness behind is a worthwhile goal, even without the issue of multiword types we were discussing. I've been bitten by char being signed or not depending on the platform.

Can still provide them for compatibility without making them the path of least resistance. E.g. by offering compat::unsigned_long_long etc. that the transpiler would special-case to produce the equivalent C++ type. This would increase the complexity of one part of the transpiler, with the hope of simplifying another part (the grammar). But I don't know if the tradeoff is worthwhile. It does seem to me to be simpler for the user.

[JohelEGP]

In what context is a multi-identifier type a problem? I think they might be OK in declarations, thanks to the : giving a heads up to the parser. Perhaps those other contexts could require :, when a single identifier is otherwise expected, to specify a multi-identifier type.

[neumannt]

From a parsing perspective it is not a problem, it is just that the rules for fundamental types are a bit funny. For example signed short int, short int, short signed int, and short are all the same type. Thus the parser has to recognize that this is a fundamental type and has to normalize it into a certain form. You cannot pretend that a fundamental type is just like a regular user defined type.

[JohelEGP]

Thus the parser has to recognize that this is a fundamental type and has to normalize it into a certain form.

Normalizing this is not a problem for cppfront since Herb wants the Cpp1 source to resemble what the user wrote for Cpp2.

[TheRustifyer]

I would vote for that. The same rule for all types. No special cases. So either unsigned long int is a type and therefore unsigned long UserType is too. Or we have, like you said, just uint32_t and no unsigned long int.

But that breaks compatibility with existing C++ code. Note that, e.g., long and long long are two distinct types, with distinct overloads, even if both map to 64bit integers. Silly, I know, but if we only support int64_t we cannot seamlessly interact with existing code bases.

But this could be a nice feature for the only allowed cpp2 code. Without the flag, backwards compatibility is not broken. With the flag, welcome to the real modern C++!

[neumannt]

Normalizing this is not a problem for cppfront since Herb wants the Cpp1 source to resemble what the user wrote for Cpp2.

but I think in the long run that is not sufficient. cppfront is currently more like a preprocessor, it takes one syntax and transforms it into another. That has the advantage that the system does not have to understand the semantic of the program, but it has the disadvantage that the compiler cannot do error checking and it also cannot do type resolution or name lookup. Therefore I have started implementing full semantic analysis in my little experiment, e.g., the compiler infers all data types and checks if the program is valid. And for that you need proper handling of fundamental types, simple text replacement is not good enough.

[JohelEGP]

I understand that. And you mentioned that parsing a multi-identifier type is not a problem. So I take it that this was a digression from the main issue.

[jcanizales]

This was all about one specific point of the many @neumannt discovered with his reimplementation of the parser: "We want to accept builtin types like int as type ids. Currently this works by accident because the parser does not even recognize these as keywords." I suggested that what can be considered an accident of the current implementation (builtin types aren't reserved keywords), feels to me like should be a feature.