xmake-io / xmake

🔥 A cross-platform build utility based on Lua
https://xmake.io
Apache License 2.0
10.14k stars 782 forks source link

set_warnings 隔离 nvcc 和 cxx #5380

Closed TOMO-CAT closed 3 weeks ago

TOMO-CAT commented 3 months ago

你在什么场景下需要该功能?

目前 set_warnings 会对所有 nvcc 和 clang 生效,假设我设置:

set_warnings("all", "extra", "error")

nvcc 编译会直接带上 -Werror cross-execution-space-call,reorder,deprecated-declarations ,按照 nvcc 官网的描述和 clang 区别不大:

https://docs.nvidia.com/cuda/cuda-compiler-driver-nvcc/#command-option-description image

问题在于 clang 可以通过 -Wno 关闭一些 werror,但是 nvcc 即使带上 supress 也无法屏蔽 error,所以 set_warnings 对我们而言杀伤力太大,希望可以分离: image image image

描述可能的解决方案

目前 add_cxxflags 和 add_cuflags 分别控制 warning,但是和 set_warnings 可能存在参数顺序的问题导致不生效

描述你认为的候选方案

No response

其他信息

No response

Issues-translate-bot commented 3 months ago

Bot detected the issue body's language is not English, translate it automatically.


Title: set_warnings isolate nvcc and cxx

waruqi commented 3 months ago

set_warnings 的目的就是抽象化,消除编译器之间的配置差异,所以区分不了。

目前 add_cxxflags 和 add_cuflags 分别控制 warning,但是和 set_warnings 可能存在参数顺序的问题导致不生效

想要区分,就用 add_cxxflags 单独设置 c++ warnings,完全不使用 set_warnings 。

或者自己研究下代码里压制,clang/gcc 都是有类似压制方式的,nvcc 本身也是调用的 gcc/clang/msvc ccbin ,按理应该也是可以的。

#pragma clang diagnostic push
#pragma clang diagnostic ignored "-wpadded" 
// warnings
#pragma clang diagnostic pop

问题在于 clang 可以通过 -Wno 关闭一些 werror,但是 nvcc 即使带上 supress 也无法屏蔽 error,所以 set_warnings 对我们而言杀伤力太大,希望可以分离:

nvcc 本身也是调用的clang/gcc,既然 clang 可以,nvcc 应该也是可以的,但是要额外传递 -Xcompiler flag ,才能透传给 clang

add_cuflags("-Xcompiler flag")
TOMO-CAT commented 3 months ago

想要区分,就用 add_cxxflags 单独设置 c++ warnings,完全不使用 set_warnings 。

我们是在公司内部的 xmake-repo 下发自己的 mode.release,要求所有人 -Werror,然后自己仓库有特殊需求的自己再加,rule 里面的是 target:on_config 会将 -Werror 加到用户自己定义 cxxflags 后面,导致用户自定义参数失效。所以这个方法行不通。我们也不可能让所有仓库用相同的 cxxflags。

或者自己研究下代码里压制,clang/gcc 都是有类似压制方式的,nvcc 本身也是调用的 gcc/clang/msvc ccbin ,按理应该也是可以的。

这个肯定是每个 repo 的人自己加,他们解决不了 warning 再特异性增加对应的 supress,这个没有问题,但是不影响我全局设置 -Werror。

nvcc 本身也是调用的clang/gcc,既然 clang 可以,nvcc 应该也是可以的,但是要额外传递 -Xcompiler flag ,才能透传给 clang

-Xcompiler flag 只能控制 nvcc 编译 cpp 相关的代码,但是 cu 里面的 warning 关不掉,这是 nvcc 的文档: image

Issues-translate-bot commented 3 months ago

Bot detected the issue body's language is not English, translate it automatically.


The purpose of set_warnings is to abstract and eliminate configuration differences between compilers, so they cannot be distinguished.

Currently add_cxxflags and add_cuflags respectively control warnings, but there may be a parameter order problem with set_warnings, causing it to not take effect.

If you want to distinguish, use add_cxxflags to set c++ warnings separately, and do not use set_warnings at all.

waruqi commented 3 months ago

我们是在公司内部的 xmake-repo 下发自己的 mode.release,要求所有人 -Werror,然后自己仓库有特殊需求的自己再加,rule 里面的是 target:on_config 会将 -Werror 加到用户自己定义 cxxflags 后面,导致用户自定义参数失效。所以这个方法行不通。我们也不可能让所有仓库用相同的 cxxflags。

既然 add_cxxflags("-Werror") 顺序靠后,为啥 set_warnings("error") 就行?

还是那句话,set_warnings 没法分开,只能自己用 add_cxxflags, add_cuflags 自己分。。顺序问题,自己 target:get()/target:set() + table.insert 自己处理

Issues-translate-bot commented 3 months ago

Bot detected the issue body's language is not English, translate it automatically.


We issue our own mode.release in the company's internal xmake-repo, requiring everyone to -Werror, and then add it if you have special needs in your own warehouse. The rule inside is target:on_config, which will add -Werror to the user himself. After defining cxxflags, user-defined parameters will become invalid. So this method doesn't work. It is also impossible for all warehouses to use the same cxxflags.

Since add_cxxflags("-Werror") is later in the order, why set_warnings("error") is OK?

Again, set_warnings cannot be separated, you can only use add_cxxflags, add_cuflags to separate them yourself. . Sequence issues, handle it yourself target:get()/target:set() + table.insert

TOMO-CAT commented 3 months ago

既然 add_cxxflags("-Werror") 顺序靠后,为啥 set_warnings("error") 就行?

因为set_warnings 在 xmake 里有特殊的处理逻辑,会放到编译参数前面。

还有另一个问题:为什么 set_warnings("all", "extra", "error") 后 nvcc 是只开了 -Werror cross-execution-space-call,reorder,deprecated-declarations 这和预期也不符合呀?官方支持 all warnings be treated as error 的: image

TOMO-CAT commented 3 months ago

还是那句话,set_warnings 没法分开,只能自己用 add_cxxflags, add_cuflags 自己分。。顺序问题,自己 target:get()/target:set() + table.insert 自己处理

这里面还有 add_cxxflags 和 add_cxflags 的问题,处理起来也相当麻烦。很难要求公司所有 repo 都用一样的写法。

Issues-translate-bot commented 3 months ago

Bot detected the issue body's language is not English, translate it automatically.


Again, set_warnings cannot be separated, you can only use add_cxxflags, add_cuflags to separate them yourself. . Sequence issues, handle it yourself target:get()/target:set() + table.insert

There are also issues with add_cxxflags and add_cxflags, which are quite troublesome to deal with. It is difficult to require all repos of the company to be written in the same way.

waruqi commented 3 months ago

还有另一个问题:为什么 set_warnings("all", "extra", "error") 后 nvcc 是只开了 -Werror cross-execution-space-call,reorder,deprecated-declarations 这和预期也不符合呀?官方支持 all warnings be treated as error 的:

这个得问下 @OpportunityLiu 也不是我这加的。https://github.com/xmake-io/xmake/commit/1671667d3121d9876398581e771cb3182e8b3683

waruqi commented 3 months ago

还是那句话,set_warnings 没法分开,只能自己用 add_cxxflags, add_cuflags 自己分。。顺序问题,自己 target:get()/target:set() + table.insert 自己处理

这里面还有 add_cxxflags 和 add_cxflags 的问题,处理起来也相当麻烦。很难要求公司所有 repo 都用一样的写法。

搞个统一的 rule 么,就跟 set_warnings("error") 一样,走 target:add("cxflags", "-Werror") + table.insert 插入到 flags 最开头,后面每个 -Wno-xxx repo 爱怎么加其他 flags 就自己加啥。。``

Issues-translate-bot commented 3 months ago

Bot detected the issue body's language is not English, translate it automatically.


Again, set_warnings cannot be separated, you can only use add_cxxflags, add_cuflags to separate them yourself. . Sequence issues, handle it yourself target:get()/target:set() + table.insert

There are also issues with add_cxxflags and add_cxflags, which are quite troublesome to deal with. It is difficult to require all repos of the company to be written in the same way.

To make a unified rule, just like set_warnings("error"), use target:add("cxflags", "-Werror") + table.insert to insert it at the beginning of flags, and each - Wno-xxx repo Add other flags as you like. . ``

TOMO-CAT commented 3 months ago

搞个统一的 rule 么,就跟 set_warnings("error") 一样,走 target:add("cxflags", "-Werror") + table.insert 插入到 flags 最开头,后面每个 -Wno-xxx repo 爱怎么加其他 flags 就自己加啥。。``

这确实是我的备选方案,就是不太优雅,所以问问看有没有更好的思路。

Issues-translate-bot commented 3 months ago

Bot detected the issue body's language is not English, translate it automatically.


Make a unified rule, just like set_warnings("error"), use target:add("cxflags", "-Werror") + table.insert to insert it at the beginning of flags, and eachafter -Wno-xxx repo Add other flags as you like. . ``

This is indeed my alternative, but it's not very elegant, so I'm asking if there are any better ideas.

waruqi commented 3 months ago

搞个统一的 rule 么,就跟 set_warnings("error") 一样,走 target:add("cxflags", "-Werror") + table.insert 插入到 flags 最开头,后面每个 -Wno-xxx repo 爱怎么加其他 flags 就自己加啥。。``

这确实是我的备选方案,就是不太优雅,所以问问看有没有更好的思路。

一种就是调整 error -> -Werror cross-execution-space-call,reorder,deprecated-declarations 的预设值,让它更符合预期 可以跟 @OpportunityLiu 确认下

一种就只能自己调整 flags 顺序

OpportunityLiu commented 3 weeks ago

我写这个功能的时候 nvcc 应该是只有那些选项

https://docs.nvidia.com/cuda/archive/9.2/cuda-compiler-driver-nvcc/index.html

Issues-translate-bot commented 3 weeks ago

Bot detected the issue body's language is not English, translate it automatically.


When I wrote this function, nvcc should have only those options

https://docs.nvidia.com/cuda/archive/9.2/cuda-compiler-driver-nvcc/index.html