antlr / grammars-v4

Grammars written for ANTLR v4; expectation that the grammars are free of actions.
MIT License
10.2k stars 3.71k forks source link

"EMPTY" is a reserved word in CSharp target #2225

Open kaby76 opened 3 years ago

kaby76 commented 3 years ago

I'm updating the PL/SQL grammar and have noticed now this bit of lovely:

C:\msys64\home\kenne\plsql\plsql\Generated\obj\Debug\net5.0\PlSqlParser.cs(122533,65): warning CS0108: 'PlSqlParser.Logical_operationContext.EMPTY()' hides inherited member 'ParserRuleContext.EMPTY'. Use the new keyword if hiding was intended. [C:\msys64\home\kenne\plsql\plsql\Generated\Test.csproj]
C:\msys64\home\kenne\plsql\plsql\Generated\obj\Debug\net5.0\PlSqlParser.cs(128199,65): warning CS0108: 'PlSqlParser.Other_functionContext.EMPTY()' hides inherited member 'ParserRuleContext.EMPTY'. Use the new keyword if hiding was intended. [C:\msys64\home\kenne\plsql\plsql\Generated\Test.csproj]
C:\msys64\home\kenne\plsql\plsql\Generated\obj\Debug\net5.0\PlSqlParser.cs(164319,65): warning CS0108: 'PlSqlParser.Non_reserved_keywords_pre12cContext.EMPTY()' hides inherited member 'ParserRuleContext.EMPTY'. Use the new keyword if hiding was intended. [C:\msys64\home\kenne\plsql\plsql\Generated\Test.csproj]

Looking at the Antlr runtime, it looks like it's a constant declared in ParserRuleContext.

Therefore, declaring EMPTY in the lexer creates a conflict with the symbol in the runtime. The solution is to either change the runtime (very, very difficult to do, and unsure as to how long it will be integrated into the next version), or rename EMPTY to EMPTY_ in all grammars.

KvanTTT commented 3 years ago

Changing identifiers on the generation stage is definitely much better but required generator changes. Such change likely won't be accepted because the author says:

I think I'd like to leave the code generation as it is. Your solution is not bad but I'm mostly in maintenance mode and don't want to make a lot of changes.

Thus could you please create an issue about conflicting EMPTY reserved keyword in the ANTLR repository (and ideally fix it) and rename such keyword to EMPTY_ in this repository?

kaby76 commented 3 years ago

See https://github.com/antlr/antlr4/issues/3199. I think NULL is another example that isn't checked, fails (for obvious reasons) when compiling for a Cpp target. "EMPTY" is used in mdx, mysql, tsql, some examples in antlr4 (should probably also correct), wkt. I'll make a change to rename, and maybe a fix over in the Antlr tool, after I get through PR#2223 and some other work I am doing for PL/SQL.