[PATCH] D151989: [AppleAccelTable][NFC] Refactor equal_range code

[Felipe de Azevedo Piovezan via Phabricator via llvm-commits]

fdeazeve created this revision.
fdeazeve added a reviewer: aprantl.
Herald added a subscriber: hiraditya.
Herald added a project: All.
fdeazeve requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

The current implementation of AppleAcceleratorTable::equal_range has a couple of
drawbacks:

1. Assumptions about how the hash table is structured are hard-coded throughout

the code. Unless you are familiar with the data layout of the table, it becomes
tricky to follow the code.

2. We currently load strings from the string table even for hashes that don't

match our current search hash. This is wasteful.

3. There is no error checking in most DataExtractor calls that can fail.

This patch cleans up (1) by making helper methods that hide the details of the
data layout from the algorithms relying on them. This should also help us in
future patches, where we want to expand the interface to allow iterating over
_all_ entries in the table, and potentially clean up the existing Iterator
class.

The changes above also fix (2), as the problem "just vanishes" when you have a
function called "idxOfHashInBucket(SearchHash)".

The changes above also fix (3), as having individual functions allow us to
expose the points in which reading data can fail. This is particularly important
as we would like to share this implementation with LLDB, which requires robust
error handling.

The changes above are also a step towards addressing a comment left by the
original author:

  /// TODO: Generalize the rest of the AppleAcceleratorTable interface and move it
  /// to this class.

I suspect a lot of these helper functions created also apply to DWARF 5's
accelerator table, so they could be moved to the base class.

The changes above also expose a bug in this implementation: the previous
algorithm looks at _one_ string inside the bucket, but never bothers checking
for collisions. When the main search loop is written as it is with this patch,
the problem becomes evident. We do not fix the issue in this patch, as it is
intended to be NFC.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151989

Files:
  llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
  llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D151989.527811.patch
Type: text/x-patch
Size: 8631 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230602/afc9224d/attachment.bin>