Bug in ODS regions generator

MLIR
1

While reviewing code for an unrelated feature, I ran across what looks like a typo.

OpFormatGen.cpp:

/// This class represents the `operands` directive. This directive represents
/// all of the operands of an operation.
using OperandsDirective = DirectiveElement<Element::Kind::OperandsDirective>;

/// This class represents the `regions` directive. This directive represents
/// all of the regions of an operation.
using RegionsDirective = DirectiveElement<Element::Kind::ResultsDirective>;

/// This class represents the `results` directive. This directive represents
/// all of the results of an operation.
using ResultsDirective = DirectiveElement<Element::Kind::ResultsDirective>;

Note that the RegionsDirective is aliasing to the Results kind. I stared at this for a few minutes, couldn’t figure out why it is working/not failing completely and decided to just phone it in.

2

It works because of sheer luck. The regions directive and the results directive can only be used in disjoint contexts, meaning that there isn’t a situation in which the presence of one could create a conflict for the use of the other. Sent out reviews.llvm.org/D94988 to fix it (Note: sent for review instead of submitting directly given that there is no test case that I can write that would fail right now, but not after the patch. Figured that at least needs an additional LGTM).

– River