[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