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

Code before:

// Forward declare indexed types, for integration with ValueStore.
class Decl;

}  // namespace clang

namespace Carbon::SemIR {

// A Clang declaration mapped to a Carbon instruction.
//
// Note that Clang's AST uses address-identity for nodes, which means the
// pointer is the canonical way to represent a specific AST node and is expected
// to be sufficient for comparison, hashing, etc.
struct ClangDecl : public Printable<ClangDecl> {
  auto Print(llvm::raw_ostream& out) const -> void;

  // Equality comparison uses the address-identity property of the Clang AST and
  // just compares the `decl` pointers. We canonicalize on these to avoid ever
  // having different instructions for the same declaration.
  auto operator==(const ClangDecl& rhs) const -> bool {
    return decl == rhs.decl;
  }
  // Support direct comparison with the Clang AST node pointer.
  auto operator==(const clang::Decl* rhs_decl) const -> bool {
    return decl == rhs_decl;
  }

  // Hashing for ClangDecl. See common/hashing.h.
  friend auto CarbonHashValue(const ClangDecl& value, uint64_t seed)
      -> HashCode {
    return HashValue(value.decl, seed);
  }

  // The Clang declaration pointing to the Clang AST.
  // TODO: Ensure we can easily serialize/deserialize this. Consider
  // `clang::LazyDeclPtr`.
  clang::Decl* decl = nullptr;

  // The instruction the Clang declaration is mapped to.
  //
  // This is stored along side the `decl` pointer to avoid having to lookup both
  // the pointer and the instruction ID in two separate areas of storage.
  InstId inst_id;
};

// The ID of a `ClangDecl`.
//
// These IDs are importantly distinct from the `inst_id` associated with each
// declaration. These form a dense range of IDs that is used to reference the
// AST node pointers without storing those pointers directly into SemIR and
// needing space to hold a full pointer. We can't avoid having these IDs without

Review comment (refactor):
IIUC the concern raised by @geoffromer correctly, we should make `ClangDecl` default comparison include all the fields. I think we should be able to customize the specific store to compare only the `decl` field for the storage needs instead of changing `ClangDecl` itself.
https://discord.com/channels/655572317891461132/768530752592805919/1384999468293947537
I think that if we can provide the information of not just what is the key of the storage but also how to extract the key from the value.
This is what I've tried to do in a clunky way in #5725, but I guess I missed changing the API appropriately.

Code after:

// Forward declare indexed types, for integration with ValueStore.
class Decl;

}  // namespace clang

namespace Carbon::SemIR {

// A Clang declaration mapped to a Carbon instruction.
//
// Note that Clang's AST uses address-identity for nodes, which means the
// pointer is the canonical way to represent a specific AST node and is expected
// to be sufficient for comparison, hashing, etc.
//
// This type is specifically designed for use in a `CanonicalValueStore` and
// provide a single canonical access from SemIR to each `clang::Decl*` used.
// This also ensures that a given `clang::Decl*` is associated with exactly one
// instruction, and the `inst_id` here provides access to that instruction from
// either the `ClangDeclId` or the `clang::Decl*`.
struct ClangDecl : public Printable<ClangDecl> {
  auto Print(llvm::raw_ostream& out) const -> void;

  // Equality comparison uses the address-identity property of the Clang AST and
  // just compares the `decl` pointers. The `inst_id` is always the same due to
  // the canonicalization.
  auto operator==(const ClangDecl& rhs) const -> bool {
    return decl == rhs.decl;
  }
  // Support direct comparison with the Clang AST node pointer.
  auto operator==(const clang::Decl* rhs_decl) const -> bool {
    return decl == rhs_decl;
  }

  // Hashing for ClangDecl. See common/hashing.h.
  friend auto CarbonHashValue(const ClangDecl& value, uint64_t seed)
      -> HashCode {
    return HashValue(value.decl, seed);
  }

  // The Clang declaration pointing to the Clang AST.
  // TODO: Ensure we can easily serialize/deserialize this. Consider
  // `clang::LazyDeclPtr`.
  clang::Decl* decl = nullptr;

  // The instruction the Clang declaration is mapped to.
  //
  // This is stored along side the `decl` pointer to avoid having to lookup both
  // the pointer and the instruction ID in two separate areas of storage.
  InstId inst_id;
};