[PATCH] D138934: mlir/tblgen: use std::optional in generation

Kazu Hirata via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 29 12:01:25 PST 2022


kazu added a comment.

@artagnon, thanks for your help. I have a couple of concerns:

- You seem to touch formatting in a few places.  Could you keep unrelated parts intact?  Alternatively, you can format the entire file in a separate patch so that your modification will stand out by itself.  You can use `git clang-format HEAD~` to format the parts you touch.
- Somewhat related to formatting, your patch touches the documentation a lot.  If it's not related to the `std::optional` migration, please upload a separate patch.
- `llvm::None` is interchangeable with `std::nullopt`.  If I were you, I might consider replacing `llvm::None` with `std::nullopt` first.  This way, even if this patch causes a regression and get reverted, the patch to migrate to `std::nullopt` should stay, resulting in a net progress.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138934



More information about the llvm-commits mailing list