This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Include "bits/alltypes.h" to provide mbstate_t when using musl libc
ClosedPublic

Authored by pzheng on May 30 2023, 11:24 AM.

Details

Summary

With D148542, we ran into the following libc++ build error when using musl libc.

.../musl/include/bits/alltypes.h:354:16:
error: definition of type '__mbstate_t' conflicts with typedef of the same name
typedef struct __mbstate_t { unsigned __opaque1, __opaque2; } mbstate_t;
               ^
.../sysroot/usr/include/bits/types/__mbstate_t.h:21:3: note: '__mbstate_t' declared here
} __mbstate_t;
  ^
1 error generated.

This is because the mbstate_t definition in musl libc conflicts with the one
from "bits/types/mbstate_t.h", and this patch attempts to fix this build issue
when musl libc is used.

Diff Detail

Event Timeline

pzheng created this revision.May 30 2023, 11:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 30 2023, 11:24 AM
pzheng requested review of this revision.May 30 2023, 11:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 30 2023, 11:24 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
iana added a comment.May 30 2023, 11:30 AM

Thanks for doing this one!

libcxx/include/__mbstate_t.h
39

Should we undef this after including alltypes?

pzheng updated this revision to Diff 526764.May 30 2023, 1:17 PM

Add undef after include

pzheng marked an inline comment as done.May 30 2023, 1:18 PM
pzheng added inline comments.
libcxx/include/__mbstate_t.h
39

That's a good point! I just added undef in the latest revision.

iana accepted this revision.May 30 2023, 1:34 PM
This revision was not accepted when it landed; it landed in state Needs Review.May 30 2023, 5:59 PM
This revision was automatically updated to reflect the committed changes.
pzheng marked an inline comment as done.

Please don't land libc++ patches until the libc++ group has approved.

Please don't land libc++ patches until the libc++ group has approved.

Oops, sorry for my ignorance, @philnik... I didn't know this rule existed for libc++ patch reviews. Thank you for educating me.

Please don't land libc++ patches until the libc++ group has approved.

Oops, sorry for my ignorance, @philnik... I didn't know this rule existed for libc++ patch reviews. Thank you for educating me.

In general in Phabricator when there are blocking reviewers you should wait on their approval.

Please don't land libc++ patches until the libc++ group has approved.

Oops, sorry for my ignorance, @philnik... I didn't know this rule existed for libc++ patch reviews. Thank you for educating me.

In general in Phabricator when there are blocking reviewers you should wait on their approval.

Thanks for pointing it out, @Mordante. Again, my apologies for overlooking this.