Prechádzať zdrojové kódy

Change `CopyOnWriteBlock::file_` from reference to pointer (#5230)

Per [the style
guide](https://github.com/carbon-language/carbon-lang/blob/trunk/docs/project/cpp_style_guide.md#syntax-and-formatting):
* If it is captured and must outlive the call expression itself, use a
pointer and document that it must not be null (unless it is also
optional).
* When storing an object's address as a non-owned member, prefer storing
a pointer.
Boaz Brickner 1 rok pred
rodič
commit
6e2dbb5b61

+ 6 - 6
toolchain/check/convert.cpp

@@ -385,10 +385,10 @@ static auto ConvertTupleToTuple(Context& context, SemIR::TupleType src_type,
   // TODO: Annotate diagnostics coming from here with the element index.
   // TODO: Annotate diagnostics coming from here with the element index.
   auto new_block =
   auto new_block =
       literal_elems_id.has_value()
       literal_elems_id.has_value()
-          ? SemIR::CopyOnWriteInstBlock(sem_ir, literal_elems_id)
+          ? SemIR::CopyOnWriteInstBlock(&sem_ir, literal_elems_id)
           : SemIR::CopyOnWriteInstBlock(
           : SemIR::CopyOnWriteInstBlock(
-                sem_ir, SemIR::CopyOnWriteInstBlock::UninitializedBlock{
-                            src_elem_types.size()});
+                &sem_ir, SemIR::CopyOnWriteInstBlock::UninitializedBlock{
+                             src_elem_types.size()});
   for (auto [i, src_type_id, dest_type_id] :
   for (auto [i, src_type_id, dest_type_id] :
        llvm::enumerate(src_elem_types, dest_elem_types)) {
        llvm::enumerate(src_elem_types, dest_elem_types)) {
     // TODO: This call recurses back into conversion. Switch to an iterative
     // TODO: This call recurses back into conversion. Switch to an iterative
@@ -496,10 +496,10 @@ static auto ConvertStructToStructOrClass(Context& context,
   // TODO: Annotate diagnostics coming from here with the element index.
   // TODO: Annotate diagnostics coming from here with the element index.
   auto new_block =
   auto new_block =
       literal_elems_id.has_value() && !dest_has_vptr
       literal_elems_id.has_value() && !dest_has_vptr
-          ? SemIR::CopyOnWriteInstBlock(sem_ir, literal_elems_id)
+          ? SemIR::CopyOnWriteInstBlock(&sem_ir, literal_elems_id)
           : SemIR::CopyOnWriteInstBlock(
           : SemIR::CopyOnWriteInstBlock(
-                sem_ir, SemIR::CopyOnWriteInstBlock::UninitializedBlock{
-                            dest_elem_fields.size()});
+                &sem_ir, SemIR::CopyOnWriteInstBlock::UninitializedBlock{
+                             dest_elem_fields.size()});
   for (auto [i, dest_field] : llvm::enumerate(dest_elem_fields)) {
   for (auto [i, dest_field] : llvm::enumerate(dest_elem_fields)) {
     if (dest_field.name_id == SemIR::NameId::Vptr) {
     if (dest_field.name_id == SemIR::NameId::Vptr) {
       if constexpr (!ToClass) {
       if constexpr (!ToClass) {

+ 3 - 3
toolchain/check/subst.cpp

@@ -155,7 +155,7 @@ static auto PopOperand(Context& context, Worklist& worklist,
                        SemIR::Inst::ArgAndKind arg) -> int32_t {
                        SemIR::Inst::ArgAndKind arg) -> int32_t {
   auto pop_block_id = [&](SemIR::InstBlockId old_inst_block_id) {
   auto pop_block_id = [&](SemIR::InstBlockId old_inst_block_id) {
     auto size = context.inst_blocks().Get(old_inst_block_id).size();
     auto size = context.inst_blocks().Get(old_inst_block_id).size();
-    SemIR::CopyOnWriteInstBlock new_inst_block(context.sem_ir(),
+    SemIR::CopyOnWriteInstBlock new_inst_block(&context.sem_ir(),
                                                old_inst_block_id);
                                                old_inst_block_id);
     for (auto i : llvm::reverse(llvm::seq(size))) {
     for (auto i : llvm::reverse(llvm::seq(size))) {
       new_inst_block.Set(i, worklist.Pop());
       new_inst_block.Set(i, worklist.Pop());
@@ -196,7 +196,7 @@ static auto PopOperand(Context& context, Worklist& worklist,
     }
     }
     case CARBON_KIND(SemIR::StructTypeFieldsId old_fields_id): {
     case CARBON_KIND(SemIR::StructTypeFieldsId old_fields_id): {
       auto old_fields = context.struct_type_fields().Get(old_fields_id);
       auto old_fields = context.struct_type_fields().Get(old_fields_id);
-      SemIR::CopyOnWriteStructTypeFieldsBlock new_fields(context.sem_ir(),
+      SemIR::CopyOnWriteStructTypeFieldsBlock new_fields(&context.sem_ir(),
                                                          old_fields_id);
                                                          old_fields_id);
       for (auto i : llvm::reverse(llvm::seq(old_fields.size()))) {
       for (auto i : llvm::reverse(llvm::seq(old_fields.size()))) {
         new_fields.Set(i, {.name_id = old_fields[i].name_id,
         new_fields.Set(i, {.name_id = old_fields[i].name_id,
@@ -207,7 +207,7 @@ static auto PopOperand(Context& context, Worklist& worklist,
     }
     }
     case CARBON_KIND(SemIR::TypeBlockId old_type_block_id): {
     case CARBON_KIND(SemIR::TypeBlockId old_type_block_id): {
       auto size = context.type_blocks().Get(old_type_block_id).size();
       auto size = context.type_blocks().Get(old_type_block_id).size();
-      SemIR::CopyOnWriteTypeBlock new_type_block(context.sem_ir(),
+      SemIR::CopyOnWriteTypeBlock new_type_block(&context.sem_ir(),
                                                  old_type_block_id);
                                                  old_type_block_id);
       for (auto i : llvm::reverse(llvm::seq(size))) {
       for (auto i : llvm::reverse(llvm::seq(size))) {
         new_type_block.Set(
         new_type_block.Set(

+ 10 - 10
toolchain/sem_ir/copy_on_write_block.h

@@ -27,16 +27,16 @@ class CopyOnWriteBlock {
   };
   };
 
 
   // Constructs the block. `source_id` is used as the initial value of the
   // Constructs the block. `source_id` is used as the initial value of the
-  // block.
-  explicit CopyOnWriteBlock(SemIR::File& file, BlockIdType source_id)
+  // block. `file` must not be null.
+  explicit CopyOnWriteBlock(SemIR::File* file, BlockIdType source_id)
       : file_(file), source_id_(source_id) {}
       : file_(file), source_id_(source_id) {}
 
 
   // Constructs the block, treating the original block as an uninitialized block
   // Constructs the block, treating the original block as an uninitialized block
-  // with `size` elements.
-  explicit CopyOnWriteBlock(SemIR::File& file, UninitializedBlock uninit)
+  // with `size` elements. `file` must not be null.
+  explicit CopyOnWriteBlock(SemIR::File* file, UninitializedBlock uninit)
       : file_(file),
       : file_(file),
         source_id_(BlockIdType::None),
         source_id_(BlockIdType::None),
-        id_((file_.*ValueStore)().AddUninitialized(uninit.size)) {}
+        id_((file_->*ValueStore)().AddUninitialized(uninit.size)) {}
 
 
   // Gets a block ID containing the resulting elements. Note that further
   // Gets a block ID containing the resulting elements. Note that further
   // modifications may or may not allocate a new ID, so this should only be
   // modifications may or may not allocate a new ID, so this should only be
@@ -46,23 +46,23 @@ class CopyOnWriteBlock {
   // Gets a canonical block ID containing the resulting elements. This assumes
   // Gets a canonical block ID containing the resulting elements. This assumes
   // the original block ID, if specified, was also canonical.
   // the original block ID, if specified, was also canonical.
   auto GetCanonical() const -> BlockIdType {
   auto GetCanonical() const -> BlockIdType {
-    return id_ == source_id_ ? id_ : (file_.*ValueStore)().MakeCanonical(id_);
+    return id_ == source_id_ ? id_ : (file_->*ValueStore)().MakeCanonical(id_);
   }
   }
 
 
   // Sets the element at index `i` within the block. Lazily allocates a new
   // Sets the element at index `i` within the block. Lazily allocates a new
   // block when the value changes for the first time.
   // block when the value changes for the first time.
   auto Set(int i, typename BlockIdType::ElementType value) -> void {
   auto Set(int i, typename BlockIdType::ElementType value) -> void {
-    if (source_id_.has_value() && (file_.*ValueStore)().Get(id_)[i] == value) {
+    if (source_id_.has_value() && (file_->*ValueStore)().Get(id_)[i] == value) {
       return;
       return;
     }
     }
     if (id_ == source_id_) {
     if (id_ == source_id_) {
-      id_ = (file_.*ValueStore)().Add((file_.*ValueStore)().Get(source_id_));
+      id_ = (file_->*ValueStore)().Add((file_->*ValueStore)().Get(source_id_));
     }
     }
-    (file_.*ValueStore)().GetMutable(id_)[i] = value;
+    (file_->*ValueStore)().GetMutable(id_)[i] = value;
   }
   }
 
 
  private:
  private:
-  SemIR::File& file_;
+  SemIR::File* file_;
   BlockIdType source_id_;
   BlockIdType source_id_;
   BlockIdType id_ = source_id_;
   BlockIdType id_ = source_id_;
 };
 };