Skip to content
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][complex] Silence pedantic warning #112239

Merged
merged 1 commit into from
Oct 14, 2024
Merged

Conversation

Sh0g0-1758
Copy link
Member

Fix buildbot errors due to #111659

@llvmbot llvmbot added the libc label Oct 14, 2024
@Sh0g0-1758
Copy link
Member Author

@lntue

@llvmbot
Copy link
Member

llvmbot commented Oct 14, 2024

@llvm/pr-subscribers-libc

Author: Shourya Goel (Sh0g0-1758)

Changes

Fix buildbot errors due to #111659


Full diff: https://github.com/llvm/llvm-project/pull/112239.diff

1 Files Affected:

  • (modified) libc/cmake/modules/LLVMLibCCompileOptionRules.cmake (+2)
diff --git a/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake b/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
index 7af67e629471cb..e3a111c62cdb71 100644
--- a/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
+++ b/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
@@ -154,6 +154,7 @@ function(_get_common_compile_options output_var flags)
     list(APPEND compile_options "-Wno-sign-conversion")
     # Silence this warning because _Complex is a part of C99.
     list(APPEND compile_options "-Wno-c99-extensions")
+    list(APPEND compile_options "-Wno-pedantic")
     list(APPEND compile_options "-Wimplicit-fallthrough")
     list(APPEND compile_options "-Wwrite-strings")
     list(APPEND compile_options "-Wextra-semi")
@@ -231,6 +232,7 @@ function(_get_common_test_compile_options output_var c_test flags)
     # list(APPEND compile_options "-Wextra-semi")
     # Silence this warning because _Complex is a part of C99.
     list(APPEND compile_options "-Wno-c99-extensions")
+    list(APPEND compile_options "-Wno-pedantic")
     # if(NOT CMAKE_COMPILER_IS_GNUCXX)
     #   list(APPEND compile_options "-Wnewline-eof")
     #   list(APPEND compile_options "-Wnonportable-system-include-path")

@lntue lntue merged commit 8906bff into llvm:main Oct 14, 2024
7 of 8 checks passed
Copy link
Contributor

@jhuber6 jhuber6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure we want to just silence all of these warnings, the issue seems to be redeclaration of some complex types, we could probably just use the compiler ones.

@lntue
Copy link
Contributor

lntue commented Oct 14, 2024

Not sure we want to just silence all of these warnings, the issue seems to be redeclaration of some complex types, we could probably just use the compiler ones.

So this is the issue: https://godbolt.org/z/zs5Px5Tco
And for some reason, adding Wno-c99-extensions there still not suppresses _Complex warnings.

@nickdesaulniers
Copy link
Member

And for some reason, adding Wno-c99-extensions there still not suppresses _Complex warnings.

Yes it does: https://godbolt.org/z/aacj54rdM -Wno-* needs to occur AFTER -Wpedantic. The way these work in Clang (and GCC which Clang matches its behavior from (AFAIK)) is that the last occurrence of these on the command line "wins." -Wpedantic is a "group" that enables a bunch of warnings implicitly, so then after you'd need to re-disable the flags you want.

-Wpedatic to me means we're sticking with strict ISO C (or C++). If we use extensions (that are supported by the two compilers we support), then -Wpedantic doesn't add much value IMO.

DanielCChen pushed a commit to DanielCChen/llvm-project that referenced this pull request Oct 16, 2024
bricknerb pushed a commit to bricknerb/llvm-project that referenced this pull request Oct 17, 2024
EricWF pushed a commit to efcs/llvm-project that referenced this pull request Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants