[Mlir-commits] [mlir] [mlir] Use `getSingleElement`/`hasSingleElement` in various places (PR #131460)

[Jakub Kuderski]

kuhar wrote:

> > yes - I'd argue that even the simple wrappers could/should be tested,
> 
> Right, the question is what is the bar for "tested". For every other kind of utilities, having enough in-tree uses counts as "tested" (we don't have C++ gtests for every possible utility functions historically).

Continuing with these two examples (min_element and getSingleElement), my thought process was roughly:
1. Is the code tested elsewhere. For `min_element`, the underlying c++ function would be tested on the STL side. For getSingleElement, `hasSingleElement` comes with unit tests but not in combination with the surrounding logic.
2. Is it possible that the new uses in the codebase don't fully exercise the code. For min_element, probably not. For getSingleElement: probably yes. We don't expect to hit the assertion anywhere, if the assertion never triggers, we wouldn't notice. We had this issue with cast functions in the pasts that stopped asserting for some time.
3. Is it possible that future modification to the implementation will introduce breakage that will go unnoticed? For min_element, highly unlikely (there aren't many ways to update this code AFAICT). For getSingleElement, potentially yes: say we change `hasSingleElement` to `hasNItemsOrLess(C, 1)` or some other functions that appears to be similar.

That's why I think it was safe to wave min_element through but not getSingleElement.

https://github.com/llvm/llvm-project/pull/131460