diff --git a/flang/documentation/C++17.md b/flang/documentation/C++17.md index cedf6923cc75..7c26c9e29fba 100644 --- a/flang/documentation/C++17.md +++ b/flang/documentation/C++17.md @@ -124,6 +124,8 @@ with lists of types. This simple little type is used wherever a value might or might not be present. +It is especially useful for function results and +rvalue reference arguments. It corresponds directly to the optional elements in the productions of the Fortran grammar. It is also used as a wrapper around a parse tree node type to define the diff --git a/flang/documentation/C++style.md b/flang/documentation/C++style.md index 58bfd1f1cd3e..b6304efb3181 100644 --- a/flang/documentation/C++style.md +++ b/flang/documentation/C++style.md @@ -147,6 +147,13 @@ We have an `ENUM_CLASS` macro that helps capture the names of constants. explicitly, it should contains either a `default:;` at its end or a `default:` label that obviously crashes; we have a `CRASH_NO_CASE` macro for such situations. +1. When using `std::optional` values, avoid unprotected access to their content. +This is usually by means of `x.has_value()` guarding execution of `*x`. +This is implicit when they are function results assigned to local variables +in `if`/`while` predicates. +When no presence test is obviously protecting a `*x` reference to the +contents, and it is assumed that the contents are present, validate that +assumption by using `x.value()` instead. #### Classes 1. Define POD structures with `struct`. 1. Don't use `this->` in (non-static) member functions, unless forced to diff --git a/flang/lib/evaluate/intrinsics.cc b/flang/lib/evaluate/intrinsics.cc index 4e689ed13aca..751329e483ea 100644 --- a/flang/lib/evaluate/intrinsics.cc +++ b/flang/lib/evaluate/intrinsics.cc @@ -908,7 +908,7 @@ std::optional IntrinsicInterface::Match( if (sameArg == nullptr) { sameArg = arg; } - argOk = *type == sameArg->GetType(); + argOk = type.value() == sameArg->GetType(); break; case KindCode::effectiveKind: common::die("INTERNAL: KindCode::effectiveKind appears on argument '%s' " @@ -1017,7 +1017,7 @@ std::optional IntrinsicInterface::Match( if (call.isSubroutineCall) { return std::nullopt; } - resultType = DynamicType{*result.categorySet.LeastElement(), 0}; + resultType = DynamicType{result.categorySet.LeastElement().value(), 0}; switch (result.kindCode) { case KindCode::defaultIntegerKind: CHECK(result.categorySet == IntType); @@ -1124,11 +1124,7 @@ std::optional IntrinsicInterface::Match( break; case Rank::shaped: CHECK(shapeArg != nullptr); - { - std::optional shapeLen{shapeArg->VectorSize()}; - CHECK(shapeLen.has_value()); - resultRank = *shapeLen; - } + resultRank = shapeArg->VectorSize().value(); break; case Rank::elementalOrBOZ: case Rank::shape: diff --git a/flang/lib/evaluate/real.h b/flang/lib/evaluate/real.h index 3d9b7cd0b0bf..82c26078e30f 100644 --- a/flang/lib/evaluate/real.h +++ b/flang/lib/evaluate/real.h @@ -51,13 +51,6 @@ public: static constexpr int maxExponent{(1 << exponentBits) - 1}; static constexpr int exponentBias{maxExponent / 2}; - // Decimal precision of a binary floating-point representation is - // actually the same as the base-5 precision, as factors of two - // can be accommodated by the binary exponent. - // log(2)/log(5) = 0.430+ in any base. - // Calculate "precision*0.43" with integer arithmetic so as to be constexpr. - static constexpr int decimalDigits{(precision * 43) / 100}; - template friend class Real; constexpr Real() {} // +0.0 diff --git a/flang/lib/evaluate/tools.h b/flang/lib/evaluate/tools.h index 94798e9117a7..68cbb7af90a2 100644 --- a/flang/lib/evaluate/tools.h +++ b/flang/lib/evaluate/tools.h @@ -295,8 +295,9 @@ template struct ConvertToKindHelper { template Expr> ConvertToKind(int kind, VALUE &&x) { - return *common::SearchDynamicTypes( - ConvertToKindHelper{kind, std::move(x)}); + return common::SearchDynamicTypes( + ConvertToKindHelper{kind, std::move(x)}) + .value(); } // Given a type category CAT, SameKindExprs is a variant that diff --git a/flang/lib/parser/basic-parsers.h b/flang/lib/parser/basic-parsers.h index c4891fc6636b..6a1167eceb8f 100644 --- a/flang/lib/parser/basic-parsers.h +++ b/flang/lib/parser/basic-parsers.h @@ -488,7 +488,7 @@ public: resultType result; result.emplace_back(std::move(*first)); if (state.GetLocation() > start) { - result.splice(result.end(), *many(parser_).Parse(state)); + result.splice(result.end(), many(parser_).Parse(state).value()); } return {std::move(result)}; } @@ -581,8 +581,7 @@ public: constexpr DefaultedParser(const PA &p) : parser_{p} {} std::optional Parse(ParseState &state) const { std::optional> ax{maybe(parser_).Parse(state)}; - CHECK(ax.has_value()); // maybe() always succeeds - if (ax.value().has_value()) { + if (ax.value().has_value()) { // maybe() always succeeds return std::move(*ax); } return {resultType{}}; diff --git a/flang/lib/parser/characters.h b/flang/lib/parser/characters.h index 362ccc43e9d6..a54df93113f2 100644 --- a/flang/lib/parser/characters.h +++ b/flang/lib/parser/characters.h @@ -148,7 +148,7 @@ void EmitQuotedChar(char32_t ch, const NORMAL &emit, const INSERTED &insert, emit('\\'); } else if (ch < ' ' || (ch >= 0x80 && ch <= 0xff)) { insert('\\'); - if (std::optional escape{BackslashEscapeChar(ch)}) { + if (std::optional escape{BackslashEscapeChar(ch)}) { emit(*escape); } else { // octal escape sequence diff --git a/flang/lib/parser/parse-tree-visitor.h b/flang/lib/parser/parse-tree-visitor.h index 664fbd92db58..8f759759203d 100644 --- a/flang/lib/parser/parse-tree-visitor.h +++ b/flang/lib/parser/parse-tree-visitor.h @@ -66,12 +66,12 @@ template void Walk(format::IntrinsicTypeDataEditDesc &, M &); // Traversal of needed STL template classes (optional, list, tuple, variant) template void Walk(const std::optional &x, V &visitor) { - if (x) { + if (x.has_value()) { Walk(*x, visitor); } } template void Walk(std::optional &x, M &mutator) { - if (x) { + if (x.has_value()) { Walk(*x, mutator); } } diff --git a/flang/lib/parser/token-parsers.h b/flang/lib/parser/token-parsers.h index c7affcabb37b..c51287aefa5c 100644 --- a/flang/lib/parser/token-parsers.h +++ b/flang/lib/parser/token-parsers.h @@ -430,9 +430,8 @@ constexpr struct SkipDigitString { static std::optional SignedInteger( const std::optional &x, Location at, bool negate, ParseState &state) { - std::optional result; if (!x.has_value()) { - return result; + return std::nullopt; } std::uint64_t limit{std::numeric_limits::max()}; if (negate) { @@ -442,8 +441,7 @@ static std::optional SignedInteger( state.Say(at, "overflow in signed decimal literal"_err_en_US); } std::int64_t value = *x; - result = negate ? -value : value; - return result; + return std::make_optional(negate ? -value : value); } struct SignedIntLiteralConstantWithoutKind { @@ -580,9 +578,7 @@ struct HollerithLiteral { } else { // Multi-byte character while (bytes-- > 0) { - std::optional byte{nextCh.Parse(state)}; - CHECK(byte.has_value()); - content += **byte; + content += *nextCh.Parse(state).value(); } } } diff --git a/flang/lib/semantics/expression.cc b/flang/lib/semantics/expression.cc index ae12fdfb9f3d..9d068c4bccc2 100644 --- a/flang/lib/semantics/expression.cc +++ b/flang/lib/semantics/expression.cc @@ -818,8 +818,8 @@ MaybeExpr AnalyzeExpr( if (dynamicType->category == TypeCategory::Character) { return WrapperHelper(dynamicType->kind, - Substring{ - std::move(*checked), std::move(first), std::move(last)}); + Substring{std::move(checked.value()), std::move(first), + std::move(last)}); } } context.Say("substring may apply only to CHARACTER"_err_en_US); @@ -848,9 +848,8 @@ MaybeExpr AnalyzeExpr(ExpressionAnalysisContext &context, lower = Expr{1}; } if (!upper.has_value()) { - std::optional size{ToInt64(length)}; - CHECK(size.has_value()); - upper = Expr{static_cast(*size)}; + upper = Expr{ + static_cast(ToInt64(length).value())}; } return std::visit( [&](auto &&ckExpr) -> MaybeExpr { @@ -861,8 +860,8 @@ MaybeExpr AnalyzeExpr(ExpressionAnalysisContext &context, staticData->set_alignment(Result::kind) .set_itemBytes(Result::kind) .Push(cp->value); - Substring substring{ - std::move(staticData), std::move(*lower), std::move(*upper)}; + Substring substring{std::move(staticData), std::move(lower.value()), + std::move(upper.value())}; return AsGenericExpr(Expr{ Expr{Designator{std::move(substring)}}}); },