[libunwind][Modules] Add unwind_arm_ehabi.h and unwind_itanium.h to the unwind module)

[iana]

Summary

Add unwind_arm_ehabi.h and unwind_itanium.h to the unwind module and use angle includes to include them.

[ldionne]

libunwind/include/unwind_arm_ehabi.h

16–18 ↗

(On Diff #498567)

Why is this needed? We only include those after checking #if defined(_LIBUNWIND_ARM_EHABI).

[iana]

libunwind/include/unwind_arm_ehabi.h

16–18 ↗

(On Diff #498567)

The module will build all of its headers, so without these guards you'll get a lot of redeclaration errors between the two unwind_ headers.

It might maybe work to put them in independent modules and mark them as conflicting. I don't really think that's the right thing to do since these two headers are just implementation details for unwind.h.

We could make them textual, but then they don't get put in the pcm which isn't what we want.

We could make them excluded and then one or the other should be pulled into unwind when it builds, but that's a fragile mechanism since some other module could unexpectedly include them. It's also not really what we're trying to do, they should be part of unwind and not really excluded.

So I think the right thing to do is make each of these headers standalone and able to be built in the same module.

[ldionne]

libunwind/include/unwind_arm_ehabi.h

16–18 ↗

(On Diff #498567)

The problem is that this is a general pattern we have in a lot of places. Today, we use headers as a mechanism to (quite textually) separate code into a different physical location. These headers are implementation details and they are never intended to be included directly by the user. I'm not a modules expert, but it looks to me like those are effectively textual headers that are meant to be copy-pasted into unwind.h depending on the API we want to provide. If so, we should make them textual even if that has a negative impact on the generated PCM, since that is the proper semantics of those headers.

[iana]

libunwind/include/unwind_arm_ehabi.h

16–18 ↗

(On Diff #498567)

Err, it would be in the unwind PCM since unwind.h includes it, it just has the potential to get copied into other PCMs. @Bigcheese suggested we make it private textual to prevent that, let me give it a try. I still feel a little icky about making the module behavior differ from textual behavior (modules will emit an error if the unwind_ headers are directly included but textual won't), but it's not so bad.

I guess it's open to interpretation, but I see textual headers as things like assert.h where it's expected that you can include it multiple times with different values of NDEBUG in your code, or for X Macro generator headers. The unwind_ headers don't really fit either of those descriptions which is why I feel like they're a bad fit for textual, especially when it's a relatively small change to make them standalone.

That said, I can be out voted, what's really important to me is these belong to a module. I'll give private textual a try and see if it works out.

[iana]

Make the unwind_ files private textual headers and revert the changes to them.

[iana]

libunwind/include/unwind_arm_ehabi.h

16–18 ↗

(On Diff #498567)

It looks like private textual works.

[ldionne]

Can you please just rebase onto main and re-upload to give this one last CI spin? There's a Windows failure I don't understand. I think it's unrelated but rebasing and running the CI again should confirm that.

[iana]

Rebasing

[iana]

In D144323#4204034, @ldionne wrote:

Can you please just rebase onto main and re-upload to give this one last CI spin? There's a Windows failure I don't understand. I think it's unrelated but rebasing and running the CI again should confirm that.

You got it!

[ldionne]

LGTM, the failures in the libc++ CI are known flakes and the Windows one is a Clangd issue, most likely unrelated.

libunwind/include/libunwind.modulemap

 module libunwind [system] {
   header "libunwind.h"
   export *
 }
 
 module unwind [system] {
   header "__libunwind_config.h"
   header "unwind.h"
+  private textual header "unwind_arm_ehabi.h"
+  private textual header "unwind_itanium.h"
+
   export *
 }

libunwind/include/unwind.h

typedef enum {
   _UA_HANDLER_FRAME = 4,
   _UA_FORCE_UNWIND = 8,
   _UA_END_OF_STACK = 16 // gcc extension to C++ ABI
 } _Unwind_Action;
 
 typedef struct _Unwind_Context _Unwind_Context;   // opaque
 
 #if defined(_LIBUNWIND_ARM_EHABI)
-#include "unwind_arm_ehabi.h"
+#include <unwind_arm_ehabi.h>
 #else
-#include "unwind_itanium.h"
+#include <unwind_itanium.h>
 #endif
 
 typedef _Unwind_Reason_Code (*_Unwind_Stop_Fn)
     (int version,
      _Unwind_Action actions,
      _Unwind_Exception_Class exceptionClass,
      _Unwind_Exception* exceptionObject,
      struct _Unwind_Context* context,