Teach libc++ to use native NetBSD's max_align_t

[krytarowski]

Summary

The NetBSD headers ship with max_align_t, that is not
compatible with the fallback version in libc++.

There is no defined a compiler specific symbol in the headers like:

• __CLANG_MAX_ALIGN_T_DEFINED
• _GCC_MAX_ALIGN_T
• __DEFINED_max_align_t

Sponsored by <The NetBSD Foundation>

[krytarowski]

ping

[krytarowski]

ping^2

[bsdjhb]

FWIW, for FreeBSD I defined __CLANG_MAX_ALIGN_T and _GCC_MAX_ALIGN_T in FreeBSD's <stddef.h> when defining the typedef to handle this.

[krytarowski]

I saw this and I think that it shall be handled in libc++. NetBSD doesn't care if c++ runtime library is libstdc++, libc++, none or a different one.

[krytarowski]

It's similar to MUSL, __DEFINED_max_align_t is musl specific and they don't care about libc++ or not on top of it.

[krytarowski]

ping^3

[chandlerc]

I don't have any problem with this patch's code in theory but:

1. The question of whether this macro approach is better or worse than FreeBSD's or MUSL's seems a question for libc++ maintainers, not me.
2. The question of whether this is correct for NetBSD seems for the NetBSD maintainer.

Since the above folks are already on the list of reviewers, I'll bow out of this review.

include/stddef.h

55

Unrelated to your patch, but this comment is now amazingly out of place.

[krytarowski]

If there are no more comments, I will land this by the end of this week.

[chandlerc]

In D47814#1206372, @krytarowski wrote:

If there are no more comments, I will land this by the end of this week.

Just for the record, this is not OK and not how LLVM's code review works.

You can and must wait for review. I think Joerg already marked this as accepted, but it didn't send out an email[1]. That means it got reviewed, so all is good here, but I wanted to make it clear to others on the list what the expectations are here.

-Chandler

[1]: FYI, it's useful to put *some* text in the text box when marking a revision as accepted on Phab so that it will in fact send email. I typically put "LGTM" or a brief thank-you note to the author.

[krytarowski]

@chandlerc thank you for your note. I've originally treated this change as an obvious code change, just the reviewers regardless of reviewing an analogous change for MUSL, had no interest in NetBSD.

[ldionne]

include/stddef.h

55

Nope, based on git blame I think it's still where it should be.

[chandlerc]

(To be clear, this continues to not be related to this patch, but happy to discuss...)

include/stddef.h

55

I don't understand ... I'm not saying this patch has anything to do with it. But commenting *this* block with the above comment doesn't parse for me at all... Maybe its that the wording of the comment doesn't parse for me.

Perhaps what it means is "Only provide our own max_align_t (rather than relying on a compiler-provided one) when we have to by testing for the cases where the compiler's <stddef.h> won't work."?

include/cstddef

 #include <__nullptr>
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 using ::ptrdiff_t;
 using ::size_t;
 
 #if defined(__CLANG_MAX_ALIGN_T_DEFINED) || defined(_GCC_MAX_ALIGN_T) || \
-    defined(__DEFINED_max_align_t)
+    defined(__DEFINED_max_align_t) || defined(__NetBSD__)
 // Re-use the compiler's <stddef.h> max_align_t where possible.
 using ::max_align_t;
 #else
 typedef long double max_align_t;
 #endif
 
 _LIBCPP_END_NAMESPACE_STD
 

include/stddef.h

 
 #ifdef __cplusplus
 
 extern "C++" {
 #include <__nullptr>
 using std::nullptr_t;
 }
 
 // Re-use the compiler's <stddef.h> max_align_t where possible.
 #if !defined(__CLANG_MAX_ALIGN_T_DEFINED) && !defined(_GCC_MAX_ALIGN_T) && \
-    !defined(__DEFINED_max_align_t)
+    !defined(__DEFINED_max_align_t) && !defined(__NetBSD__)
 typedef long double max_align_t;
 #endif
 
 #endif
 
 #endif  // _LIBCPP_STDDEF_H