Repository: carbon-language/carbon-lang
File: toolchain/sem_ir/inst.h (C/C++)

Code before:
    }
    return {std::nullopt, InstId::None};
  }

  // Returns a resolved LocId, which will point to a parse node, an import, or
  // be None.
  //
  // Unresolved LocIds can be backed by an InstId which may or may not have a
  // value after being resolved, so this operation needs to be done before using
  // most operations on LocId.
  auto GetCanonicalLocId(LocId loc_id) const -> LocId {
    while (loc_id.kind() == LocId::Kind::InstId) {
      loc_id = GetNonCanonicalLocId(loc_id.inst_id());
    }
    return loc_id;
  }

  // Gets the resolved LocId for an instruction. InstId can directly construct
  // an unresolved LocId. This skips that step when a resolved LocId is needed.
  auto GetCanonicalLocId(InstId inst_id) const -> LocId {
    return GetCanonicalLocId(GetNonCanonicalLocId(inst_id));
  }

  // Works like `GetCanonicalLocId`, but called as part of desugaring. Marks the
  // returned location as desugared.
  auto GetDesugaredLocId(LocId loc_id) const -> LocId {
    return GetCanonicalLocId(loc_id).AsDesugared();
  }
  auto GetDesugaredLocId(InstId inst_id) const -> LocId {
    return GetCanonicalLocId(inst_id).AsDesugared();
  }

  // Returns the instruction that this instruction was imported from, or
  // ImportIRInstId::None if this instruction was not generated by importing
  // another instruction.
  auto GetImportSource(InstId inst_id) const -> ImportIRInstId {
    auto loc_id = GetNonCanonicalLocId(inst_id);
    return loc_id.kind() == LocId::Kind::ImportIRInstId
               ? loc_id.import_ir_inst_id()
               : ImportIRInstId::None;
  }

  // Overwrites a given instruction with a new value.
  auto Set(InstId inst_id, Inst inst) -> void { values_.Get(inst_id) = inst; }

  // Overwrites a given instruction's location with a new value.
  auto SetLocId(InstId inst_id, LocId loc_id) -> void {
    loc_ids_[inst_id.index] = loc_id;
  }

  // Overwrites a given instruction and location ID with a new value.

Review comment (suggestion):
I'm finding the naming here a bit confusing. I think the issue is that I think of desugaring as a process of going from a wrapped / sugared thing and removing the wrapping to produce the underlying desugared thing, but what we're actually doing *to the location* here is more like the opposite of that: we're starting from a location in the source, and effectively *wrapping* it in something that says it's the location of the desugared code rather than the original location.

I wonder if there's some small change we can make here so that this reads as "get the location for the desugaring of the instruction at this location" rather than "get the desugared version of this location". Maybe something like:
```suggestion
  // Returns a virtual location to use for the desugaring of the code at the
  // specified location.
  auto GetLocForDesugaringOf(LocId loc_id) const -> LocId {
```

Code after:
    }
    return {std::nullopt, InstId::None};
  }

  // Returns a resolved LocId, which will point to a parse node, an import, or
  // be None.
  //
  // Unresolved LocIds can be backed by an InstId which may or may not have a
  // value after being resolved, so this operation needs to be done before using
  // most operations on LocId.
  auto GetCanonicalLocId(LocId loc_id) const -> LocId {
    while (loc_id.kind() == LocId::Kind::InstId) {
      loc_id = GetNonCanonicalLocId(loc_id.inst_id());
    }
    return loc_id;
  }

  // Gets the resolved LocId for an instruction. InstId can directly construct
  // an unresolved LocId. This skips that step when a resolved LocId is needed.
  auto GetCanonicalLocId(InstId inst_id) const -> LocId {
    return GetCanonicalLocId(GetNonCanonicalLocId(inst_id));
  }

  // Returns a virtual location to use for the desugaring of the code at the
  // specified location.
  auto GetLocIdForDesugaring(LocId loc_id) const -> LocId {
    return GetCanonicalLocId(loc_id).AsDesugared();
  }
  auto GetLocIdForDesugaring(InstId inst_id) const -> LocId {
    return GetCanonicalLocId(inst_id).AsDesugared();
  }

  // Returns the instruction that this instruction was imported from, or
  // ImportIRInstId::None if this instruction was not generated by importing
  // another instruction.
  auto GetImportSource(InstId inst_id) const -> ImportIRInstId {
    auto loc_id = GetNonCanonicalLocId(inst_id);
    return loc_id.kind() == LocId::Kind::ImportIRInstId
               ? loc_id.import_ir_inst_id()
               : ImportIRInstId::None;
  }

  // Overwrites a given instruction with a new value.
  auto Set(InstId inst_id, Inst inst) -> void { values_.Get(inst_id) = inst; }

  // Overwrites a given instruction's location with a new value.
  auto SetLocId(InstId inst_id, LocId loc_id) -> void {
    loc_ids_[inst_id.index] = loc_id;
  }

  // Overwrites a given instruction and location ID with a new value.