[llvm] [mlir] [Python] Develop python bindings for Presburger library (PR #113233)

[Christopher Bate via llvm-commits]

christopherbate wrote:

> > This will need some top-level documentation about object ownership. Tableaus are rather large
> 
> I don't think that's true? They just hold `MlirPresburgerIntegerRelation` which is our standard opaque-ish pointer thing?

That's right, the `PyPresburgerTableau` there is just to assist with the indirection of which C API method to call, in order to enable the `relation.equalities[...]` API.

> 
> > and I see a bunch of `keep_alive` that are difficult to follow without knowing the overall goal.
> 
> There are only two and I believe they're to prevent a use-after-free here:
> 
> https://github.com/llvm/llvm-project/blob/0744372033893bf33ee3e8ae71241a2d8239a421/mlir/lib/Bindings/Python/Presburger.cpp#L48-L52
> 
> I made a comment above that it would be simpler to just hoist `at64` to `PyPresburgerRelation` itself, which would resolve your qualm as well.

I don't see how the tableau object could be liminated if we want to keep the `relation.equalities[...]` API. We would need to move to a `relation.at_eq(...)` API, which seems less satisfying, but is overall simpler. 

I think the current implementation is correct in terms of ensuring that `PyPresburgerRelation` should not be freed by Python while any connected `PyPresburgerTableau` are live. 

The `py::keep_alive` syntax is confusing, so it might be better to add in some readability hints, e.g.

```
// here "0" refers to the return value, "1" refers to the PyPresburgerRelation self argument
py::keep_alive</*nurse=*/0, /*patient=*/1>
```

@shelkesagar29 We also probably want to add an explicit test for ensuring this is correct, some thing like


```
# construct relation
rel = IntegerRelation(...)
eq = rel.equalities
....


# Delete rel and collect garbage
del rel
gc.collect()

# call some more methods on "eq"
print(eq[i, j])


```


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