赛题四：在飞桨框架中引入 clang-tidy Tracking Issue

GreatV commented 1 year ago

一、项目说明

用于跟踪 clang-tidy代码风格检查工具的引入并记录该计划下各个子任务的计划和进度。

二、主要工作

📓 操作指南

（以readability-container-size-empty为例）

编译Paddle （PR-CI-Coverage中的编译选项），需要打开-DCMAKE_EXPORT_COMPILE_COMMANDS=ON

cmake .. -DCMAKE_BUILD_TYPE=Release -DWITH_GPU=ON -DWITH_CUDNN_DSO=OFF -DWITH_SHARED_PHI=ON \
-DWITH_TENSORRT=ON -DWITH_ROCM=OFF -DWITH_CINN=OFF -DWITH_DISTRIBUTE=ON \
-DWITH_MKL=ON -DWITH_AVX=ON -DCUDA_ARCH_NAME=Volta -DNEW_RELEASE_PYPI=OFF \
-DNEW_RELEASE_ALL=OFF -DNEW_RELEASE_JIT=OFF -DWITH_PYTHON=ON \
-DCUDNN_ROOT=/usr/ -DWITH_TESTING=ON -DWITH_COVERAGE=ON \
-DWITH_INCREMENTAL_COVERAGE=OFF -DCMAKE_MODULE_PATH=/opt/rocm/hip/cmake \
-DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DWITH_INFERENCE_API_TEST=ON \
-DINFERENCE_DEMO_INSTALL_DIR=/home/data/cfs/.cache/coverage -DPY_VERSION=3.8 \
-DCMAKE_INSTALL_PREFIX=/paddle/build -DWITH_PSCORE=ON -DWITH_PSLIB=OFF \
-DWITH_GLOO=ON -DLITE_GIT_TAG=release/v2.10 -DWITH_XPU=OFF -DWITH_IPU=OFF \
-DXPU_SDK_ROOT= -DWITH_LITE=ON -DWITH_XPU_BKCL=OFF -DWITH_ARM=OFF \
-DWITH_STRIP=ON -DON_INFER=OFF -DWITH_HETERPS=OFF -DWITH_GPU_GRAPH=OFF \
-DCUDA_ARCH_BIN= -DWITH_RECORD_BUILDTIME=OFF -DWITH_UNITY_BUILD=ON \
-DWITH_ONNXRUNTIME=OFF -DWITH_CUDNN_FRONTEND=OFF

# 不执行make，会导致clang-tidy无法找到第三方库
make -j$(nproc)

打开检查在.clang-tidy中移除-readability-container-size-empty的前缀-
运行clang-tidy，并根据clang-tidy提示修改代码

重新运行pre-commit确认修改完成

# paddle on develop [!+?] via △ v3.18.0 via 🐍 v3.7.16 took 27m16s
pre-commit run clang-tidy --all-files > clang-tidy.log 2>&1
grep "\[readability-container-size-empty,-warnings-as-errors\]" clang-tidy.log

也可以直接运行clang-tidy并行检查工具：

python ./tools/codestyle/clang-tidy.py -p=build -j=10 \
-clang-tidy-binary=clang-tidy \
-extra-arg=-Wno-unknown-warning-option \
-extra-arg=-Wno-pessimizing-move \
-extra-arg=-Wno-braced-scalar-init \
-extra-arg=-Wno-deprecated-copy \
-extra-arg=-Wno-dangling-gsl \
-extra-arg=-Wno-final-dtor-non-final-class \
-extra-arg=-Wno-implicit-int-float-conversion \
-extra-arg=-Wno-inconsistent-missing-override \
-extra-arg=-Wno-infinite-recursion \
-extra-arg=-Wno-mismatched-tags  \
-extra-arg=-Wno-self-assign \
-extra-arg=-Wno-sign-compare \
-extra-arg=-Wno-sometimes-uninitialized \
-extra-arg=-Wno-tautological-overlap-compare \
-extra-arg=-Wno-unused-const-variable \
-extra-arg=-Wno-unused-lambda-capture \
-extra-arg=-Wno-unused-private-field \
-extra-arg=-Wno-unused-value \
-extra-arg=-Wno-unused-variable  \
-extra-arg=-Wno-overloaded-virtual  \
-extra-arg=-Wno-defaulted-function-deleted  \
-extra-arg=-Wno-delete-non-abstract-non-virtual-dtor  \
-extra-arg=-Wno-return-type-c-linkage  > clang-tidy.log  2>&1

整体进展（39/94）

按完成时间顺序，排名不分先后: @GreatV (2) @ccsuzzh (1) @enkilee (36)

1. 存量修复相关

注：错误数量没有统计 CINN 目录下，CINN 目录下也暂时无需修复，原因见 https://github.com/PaddlePaddle/Paddle/issues/54073#issuecomment-1641368364

注：2023/07/21重新扫描的日志文件clang-tidy.log

注：2023/11/15重新扫描的日志文件clang-tidy.log

编号	错误类型	错误数量	认领人	PR链接
1	cppcoreguidelines-init-variables	11002
2	modernize-redundant-void-arg	1294
3	bugprone-narrowing-conversions,cppcoreguidelines-narrowing-conversions	327	@enkilee	#61759 ✅ #62109 ✅
4	cppcoreguidelines-pro-type-member-init	216
5✅ (2024/3/1)	cppcoreguidelines-avoid-c-arrays,modernize-avoid-c-arrays	151	@enkilee	#61751
6	modernize-unary-static-assert	132
7✅ (2024/3/1)	bugprone-branch-clone	132	@enkilee	#61735
8	performance-unnecessary-copy-initialization	127
9	cppcoreguidelines-avoid-goto,hicpp-avoid-goto	109
10	cppcoreguidelines-pro-type-const-cast	90
11	modernize-pass-by-value	65
12✅ (2024/3/7)	modernize-loop-convert	46	@enkilee	#61725
13✅ (2024/2/21)	modernize-deprecated-headers	44	@enkilee	#61721
14✅ (2024/2/20)	misc-unused-alias-decls	44	@enkilee	#61716
15✅ (2024/2/20)	performance-inefficient-vector-operation	30	@enkilee	#61715
16	clang-analyzer-optin.cplusplus.VirtualCall	26
17✅ (2024/3/1)	cppcoreguidelines-explicit-virtual-functions,modernize-use-override	20	@enkilee	#61714
18	clang-analyzer-core.NullDereference	18
19✅ (2024/2/26)	readability-container-size-empty	16	@enkilee	#61713
20✅ (2023/12/5)	modernize-use-nullptr	15	@ccsuzzh	#59626
21✅ (2024/2/21)	performance-for-range-copy	14	@enkilee	#61712
22	cppcoreguidelines-no-malloc	13
23	modernize-use-emplace	11
24✅ (2024/3/1)	hicpp-exception-baseclass	11	@enkilee	#61691
25✅ (2024/2/26)	modernize-use-transparent-functors	9	@enkilee	#61689
26✅ (2024/2/20)	misc-unused-using-decls	9	@enkilee	#61616
27✅ (2024/2/21)	performance-move-const-arg	7	@enkilee	#61615
28✅ (2024/2/21)	modernize-use-equals-default	7	@enkilee	#61614
29✅ (2024/2/20)	bugprone-exception-escape	7	@enkilee	#61609
30	performance-inefficient-string-concatenation	5
31✅ (2024/2/29)	clang-analyzer-cplusplus.NewDeleteLeaks	5	@enkilee	#62129
32✅ (2024/2/29)	bugprone-unused-raii	5	@enkilee	#62129
33✅ (2024/2/20)	bugprone-inaccurate-erase	5	@enkilee	#61589
34✅ (2024/2/29)	bugprone-copy-constructor-init	5	@enkilee	#62129
35✅ (2024/2/20)	modernize-use-bool-literals	3	@enkilee	#61580
36✅ (2024/2/20)	clang-analyzer-core.DivideZero	3	@enkilee	#61580
37✅ (2024/2/20)	bugprone-integer-division	3	@enkilee	#61580
38✅ (2024/2/26)	performance-trivially-destructible	2	@enkilee	#61556
39✅ (2024/2/26)	modernize-make-unique	2	@enkilee	#61556
40✅ (2024/2/26)	modernize-avoid-bind	2	@enkilee	#61556
41✅ (2024/2/29)	cppcoreguidelines-slicing	2	@enkilee	#62129
42✅ (2024/2/20)	performance-noexcept-move-constructor	1	@enkilee	#61555
43✅ (2024/2/20)	clang-diagnostic-unused-but-set-variable	1	@enkilee	#61555
44✅ (2024/2/20)	clang-analyzer-security.FloatLoopCounter	1	@enkilee	#61555
45✅ (2024/2/29)	clang-analyzer-cplusplus.NewDelete	1	@enkilee	#62129
46✅ (2024/2/21)	clang-analyzer-core.NonNullParamChecker	1	@enkilee	#61494
47✅ (2024/2/21)	bugprone-unhandled-self-assignment	1	@enkilee	#61494
48✅ (2024/2/20)	bugprone-string-integer-assignment	1	@enkilee	#61492
49✅ (2024/2/20)	bugprone-misplaced-widening-cast	1	@enkilee	#61492
50✅ (2024/2/20)	bugprone-infinite-loop	1	@enkilee	#61492

旧任务表格如下所示（旧任务进展 55/94）：按完成时间顺序，排名不分先后： @GreatV (4) @BeingGod (4) @gouzil (19) @ccsuzzh (5) @enkilee (13) @xiaoyewww (6) @SigureMo (3) @RedContritio (1)

编号 | 错误类型 | 错误数量 | 认领人 | PR链接 :------ | :------ | :------ | :------ | :------ 1 | [hicpp-avoid-goto] | 1603 2 | [cppcoreguidelines-avoid-goto] (🏷️ alias of task 1) | 1603 3✅ (2023/7/31) | [modernize-redundant-void-arg] | 1326 | @gouzil | #55652 #55741 4✅ (2023/8/3) | [modernize-loop-convert] | 1091 | @GreatV | #55704 5✅ (2023/8/30) | [modernize-make-shared] | 842 | @SigureMo | #55807 6✅ (2023/8/14) | [modernize-avoid-c-arrays] | 820 | @gouzil | #55774✅ #55954✅ 7✅ (2023/8/14) | [cppcoreguidelines-avoid-c-arrays] (🏷️ alias of task 6) | 820 | @gouzil | #55774✅ #56208✅ 8✅ (2023/9/15) | [cppcoreguidelines-narrowing-conversions] | 596 | @gouzil | #56218 ✅ #56895✅ #56904✅ 57114✅ 9✅ (2023/10/11) | [bugprone-narrowing-conversions] (🏷️ alias of task 8) | 596 | @gouzil | #57861 10✅ (2023/10/11) | [cppcoreguidelines-init-variables] | 576| @gouzil | #57871 11 ✅ (2023/7/11) | [readability-container-size-empty] | 458 | @GreatV | #55279 12✅ (2023/8/4) | [modernize-use-nullptr] | 409 | @ccsuzzh | #55800 13✅ (2023/8/4) | [modernize-use-emplace] | 401 | @BeingGod | #55799 14 | [performance-unnecessary-copy-initialization] | 395 15 | [cppcoreguidelines-pro-type-const-cast] | 164 | @enkilee | #58285 16✅ (2023/8/7) | [modernize-use-equals-default] | 150 | @BeingGod | #55983 17✅ (2023/10/12) | [performance-for-range-copy] | 127 | @enkilee | #57809 18 | [cppcoreguidelines-pro-type-member-init] | 127 19✅ (2023/10/13) | [performance-move-const-arg] | 113 | @enkilee | #57627 20✅ (2023/9/7) | [clang-analyzer-core.CallAndMessage] | 96 | @enkilee | #56954 21 | [modernize-pass-by-value] | 95 | @Rruown 22✅ (2023/8/9) | [modernize-make-unique] | 81 | @BeingGod | #55506✅ #55764✅ 23✅ (2023/9/21) | [bugprone-branch-clone] | 74 | @enkilee | #57522 24 | [performance-inefficient-vector-operation] | 71 | @gouzil | #58028 ✅ 25✅ (2023/9/7) | [modernize-deprecated-headers] | 34 | @enkilee | #56994 26✅ (2023/8/29) | [misc-unused-using-decls] | 33 | @enkilee | #56485 27✅ (2023/8/29) | [misc-unused-alias-decls] | 32 | @enkilee | #56485 28 | [clang-analyzer-core.NullDereference] | 32 29✅ (2023/9/18) | [cppcoreguidelines-no-malloc] | 29 | @gouzil | #57343 30 | [clang-analyzer-optin.cplusplus.VirtualCall] | 26 31✅ (2023/8/14) | [modernize-use-bool-literals] | 22 | @enkilee | #56216 32✅ (2023/9/20) | [clang-analyzer-deadcode.DeadStores] | 22 | @enkilee | #57483 33✅ (2023/9/7) | [bugprone-signed-char-misuse] | 21 | @ccsuzzh | #56744 34✅ (2023/9/1) | [performance-noexcept-move-constructor] | 18 | @enkilee | #56261 35✅ (2023/9/1) | [bugprone-incorrect-roundings] | 18| @gouzil | #56747 36✅ (2023/9/1) | [modernize-use-transparent-functors] | 17| @enkilee | #56261 37✅ (2023/8/15) | [performance-faster-string-find] | 14 | @enkilee | #56255 38✅ (2023/8/28) | [bugprone-exception-escape] | 14| @gouzil | #56692 39✅ (2023/8/30) | [clang-analyzer-optin.cplusplus.UninitializedObject] | 13| @gouzil | #56648 40✅ (2023/8/29) | [performance-inefficient-string-concatenation] | 12| @gouzil | #56647 41✅ (2023/8/29) | [bugprone-unhandled-self-assignment] | 9| @gouzil | #56640 42✅ (2023/8/29) | [clang-analyzer-core.uninitialized.Assign] | 8| @gouzil | #56637 43✅ (2023/8/29) | [clang-analyzer-core.UndefinedBinaryOperatorResult] | 8| @gouzil | #56636 44✅ (2023/8/29) | [bugprone-misplaced-widening-cast] | 8| @gouzil | #56635 45✅ (2023/8/1) | [bugprone-unused-raii] | 7 | @ccsuzzh | #55815 46✅ (2023/8/3) | [modernize-avoid-bind] | 6 | @gouzil 47 ✅ (2023/7/21) | [modernize-use-override] | 4 | @BeingGod | #55491 48✅(2023/11/6) | [clang-analyzer-unix.Malloc] | 4 | @RedContritio| #55658 49 | [clang-analyzer-cplusplus.NewDeleteLeaks] | 4 | 50 | [clang-analyzer-core.DivideZero] | 4 | 51✅ (2023/8/14) | [bugprone-copy-constructor-init] | 4 | @enkilee | #56219 52✅ (2023/8/14) | [bugprone-argument-comment] | 4 | @enkilee | #56217 53✅ (2023/8/29) | [cppcoreguidelines-c-copy-assignment-signature] | 3 | @xiaoyewww | #56601 54✅ (2023/8/29) | [bugprone-use-after-move] | 3| @xiaoyewww | #56601 55 | [bugprone-sizeof-expression] | 3| @xiaoyewww 56✅ (2023/8/29) | [modernize-raw-string-literal] | 2 | @SigureMo | #55675 | 57✅ (2023/9/5) | [cppcoreguidelines-explicit-virtual-functions] | 2| @xiaoyewww | #56649 58✅ (2023/9/5) | [clang-analyzer-core.NonNullParamChecker] | 2| @xiaoyewww | #56649 59✅ (2023/9/5) | [bugprone-unused-return-value] | 2| @xiaoyewww | #56676 60✅ (2023/9/5) | [bugprone-suspicious-missing-comma] | 2| @xiaoyewww | #56676 61✅ (2023/8/29) | [hicpp-exception-baseclass] | 1 | @SigureMo | #55847 | 62✅ (2023/8/15) | [clang-analyzer-unix.Vfork] | 1 | @gouzil | #56253 63✅ (2023/8/15) | [clang-analyzer-security.insecureAPI.vfork] | 1 | @gouzil | #56253 64✅ (2023/9/7) | [clang-analyzer-optin.portability.UnixAPI] | 1| @ccsuzzh | #56744 65✅ (2023/8/28) | [clang-analyzer-cplusplus.InnerPointer] | 1 | @ccsuzzh | #56693 66✅ (2023/8/14) | [bugprone-integer-division] | 1 | @gouzil | #56246

2. 增量拦截相关

编号	任务	认领人	PR链接
67✅ (2023/7/11)	在 `.pre-commit-config.yaml` 在添加 `clang-tidy` 检查项	@GreatV	#55279 #55894
68✅(2023/7/11)	使用单独的脚本运行 `clang-tidy`，开发者便于手动执行检查	运行`tools/codestyle/clang-tidy.py`即可	#55279
69	将 `clang-tidy` 工具同步到release分支	@Liyulingyue
70	将 `clang-tidy` 同步到CI镜像中，`PR-CI-Build`中打开CI, 避免重复安装下载	@ccsuzzh	#55619

3. 移除pre-commit中的`-WnoXXX` （.pre-commit-config.yaml）

编号	任务	认领人
71	-extra-arg=-Wno-pessimizing-move	@enkilee
72	-extra-arg=-Wno-braced-scalar-init
73	-extra-arg=-Wno-dangling-gsl
74	-extra-arg=-Wno-deprecated-copy
75	-extra-arg=-Wno-final-dtor-non-final-class
76	-extra-arg=-Wno-implicit-int-float-conversion
77	-extra-arg=-Wno-inconsistent-missing-override
78	-extra-arg=-Wno-infinite-recursion
79	-extra-arg=-Wno-mismatched-tags
80	-extra-arg=-Wno-self-assign
81	-extra-arg=-Wno-sign-compare
82	-extra-arg=-Wno-sometimes-uninitialized
83	-extra-arg=-Wno-tautological-overlap-compare
84	-extra-arg=-Wno-unused-const-variable
85	-extra-arg=-Wno-unused-lambda-capture
86	-extra-arg=-Wno-unused-private-field
87	-extra-arg=-Wno-unused-value
88	-extra-arg=-Wno-unused-variable
89	-extra-arg=-Wno-overloaded-virtual
90	-extra-arg=-Wno-defaulted-function-deleted
91	-extra-arg=-Wno-delete-non-abstract-non-virtual-dtor
92	-extra-arg=-Wno-return-type-c-linkage
93	将`paddle/cinn` 重新加入 clang-tidy pre-commit 扫描路径，并确保打开的检查项能够通过
94	将cmake时自动生成的c++文件路径加入clang-tidy排除项

RedContritio commented 1 year ago

认领 1. 存量修复相关 32 - 42

BeingGod commented 1 year ago

认领26

luotao1 commented 1 year ago

建议先把非CINN目录的代码格式修复完+CI监控上，再考虑CINN目录。原因如下： @GreatV @zhangbo9674

clang-tidy的precommit需要在make后执行，即CI上需要在build流水线或Coverage流水线中新增clang-tidy的precommit，不能在CodeStyle流水线中加。
build和coverage流水线都没有打开WITH_CINN=ON，CINN的触发规则如下。

luotao1 commented 1 year ago

@GreatV @ccsuzzh :

和 @zhangbo9674 @risemeup1 讨论后：可以先把CI加在build流水线后面，PR中需要附上测试时间（比如修改10个、50个、100个等文件的precommit clang-tidy CI时间）
最近在做流水线的模块和梳理过程，后续再针对性的统一调整

GreatV commented 1 year ago

@luotao1 好的，收到。

gouzil commented 1 year ago

认领6

GreatV commented 1 year ago

@zrr1999 @RedContritio @gouzil 任务表格更新了一下，有些任务已经没有了，麻烦重新领取一下任务。

旧任务表格如下所示：

编号|错误类型|错误数量|认领人|PR链接 |:------|:------:|:------|:------|:------ 1|~~[clang-diagnostic-error]~~|~~4134~~| 2|[cppcoreguidelines-pro-bounds-pointer-arithmetic]|4415 3|[cppcoreguidelines-pro-type-cstyle-cast]|4337| @RedContritio | 4|[cppcoreguidelines-pro-type-reinterpret-cast]|2279 | @gouzil | 5|[cppcoreguidelines-pro-type-vararg]|1882 6|[modernize-redundant-void-arg]|1280| @gouzil | 7|[cppcoreguidelines-pro-bounds-array-to-pointer-decay]|1054 8✅ (2023/7/11)|[readability-container-size-empty]|766 | @GreatV | #55279 9|[modernize-loop-convert]|543| @GreatV | 10|[modernize-use-nullptr]|436 | @ccsuzzh | 11|[cppcoreguidelines-pro-bounds-constant-array-index]|363 12|[cppcoreguidelines-pro-type-static-cast-downcast]|315 13|[cppcoreguidelines-pro-type-const-cast]|249 14|[modernize-use-auto]|171 | @zrr1999 | 15|[modernize-pass-by-value]|132 | @Rruown 16✅ (2023/7/21)|[modernize-use-override]|125| @BeingGod |#55491 17|[cppcoreguidelines-pro-type-union-access]|120 18|~~[clang-diagnostic-pessimizing-move]~~|70 | @enkilee |对应54| 19|[misc-unused-alias-decls]|52 20|[clang-analyzer-core.StackAddressEscape]|35 21|[modernize-use-default]|33 22|[clang-analyzer-core.CallAndMessage]|32 | @enkilee | 23|[clang-diagnostic-missing-braces]|22 24|[clang-analyzer-core.NullDereference]|19 25|[performance-unnecessary-copy-initialization]|16 26✅(2023/7/21)|[modernize-make-unique]|15| @BeingGod | #55506 27|[clang-diagnostic-unused-value]|9 28|[clang-diagnostic-sign-compare]|8 29|[clang-analyzer-core.UndefinedBinaryOperatorResult]|7 30|[clang-analyzer-deadcode.DeadStores]|6 31|[cppcoreguidelines-c-copy-assignment-signature]|4 32|[clang-analyzer-unix.Malloc]|3| @RedContritio | 33|[clang-diagnostic-overloaded-virtual]|2 | @RedContritio | 34|[clang-diagnostic-mismatched-tags]|2 | @RedContritio | 35|[clang-diagnostic-inconsistent-missing-override]|2 | @RedContritio | 36|[clang-diagnostic-compare-distinct-pointer-types]|2 | @RedContritio | 37|[clang-diagnostic-braced-scalar-init]|2| @RedContritio | 38|[clang-analyzer-cplusplus.NewDelete]|2| @RedContritio | 39|[clang-analyzer-core.DivideZero]|2| @RedContritio | 40|[clang-diagnostic-unused-variable]|1| @RedContritio | 41|[clang-diagnostic-unused-private-field]|1| @RedContritio | 42|[clang-diagnostic-unused-const-variable]|1| @RedContritio | 43|[clang-diagnostic-self-assign]|1 44|[clang-diagnostic-return-type-c-linkage]|1 45|[clang-analyzer-unix.Vfork]|1 46|[clang-analyzer-security.insecureAPI.vfork]|1 47|[clang-analyzer-cplusplus.NewDeleteLeaks]|1 48|[clang-analyzer-core.uninitialized.Assign]|1 49|[clang-analyzer-core.NonNullParamChecker]|1

GreatV commented 1 year ago

@BeingGod 我重新扫描了一下，发现 modernize-make-unique 还有一些没有处理的，麻烦再处理一下。

BeingGod commented 1 year ago

@GreatV 好的

BeingGod commented 1 year ago

认领 13,16

gouzil commented 1 year ago

认领46

gouzil commented 1 year ago

认领8

xiaoyewww commented 1 year ago

认领 53-55，57-60

gouzil commented 1 year ago

认领38-44

xiaoyewww commented 1 year ago

认领 53-55，57-60

@GreatV @luotao1 编号55这个相关修改在pb.cc的两个生成文件中，是不是不用修改了？

gouzil commented 1 year ago

认领 53-55，57-60

@GreatV @luotao1 编号55这个相关修改在pb.cc的两个生成文件中，是不是不用修改了？

生成的文件不修改, 直接放开检查就行

PaddlePaddle / Paddle