[flang] Restore Optional::value() (NFC)

[kazu]

Summary

This patch restores several calls to Optional::value() in preference
to Optional::operator*.

The Flang C++ Style Guide tells us to use x.value() where no presence
test is obviously protecting a *x reference to the contents.

[clementval]

I'm a bit lost with your changes here. Earlier you removed the use of the value() or equivalent function and now you are introducing it back? Can we have a clear plan of action on this? Or is it just to stick with our guidelines? Is so there are more to change.

[kazu]

In D128590#3610370, @clementval wrote:

I'm a bit lost with your changes here. Earlier you removed the use of the value() or equivalent function and now you are introducing it back? Can we have a clear plan of action on this? Or is it just to stick with our guidelines? Is so there are more to change.

Yes, I'm trying to correct my early patch and trying to stick with the guidelines.

My plan is to move away from Optional::{hasValue,getValue} while sticking to the coding style of respective subprojects.

This means that for flang, hasValue will be dropped inside control-flow conditions and converted to has_value outside control-flow conditions. getValue will be converted to '*' where the protection is obvious and .value() otherwise.

[clementval]

Ok thanks for the explanation and sticking to our previous guidelines. Lgtm.

flang/lib/Lower/ConvertExpr.cpp

ExtValue lowerScalarUserAssignment(mlir::func::FuncOp userAssignment,
     auto lexv = genarr(lhs)(iters);
     bool isIllFormedLHS = false;
     // 3) Insert the call
     if (auto modifyOp = mlir::dyn_cast<fir::ArrayModifyOp>(
             fir::getBase(lexv).getDefiningOp())) {
       mlir::Value oldInnerArg = modifyOp.getSequence();
       std::size_t offset = explicitSpace->argPosition(oldInnerArg);
       explicitSpace->setInnerArg(offset, fir::getBase(lexv));
-      fir::ExtendedValue exv = arrayModifyToExv(
-          builder, loc, *explicitSpace->getLhsLoad(0), modifyOp.getResult(0));
+      fir::ExtendedValue exv =
+          arrayModifyToExv(builder, loc, explicitSpace->getLhsLoad(0).value(),
+                           modifyOp.getResult(0));
       genScalarUserDefinedAssignmentCall(builder, loc, userAssignment, exv,
                                          elementalExv);
     } else {
       // LHS is ill formed, it is a scalar with no references to FORALL
       // subscripts, so there is actually no array assignment here. The user
       // code is probably bad, but still insert user assignment call since it
       // was not rejected by semantics (a warning was emitted).
       isIllFormedLHS = true;
private:
 
   /// Convert the original value, \p origVal, to type \p eleTy. When in a
   /// pointer assignment context, generate an appropriate `fir.rebox` for
   /// dealing with any bounds parameters on the pointer assignment.
   mlir::Value convertElementForUpdate(mlir::Location loc, mlir::Type eleTy,
                                       mlir::Value origVal) {
     mlir::Value val = builder.createConvert(loc, eleTy, origVal);
     if (isBoundsSpec()) {
-      auto lbs = *lbounds;
+      auto lbs = lbounds.value();
       if (lbs.size() > 0) {
         // Rebox the value with user-specified shift.
         auto shiftTy = fir::ShiftType::get(eleTy.getContext(), lbs.size());
         mlir::Value shiftOp = builder.create<fir::ShiftOp>(loc, shiftTy, lbs);
         val = builder.create<fir::ReboxOp>(loc, eleTy, val, shiftOp,
                                            mlir::Value{});
       }
     } else if (isBoundsRemap()) {
for (const Fortran::evaluate::ArrayConstructorValue<A> &acv : x.values()) {
       mlir::Value eleSz = computeElementSize(exv, eleTy, resTy);
       mem = copyNeeded ? copyNextArrayCtorSection(exv, buffPos, buffSize, mem,
                                                   eleSz, eleTy, eleRefTy, resTy)
                        : fir::getBase(exv);
       if (fir::isa_char(seqTy.getEleTy()) && !charLen) {
         charLen = builder.createTemporary(loc, builder.getI64Type());
         mlir::Value castLen =
             builder.createConvert(loc, builder.getI64Type(), fir::getLen(exv));
-        builder.create<fir::StoreOp>(loc, castLen, *charLen);
+        builder.create<fir::StoreOp>(loc, castLen, charLen.value());
       }
     }
     stmtCtx.finalize(/*popScope=*/true);
 
     builder.create<fir::ResultOp>(loc, mem);
     builder.restoreInsertionPoint(insPt);
     mem = loop.getResult(0);
     symMap.popImpliedDoBinding();
     llvm::SmallVector<mlir::Value> extents = {
         builder.create<fir::LoadOp>(loc, buffPos).getResult()};
 
     // Convert to extended value.
     if (fir::isa_char(seqTy.getEleTy())) {
-      auto len = builder.create<fir::LoadOp>(loc, *charLen);
+      auto len = builder.create<fir::LoadOp>(loc, charLen.value());
       return {fir::CharArrayBoxValue{mem, len, extents}, /*needCopy=*/false};
     }
     return {fir::ArrayBoxValue{mem, extents}, /*needCopy=*/false};
   }
 
   // To simplify the handling and interaction between the various cases, array
   // constructors are always lowered to the incremental construction code
   // pattern, even if the extent of the array value is constant. After the
for (const auto &expr : x) {
       mlir::Value eleSz = computeElementSize(exv, eleTy, resTy);
       mem = copyNeeded ? copyNextArrayCtorSection(exv, buffPos, buffSize, mem,
                                                   eleSz, eleTy, eleRefTy, resTy)
                        : fir::getBase(exv);
       if (fir::isa_char(seqTy.getEleTy()) && !charLen) {
         charLen = builder.createTemporary(loc, builder.getI64Type());
         mlir::Value castLen =
             builder.createConvert(loc, builder.getI64Type(), fir::getLen(exv));
-        builder.create<fir::StoreOp>(loc, castLen, *charLen);
+        builder.create<fir::StoreOp>(loc, castLen, charLen.value());
       }
     }
     mem = builder.createConvert(loc, fir::HeapType::get(resTy), mem);
     llvm::SmallVector<mlir::Value> extents = {
         builder.create<fir::LoadOp>(loc, buffPos)};
 
     // Cleanup the temporary.
     fir::FirOpBuilder *bldr = &converter.getFirOpBuilder();

flang/lib/Optimizer/CodeGen/CodeGen.cpp

matchAndRewrite(fir::SelectCaseOp caseOp, OpAdaptor adaptor,
     mlir::Value selector = caseOp.getSelector(adaptor.getOperands());
     auto loc = caseOp.getLoc();
     for (unsigned t = 0; t != conds; ++t) {
       mlir::Block *dest = caseOp.getSuccessor(t);
       llvm::Optional<mlir::ValueRange> destOps =
           caseOp.getSuccessorOperands(adaptor.getOperands(), t);
       llvm::Optional<mlir::ValueRange> cmpOps =
           *caseOp.getCompareOperands(adaptor.getOperands(), t);
-      mlir::Value caseArg = *(cmpOps->begin());
+      mlir::Value caseArg = *(cmpOps.value().begin());
       mlir::Attribute attr = cases[t];
       if (attr.isa<fir::PointIntervalAttr>()) {
         auto cmp = rewriter.create<mlir::LLVM::ICmpOp>(
             loc, mlir::LLVM::ICmpPredicate::eq, selector, caseArg);
         genCaseLadderStep(loc, cmp, dest, destOps, rewriter);
         continue;
       }
       if (attr.isa<fir::LowerBoundAttr>()) {
for (unsigned t = 0; t != conds; ++t) {
         auto cmp = rewriter.create<mlir::LLVM::ICmpOp>(
             loc, mlir::LLVM::ICmpPredicate::sle, caseArg, selector);
         auto *thisBlock = rewriter.getInsertionBlock();
         auto *newBlock1 = createBlock(rewriter, dest);
         auto *newBlock2 = createBlock(rewriter, dest);
         rewriter.setInsertionPointToEnd(thisBlock);
         rewriter.create<mlir::LLVM::CondBrOp>(loc, cmp, newBlock1, newBlock2);
         rewriter.setInsertionPointToEnd(newBlock1);
-        mlir::Value caseArg0 = *(cmpOps->begin() + 1);
+        mlir::Value caseArg0 = *(cmpOps.value().begin() + 1);
         auto cmp0 = rewriter.create<mlir::LLVM::ICmpOp>(
             loc, mlir::LLVM::ICmpPredicate::sle, selector, caseArg0);
         genCondBrOp(loc, cmp0, dest, destOps, rewriter, newBlock2);
         rewriter.setInsertionPointToEnd(newBlock2);
         continue;
       }
       assert(attr.isa<mlir::UnitAttr>());
       assert((t + 1 == conds) && "unit must be last");