Possible Bug in IntegerSet/AffineMap Parser

MLIR
#1

Parsing the following IntegerSet in MLIR:

#set = affine_set<(i0, i1) : ()>

gives the following error:

error: expected bare identifier
#set = affine_set<(i0, i1) : ()>
                   ^

On further investigation, the i0 here is parsed as an inttype instead of a bare-identifier. This is contrary to what MLIR Langref claims:

MLIR guarantees identifiers never collide with keywords by prefixing identifiers with a sigil (e.g. %, #, @, ^, !). In certain unambiguous contexts (e.g. affine expressions), identifiers are not prefixed, for brevity. New keywords may be added to future versions of MLIR without danger of collision with existing identifiers.

Is this the intended behaviour of AffineMap/IntegerSet? If yes, could the error be updated to say usage of the reserved word is not allowed and the Langref be updated to mention this? Otherwise, should this set be parsed?

I would be willing to send a patch to fix this based on what the correct behaviour should be (if needed).

#2

I actually reported this in Jan 2020! 45386 – bare identifier parse error when colliding with int/float types – migrated to github issues: bare identifier parse error when colliding with int/float types · Issue #44731 · llvm/llvm-project · GitHub
I definitely agree that the error message is misleading/leaves the user clueless to start with.

#3

Right. This is also not restricted only to int/float types. It also causes problems while using restricted keywords like size:

#set = affine_set<(d0, d1)[size] : ()>

Error:

error: expected bare identifier
#set = affine_set<(d0, d1)[size] : ()>
                           ^

If we can decide the intended behaviour, I can send a patch resolving that github issue.

#4

No keyword in the parser is a “fully reserved” keyword, which is why Attribute/Op/Type parsers don’t have a notion of “reserved” keywords (parseKeyword checks for any bare_identifier/inttype/keyword token). The only things that hit these types of errors are things implemented in the internal parser (the ones that actually need to differentiate the internal token types).

That shouldn’t even be a keyword anymore AFAIK, nothing uses it (same applies to several others). Which goes to the point above, “keyword” tokens are only added when something in the internal implementation wants to check for that identifier directly, it has no bearing on being “reserved” for anything.

– River

#5

Sent a patch fixing this: ⚙ D127076 [MLIR][Parser] Fix AffineParser colliding bare identifiers with primitive types