[PATCH] D129373: [NFC] Minor cleanup of usage of FloatModeKind with bitmask enums

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added inline comments.


================
Comment at: clang/include/clang/Basic/TargetInfo.h:894
   bool useObjCFPRetForRealType(FloatModeKind T) const {
-    return RealTypeUsesObjCFPRetMask & llvm::BitmaskEnumDetail::Underlying(T);
+    return (int)((FloatModeKind)RealTypeUsesObjCFPRetMask & T);
   }
----------------
jolanta.jensen wrote:
> shafik wrote:
> > Why not just cast directly to `bool`? 
> Yes, it will work. But as int is the underlying type I find this way a bit clearer. I hope to be forgiven if I keep it.
I'm ok in either case. It wasn't clear to me what the rationale was for casting to `int`; your explanation was helpful. Since the underlying type of an enumeration tends to not be well understood (I had to go look up the rules again), it would be nice if we could use C++23 `std::to_underlying()`:
  return std::to_underlying((FloatModeKind)RealTypeUsesObjCFPRetMask & T);
LLVM's `BitmaskEnum.h` has `Underlying()`, but only exposed in the detail namespace and I don't think it is worth changing just for this. We could do:
  return static_cast<std::underlying_type_t<FloatModeKind>>((FloatModeKind)RealTypeUsesObjCFPRetMask & T);
But I personally don't find that to be an improvement in this case. I think the code is fine as is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129373