This is an archive of the discontinued LLVM Phabricator instance.

[FaultsMaps][llvm-objdump] Move FaultMapParser to Object/. Remove CodeGen dependency from llvm-objdump
ClosedPublic

Authored by craig.topper on Jan 25 2021, 7:00 PM.

Details

Summary

FaultsMapParser lived in CodeGen and was forcing llvm-objdump to
link CodeGen and everything CodeGen depends on.

This was previously attempted in r240364 to fix a link failure.
The CodeGen dependency was independently added to fix the same
link failure, and that ended up being kept.

Removing the dependency seems like the correct layering for
llvm-objdump.

Diff Detail

Event Timeline

craig.topper created this revision.Jan 25 2021, 7:00 PM
craig.topper requested review of this revision.Jan 25 2021, 7:00 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 25 2021, 7:00 PM

I agree that lib/Object is a better place.

llvm/include/llvm/Object/FaultMapParser.h
5

The license header should be Apache-2.0 WITH LLVM-exception

llvm/lib/Object/FaultMapParser.cpp
5

Apache-2.0 WITH LLVM-exception

craig.topper added inline comments.Jan 25 2021, 7:34 PM
llvm/include/llvm/Object/FaultMapParser.h
5

Doh! I started from reverting the original revert and forgot that the headers would be old. Thanks.

MaskRay added inline comments.Jan 25 2021, 7:43 PM
llvm/include/llvm/Object/FaultMapParser.h
13

clang-tidy: warning: code/includes outside of area guarded by header guard; consider moving it [llvm-header-guard]

Move the guard before other includes.

156

Consider reformat it while you are moving code.

Address review comments

MaskRay accepted this revision.Jan 25 2021, 9:32 PM

Thanks! Please wait a bit for other reviewers.

This revision is now accepted and ready to land.Jan 25 2021, 9:32 PM
jhenderson accepted this revision.Jan 26 2021, 1:14 AM

Have you made sure the includes in both the files being moved from and being moved to are minimal? Seems like an easy thing to forget to do, but I have no specific example of any that are incorrect (I haven't gone digging). LGTM, aside from that and the inline comments.

llvm/lib/Object/FaultMapParser.cpp
46

Consider fixing these clang-tidy warnings whilst you're moving the code.

60

Ditto.

This revision was landed with ongoing or failed builds.Jan 27 2021, 10:41 AM
This revision was automatically updated to reflect the committed changes.