diff --git a/toolchain/check/diagnostic_helpers.h b/toolchain/check/diagnostic_helpers.h index 4c4a74ceb395..90b782d4ecdc 100644 --- a/toolchain/check/diagnostic_helpers.h +++ b/toolchain/check/diagnostic_helpers.h @@ -12,12 +12,14 @@ namespace Carbon::Check { -// Diagnostic locations produced by checking may be either a parse node -// directly, or an inst ID which is later translated to a parse node. -struct SemIRLoc { +// Tracks a location for diagnostic use, which is either a parse node or an inst +// ID which can be translated to a parse node. Used when code needs to support +// multiple possible ways of reporting a diagnostic location. +class SemIRLoc { + public: // NOLINTNEXTLINE(google-explicit-constructor) SemIRLoc(SemIR::InstId inst_id) - : inst_id(inst_id), is_inst_id(true), token_only(false) {} + : inst_id_(inst_id), is_inst_id_(true), token_only_(false) {} // NOLINTNEXTLINE(google-explicit-constructor) SemIRLoc(Parse::NodeId node_id) : SemIRLoc(node_id, false) {} @@ -25,16 +27,22 @@ struct SemIRLoc { // NOLINTNEXTLINE(google-explicit-constructor) SemIRLoc(SemIR::LocId loc_id) : SemIRLoc(loc_id, false) {} + // If `token_only` is true, refers to the specific node; otherwise, refers to + // the node and its children. explicit SemIRLoc(SemIR::LocId loc_id, bool token_only) - : loc_id(loc_id), is_inst_id(false), token_only(token_only) {} + : loc_id_(loc_id), is_inst_id_(false), token_only_(token_only) {} + + private: + // Only allow member access for diagnostics. + friend class SemIRDiagnosticConverter; union { - SemIR::InstId inst_id; - SemIR::LocId loc_id; + SemIR::InstId inst_id_; + SemIR::LocId loc_id_; }; - bool is_inst_id; - bool token_only; + bool is_inst_id_; + bool token_only_; }; inline auto TokenOnly(SemIR::LocId loc_id) -> SemIRLoc { diff --git a/toolchain/check/eval.cpp b/toolchain/check/eval.cpp index f660bbd543bb..fbf7e4b98ede 100644 --- a/toolchain/check/eval.cpp +++ b/toolchain/check/eval.cpp @@ -2126,9 +2126,6 @@ auto TryEvalBlockForSpecific(Context& context, SemIRLoc loc, &context.emitter(), [&](auto& builder) { CARBON_DIAGNOSTIC(ResolvingSpecificHere, Note, "in {0} used here", InstIdAsType); - if (loc.is_inst_id && !loc.inst_id.has_value()) { - return; - } builder.Note(loc, ResolvingSpecificHere, GetInstForSpecific(context, specific_id)); }); diff --git a/toolchain/check/sem_ir_diagnostic_converter.cpp b/toolchain/check/sem_ir_diagnostic_converter.cpp index 26f2b2108f02..e2568c57e61b 100644 --- a/toolchain/check/sem_ir_diagnostic_converter.cpp +++ b/toolchain/check/sem_ir_diagnostic_converter.cpp @@ -53,7 +53,7 @@ auto SemIRDiagnosticConverter::ConvertLocImpl(SemIRLoc loc, if (import_loc_id.is_node_id()) { // For imports in the current file, the location is simple. in_import_loc = ConvertLocInFile(cursor_ir, import_loc_id.node_id(), - loc.token_only, context_fn); + loc.token_only_, context_fn); } else if (import_loc_id.is_import_ir_inst_id()) { // For implicit imports, we need to unravel the location a little // further. @@ -67,7 +67,7 @@ auto SemIRDiagnosticConverter::ConvertLocImpl(SemIRLoc loc, "Should only be one layer of implicit imports"); in_import_loc = ConvertLocInFile(implicit_ir.sem_ir, implicit_loc_id.node_id(), - loc.token_only, context_fn); + loc.token_only_, context_fn); } // TODO: Add an "In implicit import of prelude." note for the case where we @@ -91,16 +91,16 @@ auto SemIRDiagnosticConverter::ConvertLocImpl(SemIRLoc loc, return std::nullopt; } else { // Parse nodes always refer to the current IR. - return ConvertLocInFile(cursor_ir, loc_id.node_id(), loc.token_only, + return ConvertLocInFile(cursor_ir, loc_id.node_id(), loc.token_only_, context_fn); } }; // Handle the base location. - if (loc.is_inst_id) { - cursor_inst_id = loc.inst_id; + if (loc.is_inst_id_) { + cursor_inst_id = loc.inst_id_; } else { - if (auto diag_loc = handle_loc(loc.loc_id)) { + if (auto diag_loc = handle_loc(loc.loc_id_)) { return *diag_loc; } CARBON_CHECK(cursor_inst_id.has_value(), "Should have been set"); @@ -135,7 +135,7 @@ auto SemIRDiagnosticConverter::ConvertLocImpl(SemIRLoc loc, } // `None` parse node but not an import; just nothing to point at. - return ConvertLocInFile(cursor_ir, Parse::NodeId::None, loc.token_only, + return ConvertLocInFile(cursor_ir, Parse::NodeId::None, loc.token_only_, context_fn); } }