keysyms: Check clashes between keysyms names and keywords

[wismill]

Due to how our parser is implemented, keysyms names that are also valid keywords require special handling.

Added a check for these clashes in the keysym generator. The only current clash, section, is already handled. Note that it means that section, Section, sEcTiOn all parse to the same keysym.

Hopefully we will never have a clash again, but the keysyms are not a frozen set.

---

I got puzzled by the following excerpt in parser.y and decided to investigate.

KeySym          :       IDENT
                …
                |       SECTION { $$ = XKB_KEY_section; }

[bluetech]

Looks good to me!

[bluetech]

I got puzzled by the following excerpt in parser.y and decided to investigate.

Perhaps also add a comment on this line?

[wismill]

I got puzzled by the following excerpt in parser.y and decided to investigate.

Perhaps also add a comment on this line?

@bluetech Good point, added.

As a general note, if I had to rewrite the parser, I would opt for the “parse, don’t validate” approach.

I wonder if it would make sense to switch to using Tree-sitter someday. I would love to get as a bonus correct syntax highlighting for XKB files.

[bluetech]

As a general note, if I had to rewrite the parser, I would opt for the “parse, don’t validate” approach.

The article is really good. But a yacc grammar is pretty much "parse" no? I'm curious which part you consider the "validate".

[bluetech]

I wonder if it would make sense to switch to using Tree-sitter someday. I would love to get as a bonus correct syntax highlighting for XKB files.

Maybe these days there is an AI clever enough to translate a yacc grammar to a tree-sitter grammar. In my experience they are quite good with translations like this.

[wismill]

As a general note, if I had to rewrite the parser, I would opt for the “parse, don’t validate” approach.

The article is really good. But a yacc grammar is pretty much "parse" no? I'm curious which part you consider the "validate".

@bluetech My point is that we use a bunch of Expr that require further check on their type after the yacc parsing. So Expr is too general and we may end up with error with no reference to erroneous file/line. Consider the silly:

xkb_keymap { };
    xkb_keycodes { };
    xkb_types {
        type "XXX" {
        modifiers[1234] = Shift;
        map[Shift] = Level2;
        level_name[Level1] = "Base";
        level_name[Level2] = "Number";
        };
    };
    xkb_compat { };
    xkb_symbols { };
};

You would just get the following warning, without any reference to the erroneous line:

xkbcommon: WARNING: The modifiers field of a key type is not an array; Illegal array subscript ignored

Imagine now having this in an included key type file: really not ideal for debugging. But at this stage we do not have parser context anymore!

On the other hand, the following file will fail with a better (but still incomplete¹) error²:

xkb_keymap {
    xkb_keycodes { 
      <};> = 123;
      <🦆> = 456;
    };
    xkb_types { };
    xkb_compat { };
    xkb_symbols { };
};

That said, I guess that fully parsing a section is slower than the current approach so any include would get slower with the parse, don’t validate approach applied strictly.

Footnotes

1. (unknown file) instead of the path of the relevant file. ↩

2. Guess what line is erroneous? The <🦆> one, not the <};> one… ↩