[PATCH] D100380: [Polly] In getBuild() use isl::noexception bindings

Michael Kruse via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 14 23:19:00 PDT 2021


Meinersbur added a comment.

Thank you for the patch and thanks @patacca for helping with the review.

Could you add `NFC` to the patch title. It signifies it is a refactoring. <https://llvm.org/docs/Lexicon.html#nfc> (I prefer ir appended as `NFC.` to the end of the title)

Could you upload the patch with context? See https://www.llvm.org/docs/Phabricator.html#phabricator-request-review-web for details (tldr: Add `-U999999` to `git diff`)

The LLVM coding prefers not using auto except in specific cases <https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable>. The current code base may not adhere to this everywhere, but the style could be made more conformant when making changes.

The harbormaster build fails indicate that there is some memory corruption going on. At a first glance I suspect it is the return value of `IslAstInfo::getBuild`. Line 722 of IslAst.cpp calls `isl::manage_copy` on it, which in case of `__isl_give` would usually be memory leak. The `__isl_give` might not actually be correct, repectively  `IslAstInfo::getBuild` should have made a copy before returning.


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

https://reviews.llvm.org/D100380



More information about the llvm-commits mailing list