We haven't eliminated C++ library dependency altogether in D57251,
UnwindCursor.hpp had an unused dependency on <algorithm> which was
pulling in other C++ headers. Removing that dependency also revealed
(correctly) that we need our own global placement new declaration. Now
libunwind should be independent of the C++ library.
Details
- Reviewers
ldionne EricWF mstorsjo mcgrathr - Commits
- rG7fac51724f88: Drop the dependency on <algorithm>, add placement new inline
rG91a606e6c460: [libunwind] Drop the dependency on <algorithm>, add placement new inline
rUNW352553: Drop the dependency on <algorithm>, add placement new inline
rL352553: Drop the dependency on <algorithm>, add placement new inline
rL352384: [libunwind] Drop the dependency on <algorithm>, add placement new inline
rUNW352384: [libunwind] Drop the dependency on <algorithm>, add placement new inline
Diff Detail
- Repository
- rUNW libunwind
Event Timeline
Should you add -nostdinc++ to the compilation flags to drive home that this code can't depend on any library headers?
libunwind/src/libunwind.cpp | ||
---|---|---|
30 | Why do we need to define this? I thought placement new was handled by Clang directly, without requiring a declaration? |
libunwind/src/libunwind.cpp | ||
---|---|---|
30 | That's what I assumed but that doesn't seem to be the case, without this definition the compilation fails because Clang cannot find the operator new. |
Done, we were already using -nostdinc++ but we were also adding libc++ include path explicitly into the list of include directories, so I dropped that logic and made -nostdinc++ unconditional.
libunwind/src/libunwind.cpp | ||
---|---|---|
30 | Actually I think you're right, if I only provide declaration, Clang handles the definition. |
libunwind/src/libunwind.cpp | ||
---|---|---|
30 | Spelling nitpick: "should not depend on", or "is not dependent on" |
This patch breaks compilation of Unwind-seh.cpp, which also needs a declaration of the placement new operator.
libunwind/src/libunwind.cpp | ||
---|---|---|
15 | Just to confirm -- this is handled by #include <stdlib.h> because we use the global ::getenv() function, right? |
libunwind/src/libunwind.cpp | ||
---|---|---|
15 | Yes, correct. |
I missed some more C++ library uses in UnwindCursor.hpp and Unwind-EHABI.cpp which broke Arm bots and so I had to revert the change. This should now be addressed. Please let me know if this is okay to submit.
I believe the code looks good, although I haven't tested the EHABI unwinding code after this change to see if it still works.
As much as I hate duplicating stuff like upper_bound, I find the removal of the circular dependency so nice that I don't care.
Just to confirm -- this is handled by #include <stdlib.h> because we use the global ::getenv() function, right?