flang-compiler / f18

F18 is a front-end for Fortran intended to replace the existing front-end in the Flang compiler
229 stars 48 forks source link

Audit code for LLVM style: else after return #1016

Open RichBarton-Arm opened 4 years ago

RichBarton-Arm commented 4 years ago

LLVM projects have a coding style guideline to strongly discourage the use of else after return. The intention is to simplify the control flow and reduce the indentation level in the code to make it more readable.

Outcomes:

  1. All uses of this LLVM style anti-pattern are assessed and either: a) Change to the LLVM style as it fits the code intention b) Change to the LLVM style with additional comments to make clear the intention if we feel the style is not a perfect fit to the intention but good enough c) Keep the style with decent justification in a comment.
  2. If we hit many instances of c) we should start a thread on llvm-dev to have the Style guide clarified in such circumstances to (hopefully) allow the patterns we see in F18 or seek guidance as to what to do.
klausler commented 4 years ago

The use of else after return in f18 is, in most instances, quite intentional.

f18 source avoids else after return when the return is handling an exceptional or error case that obviates the remainder of the sequence, and the corresponding if is not constexpr. These cases include most (all?) instances where the return statements have no expressions.

Where the if condition is constexpr, an else block that follows is present because it is semantically necessary to prevent build-time compilation errors.

In situations where an if/else if/else if/else chain implements a total function, and the predicates of the if statements are functioning like guards in a function definition (or case construct) in a functional programming language, f18 style uses a return in each branch of the construct as a sign that the entire chain is essentially implementing a single return whose result is determined by a complicated conditional expression. This is a natural representation in C++ of the much more functional style used to good effect throughout f18.

However: where f18 uses else after return, the corresponding if is not constexpr, and it is possible for control to flow out of the else block without returning from the enclosing function or lambda, then the else is not being used properly and should be removed. There should not be many instances of this usage pattern in f18.