Page MenuHomePhabricator

[yaml2obj] - Change NameIndex to StName for Symbol.
ClosedPublic

Authored by grimar on Wed, Feb 5, 2:56 AM.

Details

Summary

It is consistent with the approach we use for Section struct.

Diff Detail

Event Timeline

grimar created this revision.Wed, Feb 5, 2:56 AM
Herald added a project: Restricted Project. · View Herald Transcript
jhenderson added inline comments.Wed, Feb 5, 6:08 AM
llvm/include/llvm/ObjectYAML/ELFYAML.h
112

Any particular reason for this blank line?

llvm/test/tools/yaml2obj/ELF/symbol-name.yaml
26–27

It might be interesting to have something that references the symbol by name too in this case (e.g. a relocation or group section).

grimar marked an inline comment as done.Wed, Feb 5, 6:10 AM
grimar added inline comments.
llvm/include/llvm/ObjectYAML/ELFYAML.h
112

Yes, it is intentional. I'd like to separate 'St*' things. They have a different nature.

grimar updated this revision to Diff 242606.Wed, Feb 5, 7:04 AM
grimar marked 2 inline comments as done.
  • Addressed review comments.
llvm/test/tools/yaml2obj/ELF/symbol-name.yaml
26–27

Done.

LGTM, with the comment changes, but you might want to wait to give others a chance to comment before committing.

llvm/test/tools/yaml2obj/ELF/symbol-name.yaml
26

has a priority -> has priority
but symbol name -> but the symbol name

27

string symbol -> symbol string

grimar updated this revision to Diff 242610.Wed, Feb 5, 7:29 AM
grimar marked 2 inline comments as done.
  • Addressed comments.
jhenderson accepted this revision.Wed, Feb 5, 7:33 AM
This revision is now accepted and ready to land.Wed, Feb 5, 7:33 AM
MaskRay accepted this revision.Wed, Feb 5, 10:31 AM
This revision was automatically updated to reflect the committed changes.