[APFloat] Remove 'else' after return. NFC

[mehdi_amini]

llvm/include/llvm/ADT/APFloat.h

772–774 ↗

(On Diff #81979)

Why aren't you doing the same transformation as before here?

[timshen]

llvm/include/llvm/ADT/APFloat.h

772–774 ↗

(On Diff #81979)

This will also work:

if (usesLayout<IEEEFloat>(*U.semantics))
  return U.IEEE.makeInf(Neg);
if (usesLayout<DoubleAPFloat>(*U.semantics))
  return U.Double.makeInf(Neg);
llvm_unreachable("Unexpected semantics");

But @jtony have opinions on returning an expression as void in D27872, and suggested removing the void.

I'm slightly in favor of the early return version, since it's more consistent with other places; but I didn't have many data points from others. I'm happy to change.

[mehdi_amini]

llvm/include/llvm/ADT/APFloat.h

772–774 ↗

(On Diff #81979)

This seems both more compact, and consistent with the other cases, so it has my preference.
I don't object to the argument that someone may find undesirable to return a void call, but then we should call it out in the coding guidelines.

[timshen]

Consistently use early return style.

[mehdi_amini]

LGTM, thanks.

llvm/trunk/include/llvm/ADT/APFloat.h

explicit Storage(DoubleAPFloat F, const fltSemantics &S)
         : Double(std::move(F)) {
       assert(&S == &PPCDoubleDouble());
     }
 
     template <typename... ArgTypes>
     Storage(const fltSemantics &Semantics, ArgTypes &&... Args) {
       if (usesLayout<IEEEFloat>(Semantics)) {
         new (&IEEE) IEEEFloat(Semantics, std::forward<ArgTypes>(Args)...);
-      } else if (usesLayout<DoubleAPFloat>(Semantics)) {
+        return;
+      }
+      if (usesLayout<DoubleAPFloat>(Semantics)) {
         new (&Double) DoubleAPFloat(Semantics, std::forward<ArgTypes>(Args)...);
-      } else {
-        llvm_unreachable("Unexpected semantics");
+        return;
       }
+      llvm_unreachable("Unexpected semantics");
     }
 
     ~Storage() {
       if (usesLayout<IEEEFloat>(*semantics)) {
         IEEE.~IEEEFloat();
-      } else if (usesLayout<DoubleAPFloat>(*semantics)) {
+        return;
+      }
+      if (usesLayout<DoubleAPFloat>(*semantics)) {
         Double.~DoubleAPFloat();
-      } else {
-        llvm_unreachable("Unexpected semantics");
+        return;
       }
+      llvm_unreachable("Unexpected semantics");
     }
 
     Storage(const Storage &RHS) {
       if (usesLayout<IEEEFloat>(*RHS.semantics)) {
         new (this) IEEEFloat(RHS.IEEE);
-      } else if (usesLayout<DoubleAPFloat>(*RHS.semantics)) {
+        return;
+      }
+      if (usesLayout<DoubleAPFloat>(*RHS.semantics)) {
         new (this) DoubleAPFloat(RHS.Double);
-      } else {
-        llvm_unreachable("Unexpected semantics");
+        return;
       }
+      llvm_unreachable("Unexpected semantics");
     }
 
     Storage(Storage &&RHS) {
       if (usesLayout<IEEEFloat>(*RHS.semantics)) {
         new (this) IEEEFloat(std::move(RHS.IEEE));
-      } else if (usesLayout<DoubleAPFloat>(*RHS.semantics)) {
+        return;
+      }
+      if (usesLayout<DoubleAPFloat>(*RHS.semantics)) {
         new (this) DoubleAPFloat(std::move(RHS.Double));
-      } else {
-        llvm_unreachable("Unexpected semantics");
+        return;
       }
+      llvm_unreachable("Unexpected semantics");
     }
 
     Storage &operator=(const Storage &RHS) {
       if (usesLayout<IEEEFloat>(*semantics) &&
           usesLayout<IEEEFloat>(*RHS.semantics)) {
         IEEE = RHS.IEEE;
       } else if (usesLayout<DoubleAPFloat>(*semantics) &&
                  usesLayout<DoubleAPFloat>(*RHS.semantics)) {
static_assert(std::is_same<T, IEEEFloat>::value ||
                   std::is_same<T, DoubleAPFloat>::value, "");
     if (std::is_same<T, DoubleAPFloat>::value) {
       return &Semantics == &PPCDoubleDouble();
     }
     return &Semantics != &PPCDoubleDouble();
   }
 
   IEEEFloat &getIEEE() {
-    if (usesLayout<IEEEFloat>(*U.semantics)) {
+    if (usesLayout<IEEEFloat>(*U.semantics))
       return U.IEEE;
-    } else if (usesLayout<DoubleAPFloat>(*U.semantics)) {
+    if (usesLayout<DoubleAPFloat>(*U.semantics))
       return U.Double.getFirst().U.IEEE;
-    } else {
-      llvm_unreachable("Unexpected semantics");
-    }
-  }
 
   const IEEEFloat &getIEEE() const {
-    if (usesLayout<IEEEFloat>(*U.semantics)) {
+    if (usesLayout<IEEEFloat>(*U.semantics))
       return U.IEEE;
-    } else if (usesLayout<DoubleAPFloat>(*U.semantics)) {
+    if (usesLayout<DoubleAPFloat>(*U.semantics))
       return U.Double.getFirst().U.IEEE;
-    } else {
-      llvm_unreachable("Unexpected semantics");
-    }
-  }
 
   void makeZero(bool Neg) { getIEEE().makeZero(Neg); }
 
   void makeInf(bool Neg) {
-    if (usesLayout<IEEEFloat>(*U.semantics)) {
+    if (usesLayout<IEEEFloat>(*U.semantics))
       return U.IEEE.makeInf(Neg);
-    } else if (usesLayout<DoubleAPFloat>(*U.semantics)) {
+    if (usesLayout<DoubleAPFloat>(*U.semantics))
       return U.Double.makeInf(Neg);
-    } else {
-      llvm_unreachable("Unexpected semantics");
-    }
-  }
 
   void makeNaN(bool SNaN, bool Neg, const APInt *fill) {
     getIEEE().makeNaN(SNaN, Neg, fill);
   }
 
   void makeLargest(bool Neg) { getIEEE().makeLargest(Neg); }
 
   void makeSmallest(bool Neg) { getIEEE().makeSmallest(Neg); }
class APFloat : public APFloatBase {
   }
 
   explicit APFloat(IEEEFloat F, const fltSemantics &S) : U(std::move(F), S) {}
   explicit APFloat(DoubleAPFloat F, const fltSemantics &S)
       : U(std::move(F), S) {}
 
   cmpResult compareAbsoluteValue(const APFloat &RHS) const {
     assert(&getSemantics() == &RHS.getSemantics());
-    if (usesLayout<IEEEFloat>(getSemantics())) {
+    if (usesLayout<IEEEFloat>(getSemantics()))
       return U.IEEE.compareAbsoluteValue(RHS.U.IEEE);
-    } else if (usesLayout<DoubleAPFloat>(getSemantics())) {
+    if (usesLayout<DoubleAPFloat>(getSemantics()))
       return U.Double.compareAbsoluteValue(RHS.U.Double);
-    } else {
-      llvm_unreachable("Unexpected semantics");
-    }
-  }
 
 public:
   APFloat(const fltSemantics &Semantics) : U(Semantics) {}
   APFloat(const fltSemantics &Semantics, StringRef S);
   APFloat(const fltSemantics &Semantics, integerPart I) : U(Semantics, I) {}
   // TODO: Remove this constructor. This isn't faster than the first one.
   APFloat(const fltSemantics &Semantics, uninitializedTag)
       : U(Semantics, uninitialized) {}
   APFloat(const fltSemantics &Semantics, const APInt &I) : U(Semantics, I) {}
   explicit APFloat(double d) : U(IEEEFloat(d), IEEEdouble()) {}
   explicit APFloat(float f) : U(IEEEFloat(f), IEEEsingle()) {}
   APFloat(const APFloat &RHS) = default;
   APFloat(APFloat &&RHS) = default;
 
   ~APFloat() = default;
 
   bool needsCleanup() const {
-    if (usesLayout<IEEEFloat>(getSemantics())) {
+    if (usesLayout<IEEEFloat>(getSemantics()))
       return U.IEEE.needsCleanup();
-    } else if (usesLayout<DoubleAPFloat>(getSemantics())) {
+    if (usesLayout<DoubleAPFloat>(getSemantics()))
       return U.Double.needsCleanup();
-    } else {
-      llvm_unreachable("Unexpected semantics");
-    }
-  }
 
   /// Factory for Positive and Negative Zero.
   ///
   /// \param Negative True iff the number should be negative.
   static APFloat getZero(const fltSemantics &Sem, bool Negative = false) {
     APFloat Val(Sem, uninitialized);
     Val.makeZero(Negative);
     return Val;
public:
   ///
   /// \param BitWidth - Select float type
   /// \param isIEEE   - If 128 bit number, select between PPC and IEEE
   static APFloat getAllOnesValue(unsigned BitWidth, bool isIEEE = false);
 
   void Profile(FoldingSetNodeID &NID) const { getIEEE().Profile(NID); }
 
   opStatus add(const APFloat &RHS, roundingMode RM) {
-    if (usesLayout<IEEEFloat>(getSemantics())) {
+    if (usesLayout<IEEEFloat>(getSemantics()))
       return U.IEEE.add(RHS.U.IEEE, RM);
-    } else if (usesLayout<DoubleAPFloat>(getSemantics())) {
+    if (usesLayout<DoubleAPFloat>(getSemantics()))
       return U.Double.add(RHS.U.Double, RM);
-    } else {
-      llvm_unreachable("Unexpected semantics");
-    }
-  }
   opStatus subtract(const APFloat &RHS, roundingMode RM) {
-    if (usesLayout<IEEEFloat>(getSemantics())) {
+    if (usesLayout<IEEEFloat>(getSemantics()))
       return U.IEEE.subtract(RHS.U.IEEE, RM);
-    } else if (usesLayout<DoubleAPFloat>(getSemantics())) {
+    if (usesLayout<DoubleAPFloat>(getSemantics()))
       return U.Double.subtract(RHS.U.Double, RM);
-    } else {
-      llvm_unreachable("Unexpected semantics");
-    }
-  }
   opStatus multiply(const APFloat &RHS, roundingMode RM) {
     return getIEEE().multiply(RHS.getIEEE(), RM);
   }
   opStatus divide(const APFloat &RHS, roundingMode RM) {
     return getIEEE().divide(RHS.getIEEE(), RM);
   }
   opStatus remainder(const APFloat &RHS) {
     return getIEEE().remainder(RHS.getIEEE());

llvm/trunk/lib/Support/APFloat.cpp

void DoubleAPFloat::makeNaN(bool SNaN, bool Neg, const APInt *fill) {
   Floats[1].makeZero(false);
 }
 
 } // End detail namespace
 
 APFloat::Storage::Storage(IEEEFloat F, const fltSemantics &Semantics) {
   if (usesLayout<IEEEFloat>(Semantics)) {
     new (&IEEE) IEEEFloat(std::move(F));
-  } else if (usesLayout<DoubleAPFloat>(Semantics)) {
+    return;
+  }
+  if (usesLayout<DoubleAPFloat>(Semantics)) {
     new (&Double)
         DoubleAPFloat(Semantics, APFloat(std::move(F), F.getSemantics()),
                       APFloat(semIEEEdouble));
-  } else {
-    llvm_unreachable("Unexpected semantics");
+    return;
   }
+  llvm_unreachable("Unexpected semantics");
 }
 
 APFloat::opStatus APFloat::convertFromString(StringRef Str, roundingMode RM) {
   return getIEEE().convertFromString(Str, RM);
 }
 
 hash_code hash_value(const APFloat &Arg) { return hash_value(Arg.getIEEE()); }
 
 APFloat::APFloat(const fltSemantics &Semantics, StringRef S)
     : APFloat(Semantics) {
   convertFromString(S, rmNearestTiesToEven);
 }
 
 APFloat::opStatus APFloat::convert(const fltSemantics &ToSemantics,
                                    roundingMode RM, bool *losesInfo) {
   if (&getSemantics() == &ToSemantics)
     return opOK;
   if (usesLayout<IEEEFloat>(getSemantics()) &&
-      usesLayout<IEEEFloat>(ToSemantics)) {
+      usesLayout<IEEEFloat>(ToSemantics))
     return U.IEEE.convert(ToSemantics, RM, losesInfo);
-  } else if (usesLayout<IEEEFloat>(getSemantics()) &&
-             usesLayout<DoubleAPFloat>(ToSemantics)) {
+  if (usesLayout<IEEEFloat>(getSemantics()) &&
     assert(&ToSemantics == &semPPCDoubleDouble);
     auto Ret = U.IEEE.convert(semPPCDoubleDoubleImpl, RM, losesInfo);
-    *this = APFloat(
-        DoubleAPFloat(semPPCDoubleDouble, std::move(*this), APFloat(semIEEEdouble)),
+    *this = APFloat(DoubleAPFloat(semPPCDoubleDouble, std::move(*this),
+                                  APFloat(semIEEEdouble)),
                     ToSemantics);
     return Ret;
-  } else if (usesLayout<DoubleAPFloat>(getSemantics()) &&
-             usesLayout<IEEEFloat>(ToSemantics)) {
+  }
+  if (usesLayout<DoubleAPFloat>(getSemantics()) &&
     auto Ret = getIEEE().convert(ToSemantics, RM, losesInfo);
     *this = APFloat(std::move(getIEEE()), ToSemantics);
     return Ret;
-  } else {
-    llvm_unreachable("Unexpected semantics");
   }
+  llvm_unreachable("Unexpected semantics");
 }
 
 APFloat APFloat::getAllOnesValue(unsigned BitWidth, bool isIEEE) {
   if (isIEEE) {
     switch (BitWidth) {
     case 16:
       return APFloat(semIEEEhalf, APInt::getAllOnesValue(BitWidth));
     case 32: