This is an archive of the discontinued LLVM Phabricator instance.

[libc++][Android] Rename user NS to User to avoid conflict with struct
ClosedPublic

Authored by rprichard on Dec 5 2022, 4:41 PM.

Details

Summary

Bionic's sys/user.h declares a "struct user". The header tends to be
included, and when it is, it conflicts with "namespace user". Rename
user to User.

Diff Detail

Event Timeline

rprichard created this revision.Dec 5 2022, 4:41 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 5 2022, 4:41 PM
Herald added a subscriber: danielkiss. · View Herald Transcript
rprichard requested review of this revision.Dec 5 2022, 4:41 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 5 2022, 4:41 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript

The capitalized namespace User spelling was already used in libcxx/test/std/algorithms/alg.nonmodifying/alg.search/search.pass.cpp (D72640).

In NDK r25b, including <system_error> includes sys/user.h via this chain:

/usr/include/c++/v1/system_error
/usr/include/c++/v1/stdexcept
/usr/include/c++/v1/iosfwd
/usr/include/c++/v1/wchar.h
/usr/local/include/wchar.h
/usr/include/wchar.h
/usr/include/time.h
/usr/include/sys/time.h
/usr/include/sys/select.h
/usr/include/signal.h
/usr/include/sys/ucontext.h
/usr/include/sys/user.h
ldionne accepted this revision.Dec 6 2022, 8:32 AM
ldionne added a subscriber: ldionne.

That's pretty terrible (note that Darwin also has its share of similar sins). Would you be able to report this as a bug to Android? Perhaps the struct can be removed when a macro is provided and they can start some sort of deprecation path?

I won't block this, but it would be really nice to at least get the ball rolling on fixing this issue in the system headers.

This revision is now accepted and ready to land.Dec 6 2022, 8:32 AM

Would you be able to report this as a bug to Android? Perhaps the struct can be removed when a macro is provided and they can start some sort of deprecation path?

We can add a macro, but I doubt we'll ever default to hiding this struct. Every time we've tried to make our headers more hygenic our users have told us they'd prefer it if we didn't because it breaks their builds (in trivially fixable ways, but they don't care; they don't want it to break regardless).

Our plan since then has been to ignore these types of problems until modules can solve them properly.

This revision was landed with ongoing or failed builds.Dec 6 2022, 5:30 PM
This revision was automatically updated to reflect the committed changes.