[all-commits] [llvm/llvm-project] 0a7980: [AppleAccelTable][NFC] Refactor equal_range code

[Felipe de Azevedo Piovezan via All-commits]

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

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

  Log Message:
  -----------
  [AppleAccelTable][NFC] Refactor equal_range code

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.

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