New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[libc] fix includes of src/__support/macros/config.h #86579
Comments
Hi! This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:
If you have any further questions about this issue, don't hesitate to ask via a comment in the thread below. |
@llvm/issue-subscribers-good-first-issue Author: Nick Desaulniers (nickdesaulniers)
A few of these are being cleaned up in #86554 so it's probably best for that to land first. But, I can see latent includes of src/__support/macros/config.h that look wrong, such as the one in libc/src/string/memory_utils/aarch64/inline_memcpy.h which has a comment that it includes that header for the definition of LIBC_INLINE.
LIBC_INLINE is defined in libc/src/__support/macros/attributes.h. As such libc/src/string/memory_utils/aarch64/inline_memcpy.h should include libc/src/__support/macros/attributes.h, not src/__support/macros/config.h and the corresponding cmake+bazel build rules fixed. Probably there are other places too where a similar pattern is occurring and show be fixed up, too. I suspect this is a fixup for 29f8e07. |
@llvm/issue-subscribers-libc Author: Nick Desaulniers (nickdesaulniers)
A few of these are being cleaned up in #86554 so it's probably best for that to land first. But, I can see latent includes of src/__support/macros/config.h that look wrong, such as the one in libc/src/string/memory_utils/aarch64/inline_memcpy.h which has a comment that it includes that header for the definition of LIBC_INLINE.
LIBC_INLINE is defined in libc/src/__support/macros/attributes.h. As such libc/src/string/memory_utils/aarch64/inline_memcpy.h should include libc/src/__support/macros/attributes.h, not src/__support/macros/config.h and the corresponding cmake+bazel build rules fixed. Probably there are other places too where a similar pattern is occurring and show be fixed up, too. I suspect this is a fixup for 29f8e07. |
@nickdesaulniers I would like to work on this issue! |
Hello @nickdesaulniers I tried looking into this and seems like you did few cleanup. Do we need more similar cleanups. I am not sure because I am facing issues while running the bazel test. Initially I tried running the same commands
but I am getting this error while running this command https://pastebin.com/NHQA677f. Then I tried running the bazel test using the command https://discord.com/channels/636084430946959380/636732994891284500/1224100594244124742 |
From discord, it sounds like you're on macos? If so, something has likely bitrot in the bazel build for ensuring we don't build linux files on non-linux hosts. EDIT: if that's the case, then just stick with the cmake based build (unless that has bitrot as well). |
Yes, these are still necessary. |
I am getting this error when running test using the |
Yeah, the issue is that the build has bitrot on Darwin (guessing you're on an aarch64 based Mac?). I broke this recently in #85955 when I deleted libc/src/__support/OSUtil/darwin/quick_exit.h rather than convert it to libc/src/__support/OSUtil/darwin/quick_exit.cpp (as was done for libc/src/__support/OSUtil/baremetal/quick_exit.cpp as an example). Currently, all first party developers are developing on Linux hosts; we could use assistance in getting the MacOS build back up to snuff. If you'd like to help with that, it may be a better place to start than this bug. We can discuss the details more in a new (not-yet-created) issue, or on discord. |
A few of these are being cleaned up in #86554 so it's probably best for that to land first. But, I can see latent includes of src/__support/macros/config.h that look wrong, such as the one in libc/src/string/memory_utils/aarch64/inline_memcpy.h which has a comment that it includes that header for the definition of LIBC_INLINE.
LIBC_INLINE is defined in libc/src/__support/macros/attributes.h. As such libc/src/string/memory_utils/aarch64/inline_memcpy.h should include libc/src/__support/macros/attributes.h, not src/__support/macros/config.h and the corresponding cmake+bazel build rules fixed.
Probably there are other places too where a similar pattern is occurring and show be fixed up, too.
I suspect this is a fixup for 29f8e07.
The text was updated successfully, but these errors were encountered: