[PATCH] D64006: [Support] Support 64-bit offsets in DataExtractor.
Adrian Prantl via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 1 13:42:56 PDT 2019
aprantl added inline comments.
================
Comment at: include/llvm/Support/DataExtractor.h:82
/// NULL will be returned.
+ const char *getCStr(uint64_t *offset_ptr) const;
+
----------------
ikudrin wrote:
> aprantl wrote:
> > Why do we need both variants? Would the 64-bit variant alone be sufficient?
> In theory, yes. But the class is widely used in LLVM, and not only to parse DWARF sections. Trying to switch to `uint64_t` everywhere will result in a huge patch which will be hard to review. Hence, I decided to move gradually.
I think I'd prefer to review one large (but mostly mechanical) patch over having two overloads that could potentially introduce subtle and hard-to-debug-bugs when, e.g., the 32-bit version is accidentally invoked where the 64-bit version was needed.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64006/new/
https://reviews.llvm.org/D64006
More information about the llvm-commits
mailing list