[PATCH] D107225: [Polly][Isl] Move to the new-polly-generator branch version of isl-noexceptions.h. NFCI

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added a comment.

Thanks for all the effort. Unfortunately the `boolean` class had a reason to exist. For instance, with isl_set_empty there are 3 possible outcomes:

- `true` if the set is empty
- `false` if the set contains at least one element
- error if the result could not be computed, for instance because the argument is NULL or the max_operations counter has been exceeded.

In `cpp.h`, with exceptions disabled, the 3rd case aborts the program. The argument being NULL could be tested before calling the method, but there is no means to determine whether the max_operations counter is going to be exceeded in the next call. We don't want to crash but to execute some bail-out code. I expected some tests to fail.
`cpp-checked.h` still has a `boolean` class and I think there is no way around using it instead of `cpp.h`. In contrast to `isl-noexceptions.h`'s boolean, the checked on requires the boolean state to be checked for errors to avoid that an `error` state is accidentally interpreted that same way as `true` if used like this:

  isl::boolean = isl_bool_error;
  if (b) {
    ...
  }

I might have given the impression that `cpp.h` is what we need; at that time I though `cpp.h` was just `boolean::checked`, but it is missing the entire `boolean` class.



================
Comment at: polly/include/polly/Support/GICHelper.h:189
 
+/// c++ wrapper for isl's dump() method.
+//@{
----------------
With isl being a C API and the official C++ binding not having a `dump()` method, there is nothing " dump() method" could refer to.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107225/new/

https://reviews.llvm.org/D107225