[PATCH] D139683: [APInt] provide a safe API for zext value and sext value.

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: llvm/include/llvm/ADT/APInt.h:1501
+    return (getActiveBits() <= 64) ? std::optional<uint64_t>(getZExtValue())
+                                   : std::optional<uint64_t>();
+  };
----------------
Peter wrote:
> Peter wrote:
> > RKSimon wrote:
> > > tschuett wrote:
> > > > RKSimon wrote:
> > > > > tschuett wrote:
> > > > > > `std::nullopt`?
> > > > > +1
> > > > `std::optional<uint64_t>(std::nullopt)`.  --> `std::nullopt` .
> > > > 
> > > > `std::optional<uint64_t>(getZExtValue())` ->. `getZExtValue()`.
> > > > 
> > > Any reason you can't remove the std::optional<uint64_t>() cast?
> > Err if I remove both cast, the static type of two branches won't match. Seems implicit cast didn't kick in.
> > ```
> > llvm/include/llvm/ADT/APInt.h:1500:36: error: incompatible operand types ('uint64_t' (aka 'unsigned long') and 'const std::nullopt_t')
> >     return (getActiveBits() <= 64) ? getZExtValue() : std::nullopt;
> >                                    ^ ~~~~~~~~~~~~~~   ~~~~~~~~~~~~
> > ```
> > 
> > Do you know any work arounds?
> Oh, If I rewrite this as an if-branch that works:
> 
> ```
>     if (getActiveBits() <= 64)
>       return getZExtValue();
>     return std::nullopt;
> ```
> 
> Which one would you prefer?
Ah - of course! Keeping as it is is fine - thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139683