This is an archive of the discontinued LLVM Phabricator instance.

Support: Simplify endian stream interface. NFCI.
ClosedPublic

Authored by pcc on May 17 2018, 2:44 PM.

Details

Summary

Provide some free functions to reduce verbosity of endian-writing
a single value, and replace the endianness template parameter with
a field.

Part of PR37466.

Diff Detail

Repository
rLLD LLVM Linker

Event Timeline

pcc created this revision.May 17 2018, 2:44 PM
pcc updated this revision to Diff 147396.May 17 2018, 2:50 PM
  • Remove unused function
ruiu accepted this revision.May 18 2018, 11:15 AM
ruiu added a subscriber: ruiu.

LGTM

If I were to design this from scratch, I'd perhaps define two classes, LEWriter and BEWriter, instead of giving the endianness as an argument. But that's an off-topic.

This revision is now accepted and ready to land.May 18 2018, 11:15 AM
pcc added a comment.May 18 2018, 12:47 PM

LGTM

If I were to design this from scratch, I'd perhaps define two classes, LEWriter and BEWriter, instead of giving the endianness as an argument. But that's an off-topic.

Maybe, but it will need to be an argument in a few other places, such as ELFObjectWriter (see D47040). Though maybe we could somehow template it on ELFT as we do in lld... but that's another separate discussion.

This revision was automatically updated to reflect the committed changes.

Note that the endianness values can be passed in to the llvm functions at
runtime, there are non template versions of the write functions that accept
endianness as arguments (that may be exactly what you used, btw, I didn’t
read the patch in detail).

It’s a shame this object hierarchy was developed separately from the
BinaryStream / StreamReader / StreamWriter interfaces, because it seems
like there’s some overlap and it would serve this use case nicely