[all-commits] [llvm/llvm-project] 0af7d9: [AppleAccelTable][NFC] Refactor iterator class

Felipe de Azevedo Piovezan via All-commits all-commits at lists.llvm.org
Fri Jun 9 09:42:18 PDT 2023


  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 0af7d97fb55380c149f7282add452e8eb1a5f6e8
      https://github.com/llvm/llvm-project/commit/0af7d97fb55380c149f7282add452e8eb1a5f6e8
  Author: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
  Date:   2023-06-09 (Fri, 09 Jun 2023)

  Changed paths:
    M llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
    M llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp

  Log Message:
  -----------
  [AppleAccelTable][NFC] Refactor iterator class

The current implementation of the AppleAcceleratorTable::Entry is problematic
for a few reasons:

1. It is heavyweight. Iterators should be cheap, but the current implementation
tracks 3 different integer values, one "Entry" object, and one pointer to the
actual accelerator table. Most of this information is redundant and can be
removed.

2. It performs "memory reads" outside of the dereference operation. This
violates the usual expectations of iterators, whereby we don't access anything
so long as we don't dereference  the iterator.

3. It doesn't commit to tracking _one_ thing only. It tries to track both an
"index" into a list of HashData entries and a pointer in a blob of data. For
this reason, it allows for multiple "invalid" states, keeps redundant
information around and is difficult to understand.

4. It couples the interpretation of the data with the iterator increment. As
such, if the *interpretation* fails, the iterator will keep on producing garbage
values without ever indicating so to consumers.

The problem this iterator is trying to solve is simple: we have a blob of data
containing many "HashData" entries and we want to iterate over them. As such,
this commit makes the iterator only track a pointer over that data, and it
decouples the iterator increments from the interpretation of this blob of data.

We maintain the already existing assumption that failures never happen, but now
make it explicit with an assert.

Depends on D152158

Differential Revision: https://reviews.llvm.org/D152159




More information about the All-commits mailing list