[llvm] [DWARFLinker][DWARFLinkerParallel][NFC] Refactor DWARFLinker&DWARFLinkerParallel to have a common library. (PR #74725)

Felipe de Azevedo Piovezan via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 14 06:20:21 PST 2023


felipepiovezan wrote:

> friendly ping.

I think the challenge here is that this change looks like it's just moving files around, but in reality there are behavior differences mixed in the middle. To give you an example, when I look at the diff there are a bunch of APIs change that are pretty difficult to look at and say "this is trivially the same as before":

For example:

```
void emitDwarfDebugRangeListFragment(const CompileUnit &Unit,
                                        const AddressRanges &LinkedRanges,
                                        PatchLocation Patch,
                                        DebugDieValuePool &AddrPool) override;
```

Became

```
  /// Emit debug ranges(.debug_ranges, .debug_rnglists) fragment.
   void emitDwarfDebugRangeListFragment(const CompileUnit &Unit,
                                        const AddressRanges &LinkedRanges,
                                        PatchLocation Patch,
                                        IndexedValuesMap<uint64_t> &AddrPool);
```

Another example:

```
  std::unique_ptr<DwarfStreamer> TheDwarfEmitter;
```

Became:

```
  DwarfStreamer *OutDwarfStreamer = nullptr;
```

Without having to do a lot of back-and-forth between the 1300 lines added and 1500 lines deleted, I'm not sure how to review this type of change.

My suggestion would be to try a more piece-wise approach, otherwise you will probably get a "LGTM" that in reality is more like "I couldn't really check this but let's merge it".

https://github.com/llvm/llvm-project/pull/74725


More information about the llvm-commits mailing list