[PATCH] D152399: [CodeGen] Fine tune MachineFunctionSplitPass (MFS) for FSAFDO.

[Han Shen via Phabricator via llvm-commits]

shenhan added a comment.

In D152399#4446768 <https://reviews.llvm.org/D152399#4446768>, @tschuett wrote:

> For optionals please do not use `.has_value()`.
>
>   if (optional) {
>   }
>   
>   if (optional.has_value()) {
>   }
>
> Is identical. Please use `.has_value()`only in unit tests.
>
> Furthermore `*optional` and `optional.value()` are identical. The former is used more wildly.

Thanks.

My rationale here to use explicit ".has_value()" is that when "std::optional<bool> v" is used in if (!v) or if(v). "std::optional<bool> v=false" leads to "if (v)" equals true is counter intuitive to the point of being confusing.

But as you said, to be consistent with llvm existing code practice, I'll drop ".has_value() and .value()" (but put a comment for .has_value() part).

Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152399