[PATCH] D45534: Add isl operator overloads for isl::pw_aff (Try II)

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added a comment.

Did you consider an `NFC` tag for the title?

We could also overload `!=`, `==`, `+=`, `-=`. `*=`, `/=`. Would these be added in a follow-up patch?



================
Comment at: include/polly/Support/ISLOperators.h:1
+//===------ IslOperators.h --------------------------------------*- C++ -*-===//
+//
----------------
File name has capitalized `ISL`


================
Comment at: include/polly/Support/ISLOperators.h:13
+//===----------------------------------------------------------------------===//
+
+#include "isl/isl-noexceptions.h"
----------------
[serious] Header guard still missing


================
Comment at: include/polly/Support/ISLOperators.h:18
+
+// Addition
+
----------------
Consider using `@{` so doxygen knowns to which functions the comment applies


================
Comment at: include/polly/Support/ISLOperators.h:41
+
+inline isl::pw_aff operator+(isl::pw_aff Left, int IntRight) {
+  isl::ctx Ctx = Left.get_ctx();
----------------
[serious] Use 'long' instead of 'int'. That's what isl::val's ctor takes.


================
Comment at: unittests/Isl/IslTest.cpp:350
+
+  isl_ctx_free(IslCtx);
+}
----------------
[serious] The other tests use 
```
std::unique_ptr<isl_ctx, decltype(&isl_ctx_free)> Ctx(isl_ctx_alloc(), &isl_ctx_free);
```
to not require manual releasing the `isl_ctx`. The dtors of the `pw_aff` objects will also be only called at the end of the scop, i.e. /after/ their parent `isl_ctx` is free'd.


Repository:
  rPLO Polly

https://reviews.llvm.org/D45534