From 0b5d1101f93ae65264136d09fdfec6b8914bdd8f Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Mon, 7 Oct 2024 10:25:35 -0700 Subject: [PATCH] Remove redundant optional wrapping llvm::function_ref (#4367) llvm::function_ref (like std::unique_ptr, for instance) already has a null/empty state, so use that to avoid confusion/duplication of empty states between optional and the nested function_refs. --- toolchain/check/context.cpp | 15 ++++++--------- toolchain/check/context.h | 13 ++++++------- toolchain/check/member_access.cpp | 9 ++++----- toolchain/check/member_access.h | 4 ++-- toolchain/check/operator.cpp | 14 ++++++-------- toolchain/check/operator.h | 10 ++++------ 6 files changed, 28 insertions(+), 37 deletions(-) diff --git a/toolchain/check/context.cpp b/toolchain/check/context.cpp index a7fa94095905..2174640eb2ac 100644 --- a/toolchain/check/context.cpp +++ b/toolchain/check/context.cpp @@ -760,8 +760,7 @@ namespace { // complete. class TypeCompleter { public: - TypeCompleter(Context& context, - std::optional diagnoser) + TypeCompleter(Context& context, Context::BuildDiagnosticFn diagnoser) : context_(context), diagnoser_(diagnoser) {} // Attempts to complete the given type. Returns true if it is now complete, @@ -870,7 +869,7 @@ class TypeCompleter { auto& class_info = context_.classes().Get(inst.class_id); if (!class_info.is_defined()) { if (diagnoser_) { - auto builder = (*diagnoser_)(); + auto builder = diagnoser_(); context_.NoteIncompleteClass(inst.class_id, builder); builder.Emit(); } @@ -1151,19 +1150,17 @@ class TypeCompleter { Context& context_; llvm::SmallVector work_list_; - std::optional diagnoser_; + Context::BuildDiagnosticFn diagnoser_; }; } // namespace auto Context::TryToCompleteType(SemIR::TypeId type_id, - std::optional diagnoser) - -> bool { + BuildDiagnosticFn diagnoser) -> bool { return TypeCompleter(*this, diagnoser).Complete(type_id); } auto Context::TryToDefineType(SemIR::TypeId type_id, - std::optional diagnoser) - -> bool { + BuildDiagnosticFn diagnoser) -> bool { if (!TryToCompleteType(type_id, diagnoser)) { return false; } @@ -1171,7 +1168,7 @@ auto Context::TryToDefineType(SemIR::TypeId type_id, if (auto interface = types().TryGetAs(type_id)) { auto interface_id = interface->interface_id; if (!interfaces().Get(interface_id).is_defined()) { - auto builder = (*diagnoser)(); + auto builder = diagnoser(); NoteUndefinedInterface(interface_id, builder); builder.Emit(); return false; diff --git a/toolchain/check/context.h b/toolchain/check/context.h index e2a5bf7c4a76..00b7daf8e77d 100644 --- a/toolchain/check/context.h +++ b/toolchain/check/context.h @@ -301,9 +301,8 @@ class Context { // If the type is not complete, `diagnoser` is invoked to diagnose the issue, // if a `diagnoser` is provided. The builder it returns will be annotated to // describe the reason why the type is not complete. - auto TryToCompleteType( - SemIR::TypeId type_id, - std::optional diagnoser = std::nullopt) -> bool; + auto TryToCompleteType(SemIR::TypeId type_id, + BuildDiagnosticFn diagnoser = nullptr) -> bool; // Attempts to complete and define the type `type_id`. Returns `true` if the // type is defined, or `false` if no definition is available. A defined type @@ -311,15 +310,15 @@ class Context { // // This is the same as `TryToCompleteType` except for interfaces, which are // complete before they are fully defined. - auto TryToDefineType( - SemIR::TypeId type_id, - std::optional diagnoser = std::nullopt) -> bool; + auto TryToDefineType(SemIR::TypeId type_id, + BuildDiagnosticFn diagnoser = nullptr) -> bool; // Returns the type `type_id` as a complete type, or produces an incomplete // type error and returns an error type. This is a convenience wrapper around - // TryToCompleteType. + // TryToCompleteType. `diagnoser` must not be null. auto AsCompleteType(SemIR::TypeId type_id, BuildDiagnosticFn diagnoser) -> SemIR::TypeId { + CARBON_CHECK(diagnoser); return TryToCompleteType(type_id, diagnoser) ? type_id : SemIR::TypeId::Error; } diff --git a/toolchain/check/member_access.cpp b/toolchain/check/member_access.cpp index 25772c32a286..b077ff114042 100644 --- a/toolchain/check/member_access.cpp +++ b/toolchain/check/member_access.cpp @@ -224,7 +224,7 @@ static auto LookupInterfaceWitness(Context& context, static auto PerformImplLookup( Context& context, SemIR::LocId loc_id, SemIR::ConstantId type_const_id, SemIR::AssociatedEntityType assoc_type, SemIR::InstId member_id, - std::optional missing_impl_diagnoser) + Context::BuildDiagnosticFn missing_impl_diagnoser = nullptr) -> SemIR::InstId { auto interface_type = context.types().GetAs(assoc_type.interface_type_id); @@ -236,7 +236,7 @@ static auto PerformImplLookup( CARBON_DIAGNOSTIC(MissingImplInMemberAccessNote, Note, "type `{1}` does not implement interface `{0}`", SemIR::NameId, SemIR::TypeId); - (*missing_impl_diagnoser)() + missing_impl_diagnoser() .Note(loc_id, MissingImplInMemberAccessNote, interface.name_id, context.GetTypeIdForTypeConstant(type_const_id)) .Emit(); @@ -336,7 +336,7 @@ static auto LookupMemberNameInScope(Context& context, SemIR::LocId loc_id, context.types().TryGetAs(type_id)) { if (ScopeNeedsImplLookup(context, lookup_scope)) { member_id = PerformImplLookup(context, loc_id, name_scope_const_id, - *assoc_type, member_id, std::nullopt); + *assoc_type, member_id); } } @@ -494,8 +494,7 @@ auto PerformMemberAccess(Context& context, SemIR::LocId loc_id, auto PerformCompoundMemberAccess( Context& context, SemIR::LocId loc_id, SemIR::InstId base_id, SemIR::InstId member_expr_id, - std::optional missing_impl_diagnoser) - -> SemIR::InstId { + Context::BuildDiagnosticFn missing_impl_diagnoser) -> SemIR::InstId { // Materialize a temporary for the base expression if necessary. base_id = ConvertToValueOrRefExpr(context, base_id); auto base_type_id = context.insts().Get(base_id).type_id(); diff --git a/toolchain/check/member_access.h b/toolchain/check/member_access.h index 35e1562361a7..87862e1c48e9 100644 --- a/toolchain/check/member_access.h +++ b/toolchain/check/member_access.h @@ -23,8 +23,8 @@ auto PerformMemberAccess(Context& context, SemIR::LocId loc_id, auto PerformCompoundMemberAccess( Context& context, SemIR::LocId loc_id, SemIR::InstId base_id, SemIR::InstId member_expr_id, - std::optional missing_impl_diagnoser = - std::nullopt) -> SemIR::InstId; + Context::BuildDiagnosticFn missing_impl_diagnoser = nullptr) + -> SemIR::InstId; // Creates SemIR to perform a tuple index with base expression `tuple_inst_id` // and index expression `index_inst_id`. Returns the result of the access. diff --git a/toolchain/check/operator.cpp b/toolchain/check/operator.cpp index 024c6523fb73..16ef74fcd0c6 100644 --- a/toolchain/check/operator.cpp +++ b/toolchain/check/operator.cpp @@ -29,10 +29,9 @@ static auto GetOperatorOpFunction(Context& context, SemIR::LocId loc_id, return PerformMemberAccess(context, loc_id, interface_id, op_name_id); } -auto BuildUnaryOperator( - Context& context, SemIR::LocId loc_id, Operator op, - SemIR::InstId operand_id, - std::optional missing_impl_diagnoser) +auto BuildUnaryOperator(Context& context, SemIR::LocId loc_id, Operator op, + SemIR::InstId operand_id, + Context::BuildDiagnosticFn missing_impl_diagnoser) -> SemIR::InstId { // Look up the operator function. auto op_fn = GetOperatorOpFunction(context, loc_id, op); @@ -48,10 +47,9 @@ auto BuildUnaryOperator( return PerformCall(context, loc_id, bound_op_id, {}); } -auto BuildBinaryOperator( - Context& context, SemIR::LocId loc_id, Operator op, SemIR::InstId lhs_id, - SemIR::InstId rhs_id, - std::optional missing_impl_diagnoser) +auto BuildBinaryOperator(Context& context, SemIR::LocId loc_id, Operator op, + SemIR::InstId lhs_id, SemIR::InstId rhs_id, + Context::BuildDiagnosticFn missing_impl_diagnoser) -> SemIR::InstId { // Look up the operator function. auto op_fn = GetOperatorOpFunction(context, loc_id, op); diff --git a/toolchain/check/operator.h b/toolchain/check/operator.h index a0d7cf37c5b2..a243e3bb14e7 100644 --- a/toolchain/check/operator.h +++ b/toolchain/check/operator.h @@ -23,9 +23,8 @@ struct Operator { // operator fails. auto BuildUnaryOperator(Context& context, SemIR::LocId loc_id, Operator op, SemIR::InstId operand_id, - std::optional - missing_impl_diagnoser = std::nullopt) - -> SemIR::InstId; + Context::BuildDiagnosticFn missing_impl_diagnoser = + nullptr) -> SemIR::InstId; // Checks and builds SemIR for a binary operator expression. For example, // `lhs_id * rhs_id`. If specified, `missing_impl_diagnoser` is used to build a @@ -33,9 +32,8 @@ auto BuildUnaryOperator(Context& context, SemIR::LocId loc_id, Operator op, // fails. auto BuildBinaryOperator(Context& context, SemIR::LocId loc_id, Operator op, SemIR::InstId lhs_id, SemIR::InstId rhs_id, - std::optional - missing_impl_diagnoser = std::nullopt) - -> SemIR::InstId; + Context::BuildDiagnosticFn missing_impl_diagnoser = + nullptr) -> SemIR::InstId; } // namespace Carbon::Check