[flang] Don't use Optional::hasValue (NFC)

[kazu]

Summary

Flang C++ Style Guide tells us to avoid .has_value() in the predicate
expressions of control flow statements. I am treating ternary
expressions as control flow statements for the purpose of this patch.

[kazu]

Friendly ping. Thanks!

[jeanPerier]

Hi @kazu, thanks for making a review and making the code style consistent. It seems part of the change is breaking builds (see bots).

flang/lib/Optimizer/Dialect/FIROps.cpp

1108

This is breaking the build bots:

/var/lib/buildkite-agent/builds/llvm-project/flang/lib/Optimizer/Dialect/FIROps.cpp:1077:7: error: value of type 'mlir::OptionalParseResult' is not contextually convertible to 'bool'
  if (parseResult && failed(*parseResult))
      ^~~~~~~~~~~

Same below in /var/lib/buildkite-agent/builds/llvm-project/flang/lib/Optimizer/Dialect/FIROps.cpp:1259:9

[kazu]

Updated FIROps.cpp and Dialect/FIROps.cpp.

[kazu]

In D128622#3614902, @jeanPerier wrote:

Hi @kazu, thanks for making a review and making the code style consistent. It seems part of the change is breaking builds (see bots).

Oops. Thank you for pointing out the breakage. It looks like I temporarily disabled flang, and I was testing anything. Please take a look again.

[jeanPerier]

Thanks

flang/include/flang/Lower/IterationSpace.h

size_t argPosition(mlir::Value arg) {
     for (auto i : llvm::enumerate(innerArgs))
       if (arg == i.value())
         return i.index();
     llvm_unreachable("inner argument value was not found");
   }
 
   llvm::Optional<fir::ArrayLoadOp> getLhsLoad(size_t i) {
     assert(i < lhsBases.size());
-    if (lhsBases[counter].hasValue())
+    if (lhsBases[counter])
       return findBinding(lhsBases[counter].getValue());
     return llvm::None;
   }
 
   /// Return the outermost loop in this FORALL nest.
   fir::DoLoopOp getOuterLoop() {
     assert(outerLoop.hasValue());
     return outerLoop.getValue();

flang/lib/Lower/ConvertExpr.cpp

private:
   /// Otherwise, \p resultTy is ignored and the expression is evaluated
   /// in the destination. \p f is a continuation built from an
   /// evaluate::Expr or an ExtendedValue.
   ExtValue lowerArrayExpression(CC f, mlir::Type resultTy) {
     mlir::Location loc = getLoc();
     auto [iterSpace, insPt] = genIterSpace(resultTy);
     auto exv = f(iterSpace);
     iterSpace.setElement(std::move(exv));
-    auto lambda = ccStoreToDest.hasValue()
+    auto lambda = ccStoreToDest
                       ? ccStoreToDest.getValue()
                       : defaultStoreToDestination(/*substring=*/nullptr);
     mlir::Value updVal = fir::getBase(lambda(iterSpace));
     finalizeElementCtx();
     builder.create<fir::ResultOp>(loc, updVal);
     builder.restoreInsertionPoint(insPt);
     return abstractArrayExtValue(iterSpace.outerResult());
   }
genIterSpace(mlir::Type resultType) {
     mlir::Location loc = getLoc();
     llvm::SmallVector<mlir::Value> shape = genIterationShape();
     if (!destination) {
       // Allocate storage for the result if it is not already provided.
       destination = createAndLoadSomeArrayTemp(resultType, shape);
     }
 
     // Generate the lazy mask allocation, if one was given.
-    if (ccPrelude.hasValue())
+    if (ccPrelude)
       ccPrelude.getValue()(shape);
 
     // Now handle the implicit loops.
     mlir::Value inner = explicitSpaceIsActive()
                             ? explicitSpace->getInnerArgs().front()
                             : destination.getResult();
     auto [iters, afterLoopNest] = genImplicitLoops(shape, inner);
     mlir::Value innerArg = iters.innerArgument();
genIterSpace(mlir::Type resultType) {
     // We're ready to lower the body (an assignment statement) for this context
     // of loop nests at this point.
     return {iters, afterLoopNest};
   }
 
   fir::ArrayLoadOp
   createAndLoadSomeArrayTemp(mlir::Type type,
                              llvm::ArrayRef<mlir::Value> shape) {
-    if (ccLoadDest.hasValue())
+    if (ccLoadDest)
       return ccLoadDest.getValue()(shape);
     auto seqTy = type.dyn_cast<fir::SequenceType>();
     assert(seqTy && "must be an array");
     mlir::Location loc = getLoc();
     // TODO: Need to thread the LEN parameters here. For character, they may
     // differ from the operands length (e.g concatenation). So the array loads
     // type parameters are not enough.
     if (auto charTy = seqTy.getEleTy().dyn_cast<fir::CharacterType>())
CC genarr(const ExtValue &extMemref, ComponentPath &components) {
     if (isProjectedCopyInCopyOut()) {
       // Semantics are projected copy-in copy-out.
       // The backing store of the destination of an array expression may be
       // partially modified. These updates are recorded in FIR by forwarding a
       // continuation that generates an `array_update` Op. The destination is
       // always loaded at the beginning of the statement and merged at the
       // end.
       destination = arrLoad;
-      auto lambda = ccStoreToDest.hasValue()
+      auto lambda = ccStoreToDest
                         ? ccStoreToDest.getValue()
                         : defaultStoreToDestination(components.substring);
       return [=](IterSpace iters) -> ExtValue { return lambda(iters); };
     }
     if (isCustomCopyInCopyOut()) {
       // Create an array_modify to get the LHS element address and indicate
       // the assignment, the actual assignment must be implemented in
       // ccStoreToDest.
CC genarr(const Fortran::evaluate::ArrayConstructor<A> &x) {
 
     // Cleanup the temporary.
     fir::FirOpBuilder *bldr = &converter.getFirOpBuilder();
     stmtCtx.attachCleanup(
         [bldr, loc, mem]() { bldr->create<fir::FreeMemOp>(loc, mem); });
 
     // Return the continuation.
     if (fir::isa_char(seqTy.getEleTy())) {
-      if (charLen.hasValue()) {
+      if (charLen) {
         auto len = builder.create<fir::LoadOp>(loc, charLen.getValue());
         return genarr(fir::CharArrayBoxValue{mem, len, extents});
       }
       return genarr(fir::CharArrayBoxValue{mem, zero, extents});
     }
     return genarr(fir::ArrayBoxValue{mem, extents});
   }
 
void Fortran::lower::createArrayLoads(
   std::size_t counter = esp.getCounter();
   fir::FirOpBuilder &builder = converter.getFirOpBuilder();
   mlir::Location loc = converter.getCurrentLocation();
   Fortran::lower::StatementContext &stmtCtx = esp.stmtContext();
   // Gen the fir.array_load ops.
   auto genLoad = [&](const auto *x) -> fir::ArrayLoadOp {
     return genArrayLoad(loc, converter, builder, x, symMap, stmtCtx);
   };
-  if (esp.lhsBases[counter].hasValue()) {
+  if (esp.lhsBases[counter]) {
     auto &base = esp.lhsBases[counter].getValue();
     auto load = std::visit(genLoad, base);
     esp.initialArgs.push_back(load);
     esp.resetInnerArgs();
     esp.bindLoad(base, load);
   }
   for (const auto &base : esp.rhsBases[counter])
     esp.bindLoad(base, std::visit(genLoad, base));
 }
 
 void Fortran::lower::createArrayMergeStores(
     Fortran::lower::AbstractConverter &converter,
     Fortran::lower::ExplicitIterSpace &esp) {
   fir::FirOpBuilder &builder = converter.getFirOpBuilder();
   mlir::Location loc = converter.getCurrentLocation();
   builder.setInsertionPointAfter(esp.getOuterLoop());
   // Gen the fir.array_merge_store ops for all LHS arrays.
   for (auto i : llvm::enumerate(esp.getOuterLoop().getResults()))
     if (llvm::Optional<fir::ArrayLoadOp> ldOpt = esp.getLhsLoad(i.index())) {
       fir::ArrayLoadOp load = ldOpt.getValue();
       builder.create<fir::ArrayMergeStoreOp>(loc, load, i.value(),
                                              load.getMemref(), load.getSlice(),
                                              load.getTypeparams());
     }
-  if (esp.loopCleanup.hasValue()) {
+  if (esp.loopCleanup) {
     esp.loopCleanup.getValue()(builder);
     esp.loopCleanup = llvm::None;
   }
   esp.initialArgs.clear();
   esp.innerArgs.clear();
   esp.outerLoop = llvm::None;
   esp.resetBindings();
   esp.incrementCounter();
 }

flang/lib/Lower/IterationSpace.cpp

if (forallContextOpen == 0) {
     outerLoop = llvm::None;
     clearLoops();
     counter = 0;
   }
 }
 
 llvm::Optional<size_t>
 Fortran::lower::ExplicitIterSpace::findArgPosition(fir::ArrayLoadOp load) {
-  if (lhsBases[counter].hasValue()) {
+  if (lhsBases[counter]) {
     auto ld = loadBindings.find(lhsBases[counter].getValue());
     llvm::Optional<size_t> optPos;
     if (ld != loadBindings.end() && ld->second == load)
       optPos = static_cast<size_t>(0u);
     assert(optPos.hasValue() && "load does not correspond to lhs");
     return optPos;
   }
   return llvm::None;
std::visit(Fortran::common::visitors{
                    [&](const Fortran::evaluate::Component *y) {
                      s << "  " << y->GetLastSymbol() << '\n';
                    }},
                u);
   };
   s << "LHS bases:\n";
   for (const llvm::Optional<Fortran::lower::ExplicitIterSpace::ArrayBases> &u :
        e.lhsBases)
-    if (u.hasValue())
+    if (u)
       dump(u.getValue());
   s << "RHS bases:\n";
   for (const llvm::SmallVector<Fortran::lower::ExplicitIterSpace::ArrayBases>
            &bases : e.rhsBases) {
     for (const Fortran::lower::ExplicitIterSpace::ArrayBases &u : bases)
       dump(u);
     s << '\n';
   }

flang/lib/Optimizer/CodeGen/BoxedProcedure.cpp

if (options.useThunks) {
           }
         } else if (auto mem = mlir::dyn_cast<AllocaOp>(op)) {
           auto ty = mem.getType();
           if (typeConverter.needsConversion(ty)) {
             rewriter.setInsertionPoint(mem);
             auto toTy = typeConverter.convertType(unwrapRefType(ty));
             bool isPinned = mem.getPinned();
             llvm::StringRef uniqName;
-            if (mem.getUniqName().hasValue())
+            if (mem.getUniqName())
               uniqName = mem.getUniqName().getValue();
             llvm::StringRef bindcName;
-            if (mem.getBindcName().hasValue())
+            if (mem.getBindcName())
               bindcName = mem.getBindcName().getValue();
             rewriter.replaceOpWithNewOp<AllocaOp>(
                 mem, toTy, uniqName, bindcName, isPinned, mem.getTypeparams(),
                 mem.getShape());
           }
         } else if (auto mem = mlir::dyn_cast<AllocMemOp>(op)) {
           auto ty = mem.getType();
           if (typeConverter.needsConversion(ty)) {
             rewriter.setInsertionPoint(mem);
             auto toTy = typeConverter.convertType(unwrapRefType(ty));
             llvm::StringRef uniqName;
-            if (mem.getUniqName().hasValue())
+            if (mem.getUniqName())
               uniqName = mem.getUniqName().getValue();
             llvm::StringRef bindcName;
-            if (mem.getBindcName().hasValue())
+            if (mem.getBindcName())
               bindcName = mem.getBindcName().getValue();
             rewriter.replaceOpWithNewOp<AllocMemOp>(
                 mem, toTy, uniqName, bindcName, mem.getTypeparams(),
                 mem.getShape());
           }
         } else if (auto coor = mlir::dyn_cast<CoordinateOp>(op)) {
           auto ty = coor.getType();
           mlir::Type baseTy = coor.getBaseType();

flang/lib/Optimizer/CodeGen/CodeGen.cpp

bool isFullRange(mlir::DenseIntElementsAttr indexes,
     }
     return true;
   }
 
   // TODO: String comparaison should be avoided. Replace linkName with an
   // enumeration.
   mlir::LLVM::Linkage
   convertLinkage(llvm::Optional<llvm::StringRef> optLinkage) const {
-    if (optLinkage.hasValue()) {
+    if (optLinkage) {
       auto name = optLinkage.getValue();
       if (name == "internal")
         return mlir::LLVM::Linkage::Internal;
       if (name == "linkonce")
         return mlir::LLVM::Linkage::Linkonce;
       if (name == "linkonce_odr")
         return mlir::LLVM::Linkage::LinkonceODR;
       if (name == "common")
matchAndRewrite(fir::NoReassocOp noreassoc, OpAdaptor adaptor,
     return mlir::success();
   }
 };
 
 static void genCondBrOp(mlir::Location loc, mlir::Value cmp, mlir::Block *dest,
                         llvm::Optional<mlir::ValueRange> destOps,
                         mlir::ConversionPatternRewriter &rewriter,
                         mlir::Block *newBlock) {
-  if (destOps.hasValue())
+  if (destOps)
     rewriter.create<mlir::LLVM::CondBrOp>(loc, cmp, dest, destOps.getValue(),
                                           newBlock, mlir::ValueRange());
   else
     rewriter.create<mlir::LLVM::CondBrOp>(loc, cmp, dest, newBlock);
 }
 
 template <typename A, typename B>
 static void genBrOp(A caseOp, mlir::Block *dest, llvm::Optional<B> destOps,
                     mlir::ConversionPatternRewriter &rewriter) {
-  if (destOps.hasValue())
+  if (destOps)
     rewriter.replaceOpWithNewOp<mlir::LLVM::BrOp>(caseOp, destOps.getValue(),
                                                   dest);
   else
     rewriter.replaceOpWithNewOp<mlir::LLVM::BrOp>(caseOp, llvm::None, dest);
 }
 
 static void genCaseLadderStep(mlir::Location loc, mlir::Value cmp,
                               mlir::Block *dest,
static void selectMatchAndRewrite(fir::LLVMTypeConverter &lowering, OP select,
   llvm::SmallVector<int32_t> caseValues;
 
   for (unsigned t = 0; t != conds; ++t) {
     mlir::Block *dest = select.getSuccessor(t);
     auto destOps = select.getSuccessorOperands(adaptor.getOperands(), t);
     const mlir::Attribute &attr = cases[t];
     if (auto intAttr = attr.template dyn_cast<mlir::IntegerAttr>()) {
       destinations.push_back(dest);
-      destinationsOperands.push_back(destOps.hasValue() ? *destOps
-                                                        : mlir::ValueRange{});
+      destinationsOperands.push_back(destOps ? *destOps : mlir::ValueRange{});
       caseValues.push_back(intAttr.getInt());
       continue;
     }
     assert(attr.template dyn_cast_or_null<mlir::UnitAttr>());
     assert((t + 1 == conds) && "unit must be last");
     defaultDestination = dest;
-    defaultOperands = destOps.hasValue() ? *destOps : mlir::ValueRange{};
+    defaultOperands = destOps ? *destOps : mlir::ValueRange{};
   }
 
   // LLVM::SwitchOp takes a i32 type for the selector.
   if (select.getSelector().getType() != rewriter.getI32Type())
     selector = rewriter.create<mlir::LLVM::TruncOp>(loc, rewriter.getI32Type(),
                                                     selector);
 
   rewriter.replaceOpWithNewOp<mlir::LLVM::SwitchOp>(

flang/lib/Optimizer/CodeGen/TargetRewrite.cpp

for (auto e : llvm::enumerate(
             newInTys.push_back(ty);
             newOpers.push_back(oper);
           });
     }
     newInTys.insert(newInTys.end(), trailingInTys.begin(), trailingInTys.end());
     newOpers.insert(newOpers.end(), trailingOpers.begin(), trailingOpers.end());
     if constexpr (std::is_same_v<std::decay_t<A>, fir::CallOp>) {
       fir::CallOp newCall;
-      if (callOp.getCallee().hasValue()) {
+      if (callOp.getCallee()) {
         newCall = rewriter->create<A>(loc, callOp.getCallee().getValue(),
                                       newResTys, newOpers);
       } else {
         // Force new type on the input operand.
         newOpers[0].setType(mlir::FunctionType::get(
             callOp.getContext(),
             mlir::TypeRange{newInTys}.drop_front(dropFront), newResTys));
         newCall = rewriter->create<A>(loc, newResTys, newOpers);

flang/lib/Optimizer/Dialect/FIROps.cpp

mlir::ParseResult fir::DispatchTableOp::parse(mlir::OpAsmParser &parser,
 
   // Convert the parsed name attr into a string attr.
   result.attributes.set(mlir::SymbolTable::getSymbolAttrName(),
                         nameAttr.getRootReference());
 
   // Parse the optional table body.
   mlir::Region *body = result.addRegion();
   mlir::OptionalParseResult parseResult = parser.parseOptionalRegion(*body);
   if (parseResult.hasValue() && failed(*parseResult))
     return mlir::failure();
 
   fir::DispatchTableOp::ensureTerminator(*body, parser.getBuilder(),
                                          result.location);
   return mlir::success();
 }
 
 void fir::DispatchTableOp::print(mlir::OpAsmPrinter &p) {
auto parseResult =
         parser.parseOptionalRegion(*result.addRegion(), /*arguments=*/{});
     if (parseResult.hasValue() && mlir::failed(*parseResult))
       return mlir::failure();
   }
   return mlir::success();
 }
 
 void fir::GlobalOp::print(mlir::OpAsmPrinter &p) {
-  if (getLinkName().hasValue())
+  if (getLinkName())
     p << ' ' << getLinkName().getValue();
   p << ' ';
   p.printAttributeWithoutType(getSymrefAttr());
   if (auto val = getValueOrNull())
     p << '(' << val << ')';
   if (getOperation()->getAttr(fir::GlobalOp::getConstantAttrNameStr()))
     p << " constant";
   p << " : ";

flang/lib/Optimizer/Support/InternalNames.cpp

 inline llvm::SmallVector<llvm::StringRef>
 convertToStringRef(llvm::ArrayRef<std::string> from) {
   return {from.begin(), from.end()};
 }
 
 inline llvm::Optional<llvm::StringRef>
 convertToStringRef(const llvm::Optional<std::string> &from) {
   llvm::Optional<llvm::StringRef> to;
-  if (from.hasValue())
+  if (from)
     to = from.getValue();
   return to;
 }
 
 static std::string readName(llvm::StringRef uniq, std::size_t &i,
                             std::size_t init, std::size_t end) {
   for (i = init; i < end && (uniq[i] < 'A' || uniq[i] > 'Z'); ++i) {
     // do nothing

flang/lib/Optimizer/Transforms/AffinePromotion.cpp

 private:
   MaybeAffineExpr affineBinaryOp(mlir::AffineExprKind kind, mlir::Value lhs,
                                  mlir::Value rhs) {
     return affineBinaryOp(kind, toAffineExpr(lhs), toAffineExpr(rhs));
   }
 
   MaybeAffineExpr affineBinaryOp(mlir::AffineExprKind kind, MaybeAffineExpr lhs,
                                  MaybeAffineExpr rhs) {
-    if (lhs.hasValue() && rhs.hasValue())
+    if (lhs && rhs)
       return mlir::getAffineBinaryOpExpr(kind, lhs.getValue(), rhs.getValue());
     return {};
   }
 
   MaybeAffineExpr toAffineExpr(MaybeAffineExpr e) { return e; }
 
   MaybeAffineExpr toAffineExpr(int64_t value) {
     return {mlir::getAffineConstantExpr(value, firCondition.getContext())};
if (auto blockArg = value.dyn_cast<mlir::BlockArgument>()) {
       return {mlir::getAffineSymbolExpr(symCount++, value.getContext())};
     }
     return {};
   }
 
   void fromCmpIOp(mlir::arith::CmpIOp cmpOp) {
     auto lhsAffine = toAffineExpr(cmpOp.getLhs());
     auto rhsAffine = toAffineExpr(cmpOp.getRhs());
-    if (!lhsAffine.hasValue() || !rhsAffine.hasValue())
+    if (!lhsAffine || !rhsAffine)
       return;
     auto constraintPair = constraint(
         cmpOp.getPredicate(), rhsAffine.getValue() - lhsAffine.getValue());
     if (!constraintPair)
       return;
     integerSet = mlir::IntegerSet::get(dimCount, symCount,
                                        {constraintPair.getValue().first},
                                        {constraintPair.getValue().second});

flang/lib/Optimizer/Transforms/ExternalNameConversion.cpp

 public:
   using OpRewritePattern::OpRewritePattern;
 
   mlir::LogicalResult
   matchAndRewrite(fir::CallOp op,
                   mlir::PatternRewriter &rewriter) const override {
     rewriter.startRootUpdate(op);
     auto callee = op.getCallee();
-    if (callee.hasValue()) {
+    if (callee) {
       auto result = fir::NameUniquer::deconstruct(
           callee.getValue().getRootReference().getValue());
       if (fir::NameUniquer::isExternalFacingUniquedName(result))
         op.setCalleeAttr(
             SymbolRefAttr::get(op.getContext(), mangleExternalName(result)));
     }
     rewriter.finalizeRootUpdate(op);
     return success();
void ExternalNameConversionPass::runOnOperation() {
   patterns.insert<MangleNameOnCallOp, MangleNameOnCallOp, MangleNameOnFuncOp,
                   MangleNameForCommonBlock, MangleNameOnAddrOfOp>(context);
 
   ConversionTarget target(*context);
   target.addLegalDialect<fir::FIROpsDialect, LLVM::LLVMDialect,
                          acc::OpenACCDialect, omp::OpenMPDialect>();
 
   target.addDynamicallyLegalOp<fir::CallOp>([](fir::CallOp op) {
-    if (op.getCallee().hasValue())
+    if (op.getCallee())
       return !fir::NameUniquer::needExternalNameMangling(
           op.getCallee().getValue().getRootReference().getValue());
     return true;
   });
 
   target.addDynamicallyLegalOp<mlir::func::FuncOp>([](mlir::func::FuncOp op) {
     return !fir::NameUniquer::needExternalNameMangling(op.getSymName());
   });