mehdi_amini added inline comments.

================
Comment at: llvm/include/llvm/ADT/APFloat.h:800
 
-  void makeLargest(bool Neg) { getIEEE().makeLargest(Neg); }
+  void makeLargest(bool Neg) {
+    if (usesLayout<IEEEFloat>(getSemantics())) {
----------------
jtony wrote:
> I know it is allowed to return a void function call inside a void function, 
> but I think this reduces the code readability in general and causes some 
> confusing to some people. Maybe it is better to avoid using this kind of 
> coding style. I think we can simply call each function in each branch without 
> the 'return' keyword, by default, the program will reach the end of function 
> and return.  
> 
> One possible equivalent code:
> 
> void makeNaN(bool SNaN, bool Neg, const APInt *fill) {
>     if (usesLayout<IEEEFloat>(getSemantics())) {
>       U.IEEE.makeNaN(SNaN, Neg, fill);
>     } else if (usesLayout<DoubleAPFloat>(getSemantics())) {
>       U.Double.makeNaN(SNaN, Neg, fill);
>     } else {
>       llvm_unreachable("Unexpected semantics");
>     }
>   }
Two reasons not to do that:

1) It makes it less consistent with all the other cases, and consistency is a 
high criteria for readability
2) returning a function call in a void function makes it so that if the callee 
signature is changed to return something in the future, it can't be ignored 
here (i.e. it makes it less error prone somehow).

Alternatively, you can also write it this way:

```
  if (usesLayout<IEEEFloat>(getSemantics())) {
    U.IEEE.makeNaN(SNaN, Neg, fill);
    return;
  } 
  if (usesLayout<DoubleAPFloat>(getSemantics())) {
    U.Double.makeNaN(SNaN, Neg, fill);
    return;
  } 
  llvm_unreachable("Unexpected semantics");
```


https://reviews.llvm.org/D27872



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to