This is an archive of the discontinued LLVM Phabricator instance.

[llvm-libtool-darwin] Refactor Slice and writeUniversalBinary
ClosedPublic

Authored by sameerarora101 on Jul 27 2020, 7:42 AM.

Details

Summary

Refactoring Slice class and function createUniversalBinary
from llvm-lipo into MachOUniversalWriter. This refactoring
is necessary so as to use the refactored code for creating
universal binaries under llvm-libtool-darwin.

Diff Detail

Event Timeline

sameerarora101 created this revision.Jul 27 2020, 7:42 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 27 2020, 7:42 AM

clang-tidy: LLVM_OBJECT_UNIVERSALBINARYWRITER_H -> LLVM_OBJECT_MACHOUNIVERSALWRITER_H

This comment was removed by sameerarora101.
llvm/include/llvm/Object/MachOUniversalWriter.h
36

explicit

sameerarora101 edited the summary of this revision. (Show Details)Jul 28 2020, 6:59 AM
sameerarora101 marked an inline comment as done.

Adding explicit keyword

alexander-shaposhnikov added inline comments.
llvm/tools/llvm-lipo/llvm-lipo.cpp
46

nit: 46, 47 can be replaced with reportError(Buf);

This revision is now accepted and ready to land.Jul 28 2020, 11:59 AM
smeenai added inline comments.Jul 28 2020, 2:02 PM
llvm/tools/llvm-lipo/llvm-lipo.cpp
543

What's the reason for not doing this sort inside writeUniversalBinary itself?

llvm/tools/llvm-lipo/llvm-lipo.cpp
543

Unless I'm missing something, it seems to be more natural / a better separation of concerns.
Plus now the signature of this function has cleaned up a bit (ArrayRef)

sameerarora101 marked an inline comment as done.Jul 28 2020, 3:32 PM

Replacing error printing with reportError(Buf).

smeenai accepted this revision.Jul 30 2020, 2:24 PM

LGTM

llvm/tools/llvm-lipo/llvm-lipo.cpp
543

Got it. I was wondering because it seems like each call is doing the sorting. The separation of concerns makes sense though, and it's definitely nicer for writeUniversalBinary to not be mutating the Slices array.