Re: [PATCH] D31702: Append -w when LLVM_ENABLE_WARNINGS is Off.

[Vassil Vassilev via cfe-commits]

On 6/4/20 6:58 AM, Sean Silva wrote:

Can we revert this?

In our downstream project we were bitten by this: 
https://github.com/google/mlir-npcomp/commit/cd7258dbd48bd9254c6741cab2d3f4e36cbd3c84


It seems that now by default anybody that depends on LLVM with CMake 
will globally get a `-w` passed to their compiler. That doesn't seem 
like good behavior.



  Wouldn't that be the case for all append clauses in that file?




-- Sean Silva

On Wed, Apr 12, 2017 at 1:56 PM Vassil Vassilev via Phabricator via 
llvm-commits > wrote:


v.g.vassilev closed this revision.
v.g.vassilev added a comment.

Landed in r300100.


https://reviews.llvm.org/D31702



___
llvm-commits mailing list
llvm-comm...@lists.llvm.org 
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D31702: Append -w when LLVM_ENABLE_WARNINGS is Off.

[Sean Silva via cfe-commits]

Can we revert this?

In our downstream project we were bitten by this:
https://github.com/google/mlir-npcomp/commit/cd7258dbd48bd9254c6741cab2d3f4e36cbd3c84

It seems that now by default anybody that depends on LLVM with CMake will
globally get a `-w` passed to their compiler. That doesn't seem like good
behavior.

-- Sean Silva

On Wed, Apr 12, 2017 at 1:56 PM Vassil Vassilev via Phabricator via
llvm-commits  wrote:

> v.g.vassilev closed this revision.
> v.g.vassilev added a comment.
>
> Landed in r300100.
>
>
> https://reviews.llvm.org/D31702
>
>
>
> ___
> llvm-commits mailing list
> llvm-comm...@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31702: Append -w when LLVM_ENABLE_WARNINGS is Off.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev closed this revision.
v.g.vassilev added a comment.

Landed in r300100.


https://reviews.llvm.org/D31702



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31702: Append -w when LLVM_ENABLE_WARNINGS is Off.

[Reid Kleckner via Phabricator via cfe-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm


https://reviews.llvm.org/D31702



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31702: Append -w when LLVM_ENABLE_WARNINGS is Off.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev updated this revision to Diff 94180.
v.g.vassilev marked an inline comment as done.
v.g.vassilev added a comment.

Ah good point. Thanks!


https://reviews.llvm.org/D31702

Files:
  cmake/modules/HandleLLVMOptions.cmake


Index: cmake/modules/HandleLLVMOptions.cmake
===
--- cmake/modules/HandleLLVMOptions.cmake
+++ cmake/modules/HandleLLVMOptions.cmake
@@ -561,6 +561,10 @@
   add_flag_if_supported("-Wstring-conversion" STRING_CONVERSION_FLAG)
 endif (LLVM_ENABLE_WARNINGS AND (LLVM_COMPILER_IS_GCC_COMPATIBLE OR CLANG_CL))
 
+if (LLVM_COMPILER_IS_GCC_COMPATIBLE AND NOT LLVM_ENABLE_WARNINGS)
+  append("-w" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
+endif()
+
 macro(append_common_sanitizer_flags)
   if (NOT MSVC)
 # Append -fno-omit-frame-pointer and turn on debug info to get better


Index: cmake/modules/HandleLLVMOptions.cmake
===
--- cmake/modules/HandleLLVMOptions.cmake
+++ cmake/modules/HandleLLVMOptions.cmake
@@ -561,6 +561,10 @@
   add_flag_if_supported("-Wstring-conversion" STRING_CONVERSION_FLAG)
 endif (LLVM_ENABLE_WARNINGS AND (LLVM_COMPILER_IS_GCC_COMPATIBLE OR CLANG_CL))
 
+if (LLVM_COMPILER_IS_GCC_COMPATIBLE AND NOT LLVM_ENABLE_WARNINGS)
+  append("-w" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
+endif()
+
 macro(append_common_sanitizer_flags)
   if (NOT MSVC)
 # Append -fno-omit-frame-pointer and turn on debug info to get better
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31702: Append -w when LLVM_ENABLE_WARNINGS is Off.

[Yaron Keren via Phabricator via cfe-commits]

yaron.keren added inline comments.



Comment at: cmake/modules/HandleLLVMOptions.cmake:562
   add_flag_if_supported("-Wstring-conversion" STRING_CONVERSION_FLAG)
+else()
+  append("-w" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)

note this is an else to 
if (LLVM_ENABLE_WARNINGS AND (LLVM_COMPILER_IS_GCC_COMPATIBLE OR CLANG_CL))
so for non-gcc non-clang_cl compilers you'd also end up here.

shouldn't the condition be
if (LLVM_COMPILER_IS_GCC_COMPATIBLE AND NOT LLVM_ENABLE_WARNINGS)



Repository:
  rL LLVM

https://reviews.llvm.org/D31702



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31702: Append -w when LLVM_ENABLE_WARNINGS is Off.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev created this revision.
Herald added a subscriber: mgorny.

This is very useful for projects which embed LLVM in their codebases.


Repository:
  rL LLVM

https://reviews.llvm.org/D31702

Files:
  cmake/modules/HandleLLVMOptions.cmake


Index: cmake/modules/HandleLLVMOptions.cmake
===
--- cmake/modules/HandleLLVMOptions.cmake
+++ cmake/modules/HandleLLVMOptions.cmake
@@ -559,6 +559,8 @@
 
   # Enable -Wstring-conversion to catch misuse of string literals.
   add_flag_if_supported("-Wstring-conversion" STRING_CONVERSION_FLAG)
+else()
+  append("-w" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
 endif (LLVM_ENABLE_WARNINGS AND (LLVM_COMPILER_IS_GCC_COMPATIBLE OR CLANG_CL))
 
 macro(append_common_sanitizer_flags)


Index: cmake/modules/HandleLLVMOptions.cmake
===
--- cmake/modules/HandleLLVMOptions.cmake
+++ cmake/modules/HandleLLVMOptions.cmake
@@ -559,6 +559,8 @@
 
   # Enable -Wstring-conversion to catch misuse of string literals.
   add_flag_if_supported("-Wstring-conversion" STRING_CONVERSION_FLAG)
+else()
+  append("-w" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
 endif (LLVM_ENABLE_WARNINGS AND (LLVM_COMPILER_IS_GCC_COMPATIBLE OR CLANG_CL))
 
 macro(append_common_sanitizer_flags)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits