[RFC] documenting when something is not added for a reason

[ftynse]

After answering the “what is the reason for <blah> not being available in MLIR” with “nobody needed it yet” for around thousandth time, I propose we adopt a policy of always explicitly documenting when we decided against adding a small feature at the place where it would have been added. For example, a comment in the list of traits for floating-point binary arithmetic operation “// IEEE FP arithmetic is not commutative”. The definition of “small” is vague, but basically I’m thinking anything that wouldn’t require an RFC in the opinion of relevant code reviewers.

[clattner]

That’s a good idea. I think it could also be useful to dust off the rationale docs: we captured some design premises early as they were moving quickly early on, but fell out of that habit.

By now I think they are fairly historical and could use a good pass over them.

-Chris

[banach-space]

Thanks for proposing this! This would make the codebase much friendlier to newcomers like myself. Big +1.

In general, we should strive to document most design decisions (within reason). Sometimes a simple note saying that “this is not supported” can save folks from asking themselves: What am I missing here?

Not sure what sort of granularity you had in mind, but some things can be documented through tests (we used that a lot for e.g. unsupported conversions in Flang).

-Andrzej

[_sean_silva]

I think another way we could help this is to be more consistent about adding comments indicating the known non-existence-but-straightforward-implementability of something. E.g. # TODO: Fill out support all the other X comments. E.g. before BF16 and F16 support were added here, it would have been nice to have a comment here saying Add BF16 and F16 support.

[ftynse]

@clattner

I think it could also be useful to dust off the rationale docs: we captured some design premises early as they were moving quickly early on, but fell out of that habit.

I hesitated to add more stuff to the rationale doc, but it always felt like dialect- or even subsystem-specific decisions don’t belong there. It’s not exactly the case of the current content for historical reasons, but I don’t think we want to add “dialect X doesn’t want operation Y” there.

@banach-space

In general, we should strive to document most design decisions (within reason). Sometimes a simple note saying that “this is not supported” can save folks from asking themselves: What am I missing here?

I’d like to add intent to that: there’s a difference between “this is intentionally not supported” and the cases I have in mind.

@_sean_silva

I think another way we could help this is to be more consistent about adding comments indicating the known non-existence-but-straightforward-implementability of something. E.g. # TODO: Fill out support all the other X comments. E.g. before BF16 and F16 support were added here, it would have been nice to have a comment here saying Add BF16 and F16 support.

This requires forward thinking of what can be useful to have. In the opposite case, we will have decided that something is not worth having, so it’s only a matter of documenting that decision (and also potentially drawing the attention of reviewers to that for further discussion).