Merge pull request #94 from Rust-for-Linux/rust-clippy

Clippy support

Author: ojeda

.github/workflows/ci.yaml

@@ -131,6 +131,9 @@ jobs:
           git checkout $(rustc -vV | grep -F 'commit-hash' | awk '{print $2}')
           git submodule update --init library
 
+      # Setup: clippy
+      - run: rustup component add clippy
+
       # Setup: bindgen
       - run: cargo install --version 0.56.0 bindgen
 
@@ -178,6 +181,9 @@ jobs:
       - run: ls -l ${{ env.BUILD_DIR }}drivers/char/rust_example.o ${{ env.BUILD_DIR }}drivers/char/rust_example_3.ko ${{ env.BUILD_DIR }}rust/*.o ${{ env.BUILD_DIR }}vmlinux ${{ env.BUILD_DIR }}${{ env.IMAGE_PATH }}
       - run: size  ${{ env.BUILD_DIR }}drivers/char/rust_example.o ${{ env.BUILD_DIR }}drivers/char/rust_example_3.ko ${{ env.BUILD_DIR }}rust/*.o ${{ env.BUILD_DIR }}vmlinux
 
+      # Clippy
+      - run: make ${{ env.MAKE_ARCH }} ${{ env.MAKE_CROSS_COMPILE }} ${{ env.MAKE_TOOLCHAIN }} ${{ env.MAKE_OUTPUT }} ${{ env.MAKE_SYSROOT }} -j3 CLIPPY=1
+
       # Docs
       - run: make ${{ env.MAKE_ARCH }} ${{ env.MAKE_CROSS_COMPILE }} ${{ env.MAKE_TOOLCHAIN }} ${{ env.MAKE_OUTPUT }} ${{ env.MAKE_SYSROOT }} -j3 rustdoc
 

Documentation/rust/coding.rst

@@ -42,6 +42,23 @@ individual files, and does not require a kernel configuration. Sometimes it may
 even work with broken code.
 
 
+Extra lints
+-----------
+
+While ``rustc`` is a very helpful compiler, some extra lints and analysis are
+available via ``clippy``, a Rust linter. To enable it, pass ``CLIPPY=1`` to
+the same invocation you use for compilation, e.g.::
+
+	make ARCH=... CROSS_COMPILE=... CC=... -j... CLIPPY=1
+
+At the moment, we do not enforce a "clippy-free" compilation, so you can treat
+the output the same way as the extra warning levels for C, e.g. like ``W=2``.
+Still, we use the default configuration, which is relatively conservative, so
+it is a good idea to read any output it may produce from time to time and fix
+the pointed out issues. The list of enabled lists will be likely tweaked over
+time, and extra levels may end up being introduced, e.g. ``CLIPPY=2``.
+
+
 Abstractions vs. bindings
 -------------------------
 

Documentation/rust/quick-start.rst

@@ -4,13 +4,13 @@ Quick Start
 ===========
 
 This document describes how to get started with kernel development in Rust.
+If you have worked previously with Rust, this will only take a moment.
 
 
-Requirements
-------------
+Requirements: Building
+----------------------
 
-This section explains how to fetch the requirements to work with Rust.
-If you have worked previously with Rust, this will only take a moment.
+This section explains how to fetch the tools needed for building.
 
 Some of these requirements might be available from your Linux distribution
 under names like ``rustc``, ``rust-src``, ``rust-bindgen``, etc. However,
@@ -64,11 +64,19 @@ which means you will need a recent LLVM installed; like when you compile
 the kernel with ``CC=clang`` or ``LLVM=1``.
 
 
+Requirements: Developing
+------------------------
+
+This section explains how to fetch the tools needed for developing. That is,
+if you only want to build the kernel, you do not need them.
+
+
 rustfmt
 *******
 
 The ``rustfmt`` tool is used to automatically format all the Rust kernel code,
-including the generated C bindings.
+including the generated C bindings (for details, please see
+:ref:`Documentation/rust/coding.rst <rust_coding>`).
 
 If you are using ``rustup``, its ``default`` profile already installs the tool,
 so you should be good to go. If you are using another profile, you can install
@@ -79,12 +87,29 @@ the component manually::
 The standalone installers also come with ``rustfmt``.
 
 
+clippy
+******
+
+``clippy`` is a Rust linter. Installing it allows you to get extra warnings
+for Rust code passing ``CLIPPY=1`` to ``make`` (for details, please see
+:ref:`Documentation/rust/coding.rst <rust_coding>`).
+
+If you are using ``rustup``, its ``default`` profile already installs the tool,
+so you should be good to go. If you are using another profile, you can install
+the component manually::
+
+    rustup component add clippy
+
+The standalone installers also come with ``clippy``.
+
+
 rustdoc
 *******
 
-Optionally, if you install the ``rustdoc`` tool, then you will be able
-to generate HTML documentation for Rust code, including for the libraries
-(crates) inside ``rust/`` that are used by the rest of the kernel.
+If you install the ``rustdoc`` tool, then you will be able to generate pretty
+HTML documentation for Rust code, including for the libraries (crates) inside
+``rust/`` that are used by the rest of the kernel (for details, please see
+:ref:`Documentation/rust/docs.rst <rust_docs>`).
 
 If you are using ``rustup``, its ``default`` profile already installs the tool,
 so you should be good to go. If you are using another profile, you can install

Makefile

@@ -212,6 +212,13 @@ ifndef KBUILD_CHECKSRC
   KBUILD_CHECKSRC = 0
 endif
 
+# Enable "clippy" (a linter) as part of the Rust compilation.
+#
+# Use 'make CLIPPY=1' to enable it.
+ifeq ("$(origin CLIPPY)", "command line")
+  KBUILD_CLIPPY := $(CLIPPY)
+endif
+
 # Use make M=dir or set the environment variable KBUILD_EXTMOD to specify the
 # directory of external module to build. Setting M= takes precedence.
 ifeq ("$(origin M)", "command line")
@@ -446,6 +453,7 @@ STRIP		= $(CROSS_COMPILE)strip
 endif
 RUSTC		= rustc
 RUSTFMT		= rustfmt
+CLIPPY_DRIVER	= clippy-driver
 BINDGEN		= bindgen
 PAHOLE		= pahole
 RESOLVE_BTFIDS	= $(objtree)/tools/bpf/resolve_btfids/resolve_btfids
@@ -516,7 +524,15 @@ KBUILD_LDFLAGS_MODULE :=
 KBUILD_LDFLAGS :=
 CLANG_FLAGS :=
 
-export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS CROSS_COMPILE LD CC RUSTC BINDGEN
+ifeq ($(KBUILD_CLIPPY),1)
+	CLIPPY_QUIET_TAG := CLIPPY$(space)
+else
+	CLIPPY_QUIET_TAG :=
+	CLIPPY_DRIVER :=
+endif
+export CLIPPY_QUIET_TAG
+
+export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS CROSS_COMPILE LD CC RUSTC CLIPPY_DRIVER BINDGEN
 export CPP AR NM STRIP OBJCOPY OBJDUMP READELF PAHOLE RESOLVE_BTFIDS LEX YACC AWK INSTALLKERNEL
 export PERL PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX
 export KGZIP KBZIP2 KLZOP LZMA LZ4 XZ ZSTD
@@ -1653,7 +1669,7 @@ help:
 	@echo  '		      kselftest to existing .config.'
 	@echo  ''
 	@echo  'Rust targets:'
-	@echo  '  rustfmt	  - Reformat all the Rust code in the kernel.'
+	@echo  '  rustfmt	  - Reformat all the Rust code in the kernel'
 	@echo  '  rustfmtcheck	  - Checks if all the Rust code in the kernel'
 	@echo  '		    is formatted, printing a diff otherwise.'
 	@echo  '  rustdoc	  - Generate Rust documentation'

rust/Makefile

@@ -8,23 +8,22 @@ extra-$(CONFIG_RUST) += exports_kernel_generated.h
 
 RUSTDOC = rustdoc
 
-rustdoc_flags = $(filter-out --emit=% --out-dir=%, $(rustc_flags))
-
 quiet_cmd_rustdoc = RUSTDOC $<
       cmd_rustdoc = \
 	RUST_BINDINGS_FILE=$(abspath $(objtree)/rust/bindings_generated.rs) \
-	$(RUSTDOC) $(rustdoc_flags) $(rustdoc_target_flags) -L $(objtree)/rust/ \
+	$(RUSTDOC) $(filter-out --emit=%, $(rustc_flags)) \
+		$(rustdoc_target_flags) -L $(objtree)/rust/ \
 		--output $(objtree)/rust/doc --crate-name $(subst rustdoc-,,$@) \
 		-Wmissing-docs @$(objtree)/include/generated/rustc_cfg $<
 
 rustdoc: rustdoc-module rustdoc-kernel
 
-rustdoc-module: rustdoc_target_flags = --crate-type proc-macro \
+rustdoc-module: private rustdoc_target_flags = --crate-type proc-macro \
     --extern proc_macro
 rustdoc-module: $(srctree)/rust/module.rs FORCE
 	$(call if_changed,rustdoc)
 
-rustdoc-kernel: rustdoc_target_flags = --extern alloc \
+rustdoc-kernel: private rustdoc_target_flags = --extern alloc \
     --extern module=$(objtree)/rust/libmodule.so
 rustdoc-kernel: $(srctree)/rust/kernel/lib.rs rustdoc-module \
     $(objtree)/rust/libmodule.so $(objtree)/rust/bindings_generated.rs FORCE
@@ -74,23 +73,23 @@ quiet_cmd_exports = EXPORTS $@
 		| xargs -n1 -Isymbol \
 		echo 'EXPORT_SYMBOL$(exports_target_type)(symbol);' > $@
 
-$(objtree)/rust/exports_core_generated.h: exports_target_type := _RUST
+$(objtree)/rust/exports_core_generated.h: private exports_target_type := _RUST
 $(objtree)/rust/exports_core_generated.h: $(objtree)/rust/core.o FORCE
 	$(call if_changed,exports)
 
-$(objtree)/rust/exports_alloc_generated.h: exports_target_type := _RUST
+$(objtree)/rust/exports_alloc_generated.h: private exports_target_type := _RUST
 $(objtree)/rust/exports_alloc_generated.h: $(objtree)/rust/alloc.o FORCE
 	$(call if_changed,exports)
 
-$(objtree)/rust/exports_kernel_generated.h: exports_target_type := _RUST_GPL
+$(objtree)/rust/exports_kernel_generated.h: private exports_target_type := _RUST_GPL
 $(objtree)/rust/exports_kernel_generated.h: $(objtree)/rust/kernel.o FORCE
 	$(call if_changed,exports)
 
 # `-Cpanic=unwind -Cforce-unwind-tables=y` overrides `rustc_flags` in order to
 # avoid the https://github.com/rust-lang/rust/issues/82320 rustc crash.
-quiet_cmd_rustc_procmacro = RUSTC P $@
+quiet_cmd_rustc_procmacro = RUSTC P $(CLIPPY_QUIET_TAG)$@
       cmd_rustc_procmacro = \
-	$(RUSTC) $(rustc_flags) --emit=dep-info,link --extern proc_macro \
+	$(CLIPPY_DRIVER) $(RUSTC) $(rustc_flags) --emit=dep-info,link --extern proc_macro \
 		-Cpanic=unwind -Cforce-unwind-tables=y \
 		--crate-type proc-macro --out-dir $(objtree)/rust/ \
 		--crate-name $(patsubst lib%.so,%,$(notdir $@)) $<; \
@@ -100,10 +99,11 @@ quiet_cmd_rustc_procmacro = RUSTC P $@
 $(objtree)/rust/libmodule.so: $(srctree)/rust/module.rs FORCE
 	$(call if_changed_dep,rustc_procmacro)
 
-quiet_cmd_rustc_library = RUSTC L $@
+quiet_cmd_rustc_library = RUSTC L $(if $(skip_clippy),,$(CLIPPY_QUIET_TAG))$@
       cmd_rustc_library = \
 	RUST_BINDINGS_FILE=$(abspath $(objtree)/rust/bindings_generated.rs) \
-	$(RUSTC) $(rustc_flags) $(rustc_cross_flags) $(rustc_target_flags) \
+	$(if $(skip_clippy),,$(CLIPPY_DRIVER)) $(RUSTC) $(rustc_flags) \
+		$(rustc_cross_flags) $(rustc_target_flags) \
 		--crate-type rlib --out-dir $(objtree)/rust/ -L $(objtree)/rust/ \
 		--crate-name $(patsubst %.o,%,$(notdir $@)) $<; \
 	mv $(objtree)/rust/$(patsubst %.o,%,$(notdir $@)).d $(depfile); \
@@ -115,20 +115,22 @@ rustc_sysroot = $(shell $(RUSTC) $(rustc_flags) --print sysroot)
 rustc_src = $(rustc_sysroot)/lib/rustlib/src/rust
 
 .SECONDEXPANSION:
+$(objtree)/rust/core.o: private skip_clippy = 1
 $(objtree)/rust/core.o: $$(rustc_src)/library/core/src/lib.rs FORCE
 	$(call if_changed_dep,rustc_library)
 
-$(objtree)/rust/compiler_builtins.o: rustc_objcopy = -w -W '__*'
+$(objtree)/rust/compiler_builtins.o: private rustc_objcopy = -w -W '__*'
 $(objtree)/rust/compiler_builtins.o: $(srctree)/rust/compiler_builtins.rs \
     $(objtree)/rust/core.o FORCE
 	$(call if_changed_dep,rustc_library)
 
+$(objtree)/rust/alloc.o: private skip_clippy = 1
 $(objtree)/rust/alloc.o: $$(rustc_src)/library/alloc/src/lib.rs \
     $(objtree)/rust/compiler_builtins.o FORCE
 	$(call if_changed_dep,rustc_library)
 
 # ICE on `--extern module`: https://github.com/rust-lang/rust/issues/56935
-$(objtree)/rust/kernel.o: rustc_target_flags = --extern alloc \
+$(objtree)/rust/kernel.o: private rustc_target_flags = --extern alloc \
     --extern module=$(objtree)/rust/libmodule.so
 $(objtree)/rust/kernel.o: $(srctree)/rust/kernel/lib.rs $(objtree)/rust/alloc.o \
     $(objtree)/rust/libmodule.so $(objtree)/rust/bindings_generated.rs FORCE

scripts/Makefile.build

@@ -301,10 +301,10 @@ $(obj)/%.lst: $(src)/%.c FORCE
 
 rustc_cross_flags := --target=$(srctree)/arch/$(SRCARCH)/rust/target.json
 
-quiet_cmd_rustc_o_rs = RUSTC $(quiet_modtag) $@
+quiet_cmd_rustc_o_rs = RUSTC $(CLIPPY_QUIET_TAG)$(quiet_modtag) $@
       cmd_rustc_o_rs = \
 	RUST_MODFILE=$(modfile) \
-	$(RUSTC) $(rustc_flags) $(rustc_cross_flags) \
+	$(CLIPPY_DRIVER) $(RUSTC) $(rustc_flags) $(rustc_cross_flags) \
 		--extern alloc --extern kernel \
 		--crate-type rlib --out-dir $(obj) -L $(objtree)/rust/ \
 		--crate-name $(patsubst %.o,%,$(notdir $@)) $<; \