[libc] `-Wimplicit-fallthrough`, `-Wwrite-strings` and non-GCC warnings #124036

vinay-deshmukh · 2025-01-23T00:19:02Z

Enabled -Wimplicit-fallthrough
Enabled -Wwrite-strings
Enabled non-GCC warnings
Fix non-GCC to mean clang
Move -Wstrict-prototypes to common section

Relates to #119281

github-actions · 2025-01-23T00:20:39Z

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Discourse for more information.

github-actions · 2025-01-23T00:22:20Z

✅ With the latest revision this PR passed the C/C++ code formatter.

llvmbot · 2025-01-23T02:28:16Z

@llvm/pr-subscribers-libc

Author: Vinay Deshmukh (vinay-deshmukh)

Changes

Enabled -Wimplicit-fallthrough
Enabled -Wwrite-strings
Enabled non-GCC warnings

See: #122835 (comment)

Relates to #119281

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

1 Files Affected:

(modified) libc/cmake/modules/LLVMLibCTestRules.cmake (+9-9)

diff --git a/libc/cmake/modules/LLVMLibCTestRules.cmake b/libc/cmake/modules/LLVMLibCTestRules.cmake
index 96fa6c3a707e47..e676130c6b9a81 100644
--- a/libc/cmake/modules/LLVMLibCTestRules.cmake
+++ b/libc/cmake/modules/LLVMLibCTestRules.cmake
@@ -33,8 +33,8 @@ function(_get_common_test_compile_options output_var c_test flags)
     endif()
     # list(APPEND compile_options "-Wconversion")
     # list(APPEND compile_options "-Wno-sign-conversion")
-    # list(APPEND compile_options "-Wimplicit-fallthrough")
-    # list(APPEND compile_options "-Wwrite-strings")
+    list(APPEND compile_options "-Wimplicit-fallthrough")
+    list(APPEND compile_options "-Wwrite-strings")
     list(APPEND compile_options "-Wextra-semi")
     # Silence this warning because _Complex is a part of C99.
     if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
@@ -46,13 +46,13 @@ function(_get_common_test_compile_options output_var c_test flags)
       list(APPEND compile_options "-Wno-gnu-imaginary-constant")
     endif()
     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")
-    #   list(APPEND compile_options "-Wstrict-prototypes")
-    #   list(APPEND compile_options "-Wthread-safety")
-    #   list(APPEND compile_options "-Wglobal-constructors")
-    # endif()
+    if(NOT CMAKE_COMPILER_IS_GNUCXX)
+      list(APPEND compile_options "-Wnewline-eof")
+      list(APPEND compile_options "-Wnonportable-system-include-path")
+      list(APPEND compile_options "-Wstrict-prototypes")
+      list(APPEND compile_options "-Wthread-safety")
+      # list(APPEND compile_options "-Wglobal-constructors")
+    endif()
   endif()
   set(${output_var} ${compile_options} PARENT_SCOPE)
 endfunction()

…misc

vinay-deshmukh · 2025-01-23T22:32:46Z

Add @nickdesaulniers as reviewer

Context: #122835 (comment)

nickdesaulniers

thanks for the patch!

libc/cmake/modules/LLVMLibCTestRules.cmake

…misc

nickdesaulniers · 2025-01-28T17:28:52Z

Sorry, I'm hitting some pre-existing build breakages with newer versions of gcc (#120427). I'd like to wait to get that resolved first, so that I can re-test this PR with both clang and gcc.

nickdesaulniers · 2025-01-28T21:12:59Z

Ok, I had to hack around a few issues (linked from this PR) to get our gcc-14 build working. But I'm confident this PR can land (once -Wextra-semi and -Wstrict-prototypes are moved back to clang-only for now).

Fixes the diagnostic observed with gcc-14: llvm-project/libc/include/llvm-libc-macros/containerof-macro.h:17:13: error: cast from type ‘const char*’ to type ‘void*’ casts away qualifiers [-Werror=cast-qual] 17 | (type *)(void *)((const char *)__ptr - offsetof(type, member)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ We're trying to turn on more warnings for the tests in llvm#124036, so enable -Wcast-qual for GCC while we're at it.

…misc

vinay-deshmukh · 2025-02-01T01:16:20Z

@nickdesaulniers This PR should be good to go

nickdesaulniers · 2025-02-04T16:50:32Z

libc/cmake/modules/LLVMLibCTestRules.cmake

-  # Remove -fno-math-errno if it was added.
-  if(LIBC_ADD_FNO_MATH_ERRNO)
-    list(REMOVE_ITEM compile_options "-fno-math-errno")
-  endif()


Did you intend to remove this?

IIRFC yes, Oh I just realized that's removing -fno-math-error

let me revert that change

libc/cmake/modules/LLVMLibCTestRules.cmake

nickdesaulniers

Thanks for the patch! Need us to merge this for you?

vinay-deshmukh · 2025-02-07T01:45:35Z

@nickdesaulniers Yes please merge it for me. Thank you!

For the next work item, if I understand correctly, the plan is to uncomment these one by one and hence, patch each warning in it's own PR, right?

llvm-project/libc/cmake/modules/LLVMLibCTestRules.cmake

Lines 33 to 42 in 6807164

    
               # list(APPEND compile_options "-Wall") 
        
               # list(APPEND compile_options "-Wextra") 
        
               # -DLIBC_WNO_ERROR=ON if you can't build cleanly with -Werror. 
        
               if(NOT LIBC_WNO_ERROR) 
        
                 # list(APPEND compile_options "-Werror") 
        
               endif() 
        
               # list(APPEND compile_options "-Wconversion") 
        
               # list(APPEND compile_options "-Wno-sign-conversion") 
        
               # list(APPEND compile_options "-Wimplicit-fallthrough") 
        
               # list(APPEND compile_options "-Wwrite-strings")

nickdesaulniers · 2025-02-07T20:50:41Z

For the next work item, if I understand correctly, the plan is to uncomment these one by one and hence, patch each warning in it's own PR, right?

Exactly, thank you!

…warnings (llvm#124036) * Enabled `-Wimplicit-fallthrough` * Enabled `-Wwrite-strings` * Enabled non-GCC warnings * Fix non-GCC to mean `clang` * Move `-Wstrict-prototypes` to common section See: llvm#122835 (comment) Relates to llvm#119281

vinay-deshmukh added 2 commits January 22, 2025 18:42

-Wwrite-strings

d600a01

-Wimplicit-fallthrough

1c5c871

vinay-deshmukh changed the title ~~[libc] -Wunused-parameter and few NOOPs~~ [libc] -Wwrite-strings and -Wextra -> -Wunused-parameter Jan 23, 2025

vinay-deshmukh changed the title ~~[libc] -Wwrite-strings and -Wextra -> -Wunused-parameter~~ [libc] -Wimplicit-fallthrough and -Wwrite-strings Jan 23, 2025

vinay-deshmukh force-pushed the vinay-issue-119281-misc branch from ca5a280 to 1c5c871 Compare January 23, 2025 02:00

non GCC warnings

51b9c57

vinay-deshmukh changed the title ~~[libc] -Wimplicit-fallthrough and -Wwrite-strings~~ [libc] -Wimplicit-fallthrough, -Wwrite-strings and non-GCC warnings Jan 23, 2025

vinay-deshmukh marked this pull request as ready for review January 23, 2025 02:27

llvmbot added the libc label Jan 23, 2025

vinay-deshmukh mentioned this pull request Jan 23, 2025

[libc] [Task] Prepare to enable disabled warnings #122835

Open

Merge remote-tracking branch 'upstream/main' into vinay-issue-119281-…

3cc6100

…misc

nickdesaulniers reviewed Jan 23, 2025

View reviewed changes

libc/cmake/modules/LLVMLibCTestRules.cmake Outdated Show resolved Hide resolved

vinay-deshmukh added 2 commits January 24, 2025 21:11

PR review comments

be9ec1a

Merge remote-tracking branch 'upstream/main' into vinay-issue-119281-…

ff3b741

…misc

vinay-deshmukh requested a review from nickdesaulniers January 25, 2025 02:38

nickdesaulniers mentioned this pull request Jan 28, 2025

[libc] gcc build errors about conversion from 'int' to 'short unsigned int' #120427

Closed

This was referenced Jan 28, 2025

[libc][test] -Werror=maybe-uninitialized in high_precision_decimal_test.cpp #124808

Open

[libc] -Wnarrowing in many _Float16 tests with gcc-14 #124801

Closed

[libc][test] spew of -Wuninitialized with gcc-14 #124814

Open

nickdesaulniers mentioned this pull request Jan 28, 2025

[libc] fix -Wcast-qual in containerof macro #124849

Open

nickdesaulniers mentioned this pull request Jan 28, 2025

[libc] register exhaustion with gcc-14 #124852

Open

vinay-deshmukh added 2 commits January 28, 2025 21:03

Move -Wextra-semi and -Wstrict-prototypes back to Clang only

56c7755

Merge remote-tracking branch 'upstream/main' into vinay-issue-119281-…

519bf7b

…misc

nickdesaulniers requested changes Feb 4, 2025

View reviewed changes

Update LLVMLibCTestRules.cmake

d8bf783

vinay-deshmukh requested a review from nickdesaulniers February 5, 2025 23:19

nickdesaulniers reviewed Feb 5, 2025

View reviewed changes

libc/cmake/modules/LLVMLibCTestRules.cmake Outdated Show resolved Hide resolved

Fix NL

9c91ac9

nickdesaulniers approved these changes Feb 6, 2025

View reviewed changes

nickdesaulniers merged commit dee2092 into llvm:main Feb 7, 2025
14 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[libc] `-Wimplicit-fallthrough`, `-Wwrite-strings` and non-GCC warnings #124036

[libc] `-Wimplicit-fallthrough`, `-Wwrite-strings` and non-GCC warnings #124036

Uh oh!

vinay-deshmukh commented Jan 23, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Jan 23, 2025

Uh oh!

github-actions bot commented Jan 23, 2025 •

edited

Loading

Uh oh!

llvmbot commented Jan 23, 2025

Uh oh!

vinay-deshmukh commented Jan 23, 2025

Uh oh!

nickdesaulniers left a comment

Uh oh!

Uh oh!

nickdesaulniers commented Jan 28, 2025

Uh oh!

nickdesaulniers commented Jan 28, 2025

Uh oh!

vinay-deshmukh commented Feb 1, 2025

Uh oh!

nickdesaulniers Feb 4, 2025

Uh oh!

vinay-deshmukh Feb 5, 2025

Uh oh!

Uh oh!

nickdesaulniers left a comment

Uh oh!

vinay-deshmukh commented Feb 7, 2025

Uh oh!

Uh oh!

nickdesaulniers commented Feb 7, 2025

Uh oh!

Uh oh!

[libc] -Wimplicit-fallthrough, -Wwrite-strings and non-GCC warnings #124036

[libc] -Wimplicit-fallthrough, -Wwrite-strings and non-GCC warnings #124036

Uh oh!

Conversation

vinay-deshmukh commented Jan 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jan 23, 2025

Uh oh!

github-actions bot commented Jan 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jan 23, 2025

Uh oh!

vinay-deshmukh commented Jan 23, 2025

Uh oh!

nickdesaulniers left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nickdesaulniers commented Jan 28, 2025

Uh oh!

nickdesaulniers commented Jan 28, 2025

Uh oh!

vinay-deshmukh commented Feb 1, 2025

Uh oh!

nickdesaulniers Feb 4, 2025

Choose a reason for hiding this comment

Uh oh!

vinay-deshmukh Feb 5, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nickdesaulniers left a comment

Choose a reason for hiding this comment

Uh oh!

vinay-deshmukh commented Feb 7, 2025

Uh oh!

Uh oh!

nickdesaulniers commented Feb 7, 2025

Uh oh!

Uh oh!

[libc] `-Wimplicit-fallthrough`, `-Wwrite-strings` and non-GCC warnings #124036

[libc] `-Wimplicit-fallthrough`, `-Wwrite-strings` and non-GCC warnings #124036

vinay-deshmukh commented Jan 23, 2025 •

edited

Loading

github-actions bot commented Jan 23, 2025 •

edited

Loading