[RFC][Style] Use `continue` and early exits in Semantics

Subprojects Flang
1

This proposes to use continue and early exits in the Semantics code.

The flang C++ style already prescribes the use of the LLVM style in the absence of direct recommendations against it.

The LLVM style clearly recommends using early exits, so this RFC could appear to be pointless, however, the opposite to that guideline seems to have established itself as a common practice.

See example

Not the author’s fault, just the effect of adhering to the nest-all convention:

void OmpStructureChecker::CheckSharedBindingInOuterContext(
    const parser::OmpObjectList &redObjectList) {
  //  TODO: Verify the assumption here that the immediately enclosing region is
  //  the parallel region to which the worksharing construct having reduction
  //  binds to.
  if (auto *enclosingContext{GetEnclosingDirContext()}) {
    for (auto it : enclosingContext->clauseInfo) {
      llvmOmpClause type = it.first;
      const auto *clause = it.second;
      if (llvm::omp::privateReductionSet.test(type)) {
        if (const auto *objList{GetOmpObjectList(*clause)}) {
          for (const auto &ompObject : objList->v) {
            if (const auto *name{parser::Unwrap<parser::Name>(ompObject)}) {
              if (const auto *symbol{name->symbol}) {
                for (const auto &redOmpObject : redObjectList.v) {
                  if (const auto *rname{
                          parser::Unwrap<parser::Name>(redOmpObject)}) {
                    if (const auto *rsymbol{rname->symbol}) {
                      if (rsymbol->name() == symbol->name()) {
                        context_.Say(GetContext().clauseSource,
                            "%s variable '%s' is %s in outer context must"
                            " be shared in the parallel regions to which any"
                            " of the worksharing regions arising from the "
                            "worksharing construct bind."_err_en_US,
                            parser::ToUpperCaseLetters(
                                getClauseName(llvm::omp::Clause::OMPC_reduction)
                                    .str()),
                            symbol->name(),
                            parser::ToUpperCaseLetters(
                                getClauseName(type).str()));
                      }
                    }
                  }
                }
              }
            }
          }
        }
      }
    }
  }
}

This RFC is to clarify (or establish, if it’s not a guideline) use of “early exits” as the practice to follow.

10 Likes
2

+1 to prefer early exits. Authors/reviewers may just need to be aware of it.

Same example with early exits 
void OmpStructureChecker::CheckSharedBindingInOuterContext(
    const parser::OmpObjectList &redObjectList) {
  //  TODO: Verify the assumption here that the immediately enclosing region is
  //  the parallel region to which the worksharing construct having reduction
  //  binds to.
  auto enclosingContext{GetEnclosingDirContext()};
  if (!enclosingContext)
    return;

  for (auto [type, clause] : enclosingContext->clauseInfo) {
    if (!llvm::omp::privateReductionSet.test(type))
      continue;

    const auto *objList{GetOmpObjectList(*clause)};
    for (const auto &ompObject : objList->v) {
      const auto *name{parser::Unwrap<parser::Name>(ompObject)};
      if (!name)
        continue;

      const auto *symbol{name->symbol};
      if (!symbol)
        continue;

      for (const auto &redOmpObject : redObjectList.v) {
        const auto *rname{parser::Unwrap<parser::Name>(redOmpObject)};
        if (!rname)
          continue;

        const auto *rsymbol{rname->symbol};
        if (!rsymbol || rsymbol->name() != symbol->name())
          continue;

        context_.Say(GetContext().clauseSource,
            "%s variable '%s' is %s in outer context must be shared in the "
            "parallel regions to which any of the worksharing regions arising "
            "from the worksharing construct bind."_err_en_US,
            parser::ToUpperCaseLetters(
                getClauseName(llvm::omp::Clause::OMPC_reduction).str()),
            symbol->name(),
            parser::ToUpperCaseLetters(getClauseName(type).str()));
      }
    }
  }
}
1 Like