-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[libc] Add -Wextra for libc tests #133643
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] Add -Wextra for libc tests #133643
Conversation
vinay-deshmukh
commented
Mar 30, 2025
- Relates to: [libc] re-enable warnings for tests #119281
|
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
edaf208 to
a2cd02a
Compare
|
Any suggestions on how I can diagnose the I'm unable to do a full build on Macbook m1, overlay build did pass successfully FWIW |
|
Looks like LlvmLibcMemcmpTest.SizeSweep is generating an illegal instruction, somehow. Perhaps you can run the test under lldb, and it can give you a backtrace of where the exeception is coming from. Build a debug build. |
|
The overlay build when done in Debug mode is passing for me, so unable to reproduce this CI failure locally. The CI seems to be doing a full build which is not supported on Macbook m1. Is there a way to 'disable' the |
|
@nickdesaulniers @lntue Please review this PR when you get a chance! Thanks! |
1 similar comment
|
@nickdesaulniers @lntue Please review this PR when you get a chance! Thanks! |
d768cb6 to
8defd89
Compare
8defd89 to
38b927c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM, does this also work when building in overlay mode or with gcc?
Unfortunately, I don't have a local gcc install available. I tried a local overlay build on commit 34accca with: and which passed. FWIW CI was building until I merged from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/71/builds/28863 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/179/builds/28273 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/171/builds/28156 Here is the relevant piece of the build log for the reference |
This reverts commit e617dc8.
|
The pre-merge seems to have not caught all the errors, or maybe the pre-merge checks took place some time ago. I'm going to revert this. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/183/builds/17157 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/196/builds/11243 Here is the relevant piece of the build log for the reference |