This is an archive of the discontinued LLVM Phabricator instance.

[DWARFYAML] Pull out common helper functions for rnglist and loclist tables. NFC.
ClosedPublic

Authored by Higuoxing on Jul 22 2020, 9:13 PM.

Details

Summary

This patch helps pull out some common helper functions for range list
and location list tables. NFC.

Diff Detail

Event Timeline

Higuoxing created this revision.Jul 22 2020, 9:13 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 22 2020, 9:13 PM
grimar added inline comments.Jul 23 2020, 1:55 AM
llvm/lib/ObjectYAML/DWARFEmitter.cpp
454

nit: you do not need curly braces around return createStringError....

481

It looks like you could pass const DWARFYAML::RnglistEntry &Entry directly to checkListEntryOperands

i.e.:

static Error checkListEntryOperands(const DWARFYAML::RnglistEntry &Entry, uint64_t ExpectedOpsNum) {
  StringRef EncodingName = dwarf::RangeListEncodingString(Entry.Operator);
...
}

and then avoid having the CheckOperands helper? (inline checkListEntryOperands)

Higuoxing marked an inline comment as done.Jul 23 2020, 2:00 AM
Higuoxing added inline comments.
llvm/lib/ObjectYAML/DWARFEmitter.cpp
481

Yeah, but I want to use checkListEntryOperands() to check operands for both the range list table and the location list table. The .debug_loclists section is implemented in D84234

Higuoxing marked an inline comment as done.Jul 23 2020, 2:04 AM
Higuoxing added inline comments.
llvm/lib/ObjectYAML/DWARFEmitter.cpp
481

My idea is having

StringRef EncodingName = dwarf::RangeListEncodingString(Entry.Operator);
auto CheckOperands = [&](uint64_t ExpectedOperands) -> Error {
  return checkListEntryOperands(EncodingName, Entry.Values, ExpectedOperands);
};

for the range list table and

StringRef EncodingName = dwarf::LocListEncodingString(Entry.Operator);
auto CheckOperands = [&](uint64_t ExpectedOperands) -> Error {
  return checkListEntryOperands(EncodingName, Entry.Values, ExpectedOperands);
};

for the location list table.

jhenderson accepted this revision.Jul 23 2020, 2:06 AM

LGTM, personally, barring @grimar's first nit.

This revision is now accepted and ready to land.Jul 23 2020, 2:06 AM
grimar added inline comments.Jul 23 2020, 2:08 AM
llvm/lib/ObjectYAML/DWARFEmitter.cpp
481

Ah, OK.

Higuoxing updated this revision to Diff 280120.Jul 23 2020, 7:56 AM
Higuoxing marked 2 inline comments as done.

Thanks for reviewing!

llvm/lib/ObjectYAML/DWARFEmitter.cpp
454

Oh, thanks for pointing this out!

This revision was automatically updated to reflect the committed changes.