Page MenuHomePhabricator

builtins: avoid duplicating unwind declarations
ClosedPublic

Authored by compnerd on Dec 31 2014, 7:22 PM.

Details

Reviewers
rengolin
joerg
Summary

Use unwind.h to get the declarations for unwinding interfaces. This header is
already provided by clang and gcc, so this adds no additional dependencies for
building the builtins library. It avoids the duplication which may drift over
time though.

Diff Detail

Event Timeline

compnerd updated this revision to Diff 17753.Dec 31 2014, 7:22 PM
compnerd retitled this revision from to builtins: avoid duplicating unwind declarations.
compnerd updated this object.
compnerd edited the test plan for this revision. (Show Details)
compnerd added reviewers: joerg, rengolin.
compnerd added a subscriber: Unknown Object (MLST).
rengolin edited edge metadata.Jan 7 2015, 2:56 AM

Hi Slaeem,

I agree we should de-duplicate stuff, but the new behaviour is different in more ways than that. Currently, the personality will be included no matter what headers you have, but with your change, it'll only be included if you do have unwind.h. If I understood correctly, this makes sense, since there's no point in having a personality routine without the unwinding framework, but I just want to make sure this is intentional.

Also, the change from struct to pointer is curious...

cheers,
--renato

lib/builtins/gcc_personality_v0.c
148

This change doesn't seem relevant (even if it is correct) to this patch.

Yes, the additional change of only including it is intentional. However, keep in mind that the header is compiler provided, so you can build this even if you only have a clang installation or if you are building it standalone with a separate copy of libunwind.

lib/builtins/gcc_personality_v0.c
148

Speling change. _Unwind_Context_t was a typedef for struct _Unwind_Context *.

rengolin accepted this revision.Jan 13 2015, 9:44 AM
rengolin edited edge metadata.

Sorry it took so long, I lost track of things. LGTM. Thanks!

lib/builtins/gcc_personality_v0.c
148

I see, due to the header change.

This revision is now accepted and ready to land.Jan 13 2015, 9:44 AM
compnerd closed this revision.Jan 16 2015, 11:32 AM

SVN r225990.