From 36a58cf16ce0aa705d294fac9cba5b2fe764dbdc Mon Sep 17 00:00:00 2001 From: Nicolas Sarlin Date: Fri, 5 Jul 2024 15:04:24 +0200 Subject: [PATCH] chore(backward): add custom lint to detect missing Versionize implem --- .gitignore | 1 + Cargo.toml | 7 +- utils/cargo-tfhe-lints-inner/Cargo.toml | 10 ++ utils/cargo-tfhe-lints-inner/README.md | 16 +++ .../rust-toolchain.toml | 4 + utils/cargo-tfhe-lints-inner/src/main.rs | 54 ++++++++ .../src/serialize_without_versionize.rs | 119 ++++++++++++++++++ utils/cargo-tfhe-lints-inner/src/utils.rs | 52 ++++++++ utils/cargo-tfhe-lints/Cargo.toml | 8 ++ utils/cargo-tfhe-lints/README.md | 25 ++++ utils/cargo-tfhe-lints/src/main.rs | 38 ++++++ 11 files changed, 332 insertions(+), 2 deletions(-) create mode 100644 utils/cargo-tfhe-lints-inner/Cargo.toml create mode 100644 utils/cargo-tfhe-lints-inner/README.md create mode 100644 utils/cargo-tfhe-lints-inner/rust-toolchain.toml create mode 100644 utils/cargo-tfhe-lints-inner/src/main.rs create mode 100644 utils/cargo-tfhe-lints-inner/src/serialize_without_versionize.rs create mode 100644 utils/cargo-tfhe-lints-inner/src/utils.rs create mode 100644 utils/cargo-tfhe-lints/Cargo.toml create mode 100644 utils/cargo-tfhe-lints/README.md create mode 100644 utils/cargo-tfhe-lints/src/main.rs diff --git a/.gitignore b/.gitignore index ac5a6f09e..0aad68ba3 100644 --- a/.gitignore +++ b/.gitignore @@ -7,6 +7,7 @@ target/ # In case of symlinked keys /keys +**/*.rmeta **/Cargo.lock **/*.bin diff --git a/Cargo.toml b/Cargo.toml index 323d22495..96f9a264f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,10 +8,13 @@ members = [ "concrete-csprng", "backends/tfhe-cuda-backend", "utils/tfhe-versionable", - "utils/tfhe-versionable-derive" + "utils/tfhe-versionable-derive", ] + exclude = [ - "tfhe/backward_compatibility_tests" + "tfhe/backward_compatibility_tests", + "utils/cargo-tfhe-lints-inner", + "utils/cargo-tfhe-lints" ] [profile.bench] diff --git a/utils/cargo-tfhe-lints-inner/Cargo.toml b/utils/cargo-tfhe-lints-inner/Cargo.toml new file mode 100644 index 000000000..400d708b1 --- /dev/null +++ b/utils/cargo-tfhe-lints-inner/Cargo.toml @@ -0,0 +1,10 @@ +[package] +name = "cargo-tfhe-lints-inner" +version = "0.1.0" +edition = "2021" + +[dependencies] +rustc-tools = "0.78" + +[package.metadata.rust-analyzer] +rustc_private=true diff --git a/utils/cargo-tfhe-lints-inner/README.md b/utils/cargo-tfhe-lints-inner/README.md new file mode 100644 index 000000000..cde2b7603 --- /dev/null +++ b/utils/cargo-tfhe-lints-inner/README.md @@ -0,0 +1,16 @@ +# TFHE-lints-inner + +Implementation of the lints in the custom [tfhe-lints](../cargo-tfhe-lints/README.md) tool. + +## Installation +``` +cargo install --path . +``` + +## Usage +This can be run directly by specifying the toolchain: +``` +cargo +nightly-2024-05-02 tfhe-lints-inner +``` + +For a more ergonomic usage, see [tfhe-lints](../cargo-tfhe-lints/README.md) diff --git a/utils/cargo-tfhe-lints-inner/rust-toolchain.toml b/utils/cargo-tfhe-lints-inner/rust-toolchain.toml new file mode 100644 index 000000000..a6a6e0cf3 --- /dev/null +++ b/utils/cargo-tfhe-lints-inner/rust-toolchain.toml @@ -0,0 +1,4 @@ +[toolchain] +channel = "nightly-2024-05-02" +components = ["rustc-dev", "rust-src", "llvm-tools-preview"] +profile = "minimal" diff --git a/utils/cargo-tfhe-lints-inner/src/main.rs b/utils/cargo-tfhe-lints-inner/src/main.rs new file mode 100644 index 000000000..d4b35995d --- /dev/null +++ b/utils/cargo-tfhe-lints-inner/src/main.rs @@ -0,0 +1,54 @@ +#![feature(rustc_private)] +#![warn(clippy::pedantic)] +#![allow(clippy::module_name_repetitions)] +#![allow(clippy::must_use_candidate)] + +mod serialize_without_versionize; +pub mod utils; + +// We need to import them like this otherwise it doesn't work. +extern crate rustc_ast; +extern crate rustc_hir; +extern crate rustc_lint; +extern crate rustc_middle; +extern crate rustc_session; +extern crate rustc_span; + +use rustc_lint::LintStore; +use rustc_tools::with_lints; +use serialize_without_versionize::SerializeWithoutVersionize; + +fn main() { + let tool_args = std::env::args().skip(2).collect::>(); + + let (cargo_args, rustc_args) = if let Some(idx) = tool_args.iter().position(|arg| arg == "--") { + tool_args.split_at(idx) + } else { + (tool_args.as_slice(), &[] as &[String]) + }; + + rustc_tools::cargo_integration(&cargo_args, |args| { + let mut args = args.to_vec(); + args.extend(rustc_args.iter().skip(1).cloned()); + args.extend( + [ + "--emit=metadata", + // These params allows to use the syntax + // `#[cfg_attr(tfhe_lints, allow(tfhe_lints::serialize_without_versionize))]` + "-Zcrate-attr=feature(register_tool)", + "-Zcrate-attr=register_tool(tfhe_lints)", + "--cfg=tfhe_lints", + ] + .iter() + .map(ToString::to_string), + ); + let serialize_without_versionize = SerializeWithoutVersionize::new(); + + with_lints(&args, vec![], move |store: &mut LintStore| { + let lint = serialize_without_versionize.clone(); + store.register_late_pass(move |_| Box::new(lint.clone())); + }) + .expect("with_lints failed"); + }) + .expect("cargo_integration failed"); +} diff --git a/utils/cargo-tfhe-lints-inner/src/serialize_without_versionize.rs b/utils/cargo-tfhe-lints-inner/src/serialize_without_versionize.rs new file mode 100644 index 000000000..3ccec6fc1 --- /dev/null +++ b/utils/cargo-tfhe-lints-inner/src/serialize_without_versionize.rs @@ -0,0 +1,119 @@ +use std::sync::{Arc, OnceLock}; + +use rustc_hir::def_id::DefId; +use rustc_hir::{Impl, Item, ItemKind}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; +use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::sym; + +use crate::utils::{get_def_id_from_ty, is_allowed_lint, symbols_list_from_str}; + +declare_tool_lint! { + pub tfhe_lints::SERIALIZE_WITHOUT_VERSIONIZE, + Warn, + "warns if `Serialize` is implemented without `Versionize`" +} + +#[derive(Default)] +pub struct SerializeWithoutVersionizeInner { + pub versionize_trait: OnceLock>, +} + +const VERSIONIZE_TRAIT: [&str; 2] = ["tfhe_versionable", "Versionize"]; +const SERIALIZE_TRAIT: [&str; 3] = ["serde", "ser", "Serialize"]; +const LINT_NAME: &str = "serialize_without_versionize"; + +impl SerializeWithoutVersionizeInner { + /// Tries to find the definition of the `Versionize` trait. The value is memoized and is + /// instantly accessed after the first lookup. + pub fn versionize_trait(&self, cx: &LateContext<'_>) -> Option { + self.versionize_trait + .get_or_init(|| { + let versionize_trait = cx.tcx.all_traits().find(|def_id| { + cx.match_def_path(*def_id, symbols_list_from_str(&VERSIONIZE_TRAIT).as_slice()) + }); + + versionize_trait + }) + .to_owned() + } +} + +#[derive(Default, Clone)] +pub struct SerializeWithoutVersionize(pub Arc); + +impl SerializeWithoutVersionize { + pub fn new() -> Self { + Self::default() + } +} + +impl_lint_pass!(SerializeWithoutVersionize => [SERIALIZE_WITHOUT_VERSIONIZE]); + +impl<'tcx> LateLintPass<'tcx> for SerializeWithoutVersionize { + fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) { + // If the currently checked item is a trait impl + if let ItemKind::Impl(Impl { + of_trait: Some(ref trait_ref), + .. + }) = item.kind + { + // Gets the target type of the implementation + let ty: rustc_middle::ty::Ty<'tcx> = + cx.tcx.type_of(item.owner_id).instantiate_identity(); + + if let Some(type_def_id) = get_def_id_from_ty(ty) { + // If the type has been automatically generated, skip it + if cx.tcx.has_attr(type_def_id, sym::automatically_derived) { + return; + } + + // Skip it if the user explicitly allowed it. + if is_allowed_lint(cx, type_def_id, LINT_NAME) { + return; + } + + // Check if the implemented trait is `Serialize` + if let Some(versionize_trait) = self.0.versionize_trait(cx) { + if let Some(def_id) = trait_ref.trait_def_id() { + if cx.match_def_path( + def_id, + symbols_list_from_str(&SERIALIZE_TRAIT).as_slice(), + ) { + // Try to find an implementation of versionize for this type + let mut found_impl = false; + cx.tcx + .for_each_relevant_impl(versionize_trait, ty, |impl_id| { + if !found_impl { + let trait_ref = cx + .tcx + .impl_trait_ref(impl_id) + .expect("must be a trait implementation"); + + if trait_ref.instantiate_identity().args.type_at(0) == ty { + found_impl = true; + } + } + }); + + if !found_impl { + // Emit a warning + cx.span_lint( + SERIALIZE_WITHOUT_VERSIONIZE, + cx.tcx.def_span(type_def_id), + format!( + "Type {ty} implements `Serialize` but does not implement `Versionize`" + ),|diag| { + diag.note("Add `#[derive(Versionize)] for this type or silence this warning using \ +`#[cfg_attr(tfhe_lints, allow(tfhe_lints::serialize_without_versionize))]``"); + diag.span_note(item.span, "`Serialize` derived here"); + }, + ); + } + } + } + } + } + } + } +} diff --git a/utils/cargo-tfhe-lints-inner/src/utils.rs b/utils/cargo-tfhe-lints-inner/src/utils.rs new file mode 100644 index 000000000..f16b0763a --- /dev/null +++ b/utils/cargo-tfhe-lints-inner/src/utils.rs @@ -0,0 +1,52 @@ +use rustc_ast::tokenstream::TokenTree; +use rustc_hir::def_id::DefId; +use rustc_lint::LateContext; +use rustc_middle::ty::{Ty, TyKind}; +use rustc_span::Symbol; + +const TOOL_NAME: &str = "tfhe_lints"; + +/// Converts an array of str into a Vec of [`Symbol`] +pub fn symbols_list_from_str(list: &[&str]) -> Vec { + list.iter().map(|s| Symbol::intern(s)).collect() +} + +/// Checks if the lint is allowed for the item represented by [`DefId`]. +/// This shouldn't be necessary since the lints are declared with the +/// `declare_tool_lint` macro but for a mysterious reason this does not +/// work automatically. +pub fn is_allowed_lint(cx: &LateContext<'_>, target: DefId, lint_name: &str) -> bool { + for attr in cx.tcx.get_attrs(target, Symbol::intern("allow")) { + let tokens = attr.get_normal_item().args.inner_tokens(); + let mut trees = tokens.trees(); + + if let Some(TokenTree::Token(tool_token, _)) = trees.next() { + if tool_token.is_ident_named(Symbol::intern(TOOL_NAME)) { + trees.next(); + if let Some(TokenTree::Token(tool_token, _)) = trees.next() { + if tool_token.is_ident_named(Symbol::intern(lint_name)) { + return true; + } + } + } + } + } + + false +} + +/// Gets the [`DefId`] of a type +pub fn get_def_id_from_ty(ty: Ty<'_>) -> Option { + match ty.kind() { + TyKind::Adt(adt_def, _) => Some(adt_def.did()), + TyKind::Alias(_, alias_ty) => Some(alias_ty.def_id), + TyKind::Dynamic(predicates, ..) => predicates.principal_def_id(), + TyKind::FnDef(def_id, _) + | TyKind::Foreign(def_id) + | TyKind::Closure(def_id, ..) + | TyKind::CoroutineClosure(def_id, _) + | TyKind::Coroutine(def_id, _) + | TyKind::CoroutineWitness(def_id, _) => Some(*def_id), + _ => None, + } +} diff --git a/utils/cargo-tfhe-lints/Cargo.toml b/utils/cargo-tfhe-lints/Cargo.toml new file mode 100644 index 000000000..c2c779838 --- /dev/null +++ b/utils/cargo-tfhe-lints/Cargo.toml @@ -0,0 +1,8 @@ +[package] +name = "cargo-tfhe-lints" +version = "0.1.0" +edition = "2021" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] diff --git a/utils/cargo-tfhe-lints/README.md b/utils/cargo-tfhe-lints/README.md new file mode 100644 index 000000000..91d98df96 --- /dev/null +++ b/utils/cargo-tfhe-lints/README.md @@ -0,0 +1,25 @@ +# TFHE-lints + +A collection of rust lints specific to the **TFHE-rs** project. This tool is built using [rustc-tools](https://github.com/GuillaumeGomez/rustc-tools). + +## List of lints +- `serilaize_without_versionize`: warns if `Serialize` is implemented without `Versionize` + +## Installation + +### Install the inner tool +``` +cargo install --path ../cargo-tfhe-lints-inner +``` + +### Install this wrapper +``` +cargo install --path . +``` + +## Usage +Use it as any other cargo tool: +``` +cargo tfhe-lints +``` +You can specify features like you would do with clippy. diff --git a/utils/cargo-tfhe-lints/src/main.rs b/utils/cargo-tfhe-lints/src/main.rs new file mode 100644 index 000000000..47bdc57e4 --- /dev/null +++ b/utils/cargo-tfhe-lints/src/main.rs @@ -0,0 +1,38 @@ +use std::process::{exit, Command}; + +fn get_supported_rustc_version() -> &'static str { + const TOOLCHAIN_FILE: &str = include_str!("../../cargo-tfhe-lints-inner/rust-toolchain.toml"); + + TOOLCHAIN_FILE + .lines() + .find(|line| line.starts_with("channel")) + .and_then(|line| { + line.rsplit('=') + .next() + .unwrap() + .trim() + .strip_prefix('"') + .unwrap() + .strip_suffix('"') + }) + .unwrap() +} + +fn main() { + let cargo_args = std::env::args().skip(2).collect::>(); + let toolchain = format!("+{}", get_supported_rustc_version()); + + if let Err(err) = Command::new("cargo") + .arg(toolchain.as_str()) + .arg("tfhe-lints-inner") + .args(&cargo_args) + .spawn() + .and_then(|mut child| child.wait()) + { + eprintln!( + "Command `cargo {toolchain} tfhe-lints-inner {}` failed: {err:?}", + cargo_args.join(" "), + ); + exit(1); + } +}