From 0ae25af077e65628efbc89dc4008eae1e26894c6 Mon Sep 17 00:00:00 2001 From: xFrednet Date: Mon, 27 Nov 2023 20:47:06 +0100 Subject: [PATCH 1/5] API: Initial `sem::TyKind::implements_trait` boilerplate --- marker_adapter/src/context.rs | 14 ++++- marker_api/src/context.rs | 11 +++- marker_api/src/sem/ty.rs | 62 ++++++++++++++++++- marker_rustc_driver/src/context.rs | 5 ++ marker_uilints/src/lib.rs | 12 ++++ marker_uilints/src/sem.rs | 34 ++++++++++ .../tests/ui/expr/ty_impls_trait.rs | 6 ++ .../tests/ui/expr/ty_impls_trait.stderr | 15 +++++ 8 files changed, 156 insertions(+), 3 deletions(-) create mode 100644 marker_uilints/src/sem.rs create mode 100644 marker_uilints/tests/ui/expr/ty_impls_trait.rs create mode 100644 marker_uilints/tests/ui/expr/ty_impls_trait.stderr diff --git a/marker_adapter/src/context.rs b/marker_adapter/src/context.rs index 8b0332fe..eeff7deb 100644 --- a/marker_adapter/src/context.rs +++ b/marker_adapter/src/context.rs @@ -128,6 +128,7 @@ impl<'ast> MarkerContextWrapper<'ast> { emit_diag, resolve_ty_ids, expr_ty, + ty_implements_trait, span, span_snippet, span_source, @@ -145,6 +146,7 @@ pub trait MarkerContextDriver<'ast> { fn resolve_ty_ids(&'ast self, path: &str) -> &'ast [TyDefId]; fn expr_ty(&'ast self, expr: ExprId) -> marker_api::sem::TyKind<'ast>; + fn ty_implements_trait(&'ast self, ty: sem::TyKind<'ast>, trait_ref: &sem::FfiUserDefinedTraitRef<'_>) -> bool; fn span(&'ast self, owner: SpanId) -> &'ast Span<'ast>; fn span_snippet(&'ast self, span: &Span<'_>) -> Option<&'ast str>; fn span_source(&'ast self, span: &Span<'_>) -> SpanSource<'ast>; @@ -165,12 +167,22 @@ extern "C" fn resolve_ty_ids<'ast>( unsafe { as_driver(data) }.resolve_ty_ids((&path).into()).into() } -// False positive because `SemTyKind` is non-exhaustive +// False positive because `TyKind` is non-exhaustive #[allow(improper_ctypes_definitions)] extern "C" fn expr_ty<'ast>(data: &'ast MarkerContextData, expr: ExprId) -> marker_api::sem::TyKind<'ast> { unsafe { as_driver(data) }.expr_ty(expr) } +// False positive because `TyKind` is non-exhaustive +#[allow(improper_ctypes_definitions)] +extern "C" fn ty_implements_trait<'ast>( + data: &'ast MarkerContextData, + ty: sem::TyKind<'ast>, + trait_ref: &sem::FfiUserDefinedTraitRef<'_>, +) -> bool { + unsafe { as_driver(data) }.ty_implements_trait(ty, trait_ref) +} + extern "C" fn span<'ast>(data: &'ast MarkerContextData, span_id: SpanId) -> &'ast Span<'ast> { unsafe { as_driver(data) }.span(span_id) } diff --git a/marker_api/src/context.rs b/marker_api/src/context.rs index f783cd9b..fcd48b38 100644 --- a/marker_api/src/context.rs +++ b/marker_api/src/context.rs @@ -11,7 +11,7 @@ use crate::{ common::{ExpnId, ExprId, ItemId, Level, MacroReport, SpanId, SymbolId, TyDefId}, diagnostic::{Diagnostic, DiagnosticBuilder, EmissionNode}, ffi, - sem::TyKind, + sem::{self, TyKind}, span::{ExpnInfo, FileInfo, FilePos, Span, SpanPos, SpanSource}, Lint, }; @@ -259,6 +259,13 @@ impl<'ast> MarkerContext<'ast> { pub(crate) fn expr_ty(&self, expr: ExprId) -> TyKind<'ast> { self.callbacks.call_expr_ty(expr) } + pub(crate) fn ty_implements_trait( + &self, + ty: sem::TyKind<'ast>, + trait_ref: &sem::FfiUserDefinedTraitRef<'_>, + ) -> bool { + (self.callbacks.ty_implements_trait)(self.callbacks.data, ty, trait_ref) + } // FIXME: This function should probably be removed in favor of a better // system to deal with spans. See rust-marker/marker#175 @@ -322,6 +329,8 @@ struct MarkerContextCallbacks<'ast> { // Internal utility pub expr_ty: extern "C" fn(&'ast MarkerContextData, ExprId) -> TyKind<'ast>, + pub ty_implements_trait: + extern "C" fn(&'ast MarkerContextData, sem::TyKind<'ast>, &sem::FfiUserDefinedTraitRef<'_>) -> bool, pub span: extern "C" fn(&'ast MarkerContextData, SpanId) -> &'ast Span<'ast>, pub span_snippet: extern "C" fn(&'ast MarkerContextData, &Span<'ast>) -> ffi::FfiOption>, pub span_source: extern "C" fn(&'ast MarkerContextData, &Span<'_>) -> SpanSource<'ast>, diff --git a/marker_api/src/sem/ty.rs b/marker_api/src/sem/ty.rs index 74166b09..bfc2eccb 100644 --- a/marker_api/src/sem/ty.rs +++ b/marker_api/src/sem/ty.rs @@ -14,7 +14,11 @@ pub use sequence_ty::*; pub use trait_ty::*; pub use user_ty::*; -use crate::common::DriverTyId; +use crate::{ + common::{DriverTyId, TyDefId}, + context::with_cx, + ffi::FfiSlice, +}; use std::{fmt::Debug, marker::PhantomData}; /// The semantic representation of a type. @@ -113,6 +117,16 @@ impl<'ast> TyKind<'ast> { } ty } + + #[must_use] + pub fn implements_trait(self, trait_ref: &UserDefinedTraitRef) -> bool { + let ids = match &trait_ref.trait_ref { + TyDefIdSource::Path(path) => with_cx(&self, |cx| cx.resolve_ty_ids(&path)), + TyDefIdSource::Id(_id) => todo!(), + }; + let ffi = FfiUserDefinedTraitRef { trait_refs: ids.into() }; + with_cx(&self, |cx| cx.ty_implements_trait(self, &ffi)) + } } #[repr(C)] @@ -154,3 +168,49 @@ macro_rules! impl_ty_data { }; } use impl_ty_data; + +// TODO docs +// TODO Use a better name +#[derive(Debug)] +pub struct UserDefinedTraitRef { + #[allow(dead_code)] + trait_ref: TyDefIdSource, + // TODO generics +} + +impl UserDefinedTraitRef { + // TODO Decide how to construct this object, probably a builder since it's user exposed + pub fn new(trait_ref: impl Into) -> Self { + Self { + trait_ref: trait_ref.into(), + } + } +} + +#[derive(Debug)] +#[cfg_attr(feature = "driver-api", visibility::make(pub))] +pub(crate) struct FfiUserDefinedTraitRef<'a> { + #[allow(dead_code)] + trait_refs: FfiSlice<'a, TyDefId>, + // TODO generics +} + +#[derive(Debug)] +#[non_exhaustive] +pub enum TyDefIdSource { + Id(TyDefId), + // TODO: Handle the path properly, since `Str` is not ABI safe + Path(String), +} + +impl From for TyDefIdSource { + fn from(value: TyDefId) -> Self { + TyDefIdSource::Id(value) + } +} + +impl From for TyDefIdSource { + fn from(value: String) -> Self { + TyDefIdSource::Path(value) + } +} diff --git a/marker_rustc_driver/src/context.rs b/marker_rustc_driver/src/context.rs index 77da9fa9..0e8bf7df 100644 --- a/marker_rustc_driver/src/context.rs +++ b/marker_rustc_driver/src/context.rs @@ -195,6 +195,11 @@ impl<'ast, 'tcx: 'ast> MarkerContextDriver<'ast> for RustcContext<'ast, 'tcx> { self.marker_converter.expr_ty(hir_id) } + fn ty_implements_trait(&'ast self, _ty: sem::TyKind<'ast>, _trait_ref: &sem::FfiUserDefinedTraitRef<'_>) -> bool { + // TODO This + todo!() + } + fn span(&'ast self, span_id: SpanId) -> &'ast Span<'ast> { let rustc_span = self.rustc_converter.to_span_from_id(span_id); self.storage.alloc(self.marker_converter.to_span(rustc_span)) diff --git a/marker_uilints/src/lib.rs b/marker_uilints/src/lib.rs index eafbddf7..ae62a30d 100644 --- a/marker_uilints/src/lib.rs +++ b/marker_uilints/src/lib.rs @@ -1,6 +1,7 @@ #![doc = include_str!("../README.md")] #![warn(clippy::pedantic)] +mod sem; mod utils; use marker_api::{ @@ -68,6 +69,14 @@ marker_api::declare_lint! { Warn, } +marker_api::declare_lint! { + /// # What it does + /// Checks if the semantic type of an identifier called `check_traits` + /// implements a trait. + TEST_TY_IMPLS_TRAIT, + Allow, +} + fn emit_item_with_test_name_lint<'ast>( cx: &'ast MarkerContext<'ast>, node: impl EmissionNode<'ast>, @@ -84,6 +93,7 @@ impl LintPass for TestLintPass { ITEM_WITH_TEST_NAME, PRINT_EVERY_EXPR, utils::TEST_CONTAINS_RETURN, + TEST_TY_IMPLS_TRAIT, ])) .build() } @@ -231,6 +241,8 @@ impl LintPass for TestLintPass { } fn check_expr<'ast>(&mut self, cx: &'ast MarkerContext<'ast>, expr: ExprKind<'ast>) { + sem::check_expr(cx, expr); + cx.emit_lint(PRINT_EVERY_EXPR, expr, "expr").decorate(|diag| { diag.note(&format!("SpanSource: {:#?}", expr.span().source())); diag.note(&format!("Snippet: {:#?}", expr.span().snippet_or("<..>"))); diff --git a/marker_uilints/src/sem.rs b/marker_uilints/src/sem.rs new file mode 100644 index 00000000..09a60eb0 --- /dev/null +++ b/marker_uilints/src/sem.rs @@ -0,0 +1,34 @@ +use marker_api::{diagnostic::DiagnosticBuilder, prelude::*, sem::UserDefinedTraitRef}; + +pub fn check_expr<'ast>(cx: &'ast MarkerContext<'ast>, expr: ExprKind<'ast>) { + test_ty_impls_trait(cx, expr) +} + +fn test_ty_impls_trait<'ast>(cx: &'ast MarkerContext<'ast>, input_expr: ExprKind<'ast>) { + let ast::ExprKind::Path(expr) = input_expr else { + return; + }; + if !expr + .path() + .segments() + .last() + .map_or(false, |seg| seg.ident().name().starts_with("check_traits")) + { + return; + } + + cx.emit_lint(super::TEST_TY_IMPLS_TRAIT, expr, "checking trait impls:") + .decorate(|diag| { + let ty: sem::TyKind<'_> = expr.ty(); + test_implements_trait(diag, ty, "std::clone::Clone"); + // diag.note(&format!("Snippet: {:#?}", expr.span().snippet_or("<..>"))); + }); +} + +fn test_implements_trait(diag: &mut DiagnosticBuilder<'_>, ty: sem::TyKind<'_>, path: impl Into) { + let path = path.into(); + diag.note(format!( + "Implements: `{path}`: {}", + ty.implements_trait(&UserDefinedTraitRef::new(path.clone())) + )); +} diff --git a/marker_uilints/tests/ui/expr/ty_impls_trait.rs b/marker_uilints/tests/ui/expr/ty_impls_trait.rs new file mode 100644 index 00000000..f2fda4b3 --- /dev/null +++ b/marker_uilints/tests/ui/expr/ty_impls_trait.rs @@ -0,0 +1,6 @@ +#![warn(marker::marker_uilints::test_ty_impls_trait)] + +fn main() { + let check_traits = 1; + let _ = check_traits; +} \ No newline at end of file diff --git a/marker_uilints/tests/ui/expr/ty_impls_trait.stderr b/marker_uilints/tests/ui/expr/ty_impls_trait.stderr new file mode 100644 index 00000000..89cafcb4 --- /dev/null +++ b/marker_uilints/tests/ui/expr/ty_impls_trait.stderr @@ -0,0 +1,15 @@ +warning: checking trait impls: + --> $DIR/ty_impls_trait.rs:5:13 + | +5 | let _ = check_traits; + | ^^^^^^^^^^^^ + | + = note: Implements: `std::clone::Clone`: false +note: the lint level is defined here + --> $DIR/ty_impls_trait.rs:1:9 + | +1 | #![warn(marker::marker_uilints::test_ty_impls_trait)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +warning: 1 warning emitted + From c3fa8b646ea1972e737dc4a133913de58a1351c6 Mon Sep 17 00:00:00 2001 From: xFrednet Date: Mon, 27 Nov 2023 22:02:49 +0100 Subject: [PATCH 2/5] Rustc: impl `ImplementsTraitBackend` --- marker_api/src/lib.rs | 2 +- marker_api/src/sem/ty.rs | 41 +++++++++++-- marker_rustc_driver/src/context.rs | 60 ++++++++++++++++++- .../src/conversion/rustc/common.rs | 2 +- marker_rustc_driver/src/lib.rs | 1 + marker_uilints/src/sem.rs | 7 ++- .../tests/ui/expr/ty_impls_trait.rs | 13 ++++ .../tests/ui/expr/ty_impls_trait.stderr | 38 ++++++++++-- 8 files changed, 148 insertions(+), 16 deletions(-) diff --git a/marker_api/src/lib.rs b/marker_api/src/lib.rs index 2d59ebde..16f23cf1 100644 --- a/marker_api/src/lib.rs +++ b/marker_api/src/lib.rs @@ -6,7 +6,7 @@ #![allow(clippy::unused_self)] // `self` is needed to change the behavior later #![allow(clippy::missing_panics_doc)] // Temporary allow for `todo!`s #![allow(clippy::new_without_default)] // Not very helpful as `new` is almost always cfged -#![cfg_attr(not(feature = "driver-api"), allow(dead_code))] +#![cfg_attr(not(feature = "driver-api"), allow(dead_code, unused_macros, unused_imports))] #![cfg_attr(marker, warn(marker::marker_lints::not_using_has_span_trait))] pub static MARKER_API_VERSION: &str = env!("CARGO_PKG_VERSION"); diff --git a/marker_api/src/sem/ty.rs b/marker_api/src/sem/ty.rs index bfc2eccb..979e1b13 100644 --- a/marker_api/src/sem/ty.rs +++ b/marker_api/src/sem/ty.rs @@ -121,14 +121,40 @@ impl<'ast> TyKind<'ast> { #[must_use] pub fn implements_trait(self, trait_ref: &UserDefinedTraitRef) -> bool { let ids = match &trait_ref.trait_ref { - TyDefIdSource::Path(path) => with_cx(&self, |cx| cx.resolve_ty_ids(&path)), + TyDefIdSource::Path(path) => with_cx(&self, |cx| cx.resolve_ty_ids(path)), TyDefIdSource::Id(_id) => todo!(), }; - let ffi = FfiUserDefinedTraitRef { trait_refs: ids.into() }; + let ffi = FfiUserDefinedTraitRef { trait_ids: ids.into() }; with_cx(&self, |cx| cx.ty_implements_trait(self, &ffi)) } } +#[cfg(feature = "driver-api")] +impl<'ast> TyKind<'ast> { + impl_ty_kind_fn!(data() -> &CommonTyData<'ast>); +} + +macro_rules! impl_ty_kind_fn { + ($method:ident () -> $return_ty:ty) => { + impl_ty_kind_fn!($method() -> $return_ty, + Bool, Num, Text, Never, + Tuple, Array, Slice, + Fn, Closure, + Ref, RawPtr, FnPtr, + TraitObj, Adt, Generic, Alias, + Unstable + ); + }; + ($method:ident () -> $return_ty:ty $(, $item:ident)+) => { + pub fn $method(&self) -> $return_ty { + match self { + $(TyKind::$item(data) => data.$method(),)* + } + } + }; +} +use impl_ty_kind_fn; + #[repr(C)] #[cfg_attr(feature = "driver-api", visibility::make(pub))] #[cfg_attr(feature = "driver-api", derive(typed_builder::TypedBuilder))] @@ -153,7 +179,7 @@ impl<'ast> CommonTyData<'ast> { macro_rules! impl_ty_data { ($self_ty:ty, $enum_name:ident) => { - #[cfg(feature = "driver_api")] + #[cfg(feature = "driver-api")] impl<'ast> $self_ty { pub fn data(&self) -> &$crate::sem::ty::CommonTyData<'ast> { &self.data @@ -191,10 +217,17 @@ impl UserDefinedTraitRef { #[cfg_attr(feature = "driver-api", visibility::make(pub))] pub(crate) struct FfiUserDefinedTraitRef<'a> { #[allow(dead_code)] - trait_refs: FfiSlice<'a, TyDefId>, + trait_ids: FfiSlice<'a, TyDefId>, // TODO generics } +#[cfg(feature = "driver-api")] +impl<'a> FfiUserDefinedTraitRef<'a> { + pub fn trait_ids(&self) -> &'a [TyDefId] { + self.trait_ids.get() + } +} + #[derive(Debug)] #[non_exhaustive] pub enum TyDefIdSource { diff --git a/marker_rustc_driver/src/context.rs b/marker_rustc_driver/src/context.rs index 0e8bf7df..399ab0a7 100644 --- a/marker_rustc_driver/src/context.rs +++ b/marker_rustc_driver/src/context.rs @@ -9,7 +9,9 @@ use marker_api::{ use rustc_hash::FxHashMap; use rustc_hir as hir; use rustc_lint::LintStore; +use rustc_middle as mid; use rustc_middle::ty::TyCtxt; +use rustc_trait_selection::traits::{Obligation, ObligationCause}; use crate::conversion::{marker::MarkerConverter, rustc::RustcConverter}; @@ -195,9 +197,36 @@ impl<'ast, 'tcx: 'ast> MarkerContextDriver<'ast> for RustcContext<'ast, 'tcx> { self.marker_converter.expr_ty(hir_id) } - fn ty_implements_trait(&'ast self, _ty: sem::TyKind<'ast>, _trait_ref: &sem::FfiUserDefinedTraitRef<'_>) -> bool { - // TODO This - todo!() + fn ty_implements_trait(&'ast self, api_ty: sem::TyKind<'ast>, trait_ref: &sem::FfiUserDefinedTraitRef<'_>) -> bool { + use rustc_middle::ty::TypeVisitableExt; + + // // TODO: This might be a hack, try what happens with too many generics + // mid::ty::ParamEnv::empty() + let ty = self.rustc_converter.to_mid_ty(api_ty.data().driver_id()); + let ty = self.rustc_cx.erase_regions(ty); + #[allow(clippy::manual_assert)] + if ty.has_escaping_bound_vars() { + // TODO: Remove this panic or handle it + panic!("When does this happen? {ty:#?}\n\n{api_ty:#?}\n\n{trait_ref:#?}"); + // return false; + } + + trait_ref + .trait_ids() + .iter() + .map(|api_id| self.rustc_converter.to_def_id(*api_id)) + .filter(|id| { + matches!( + self.rustc_cx.def_kind(id), + hir::def::DefKind::Trait | hir::def::DefKind::TraitAlias + ) + }) + .any(|id| { + // TODO Handle generic arguments + let test_ref = + mid::ty::TraitRef::new(self.rustc_cx, id, std::iter::once(mid::ty::GenericArg::from(ty))); + self.check_implements_trait(ty, test_ref) + }) } fn span(&'ast self, span_id: SpanId) -> &'ast Span<'ast> { @@ -300,3 +329,28 @@ fn select_children_with_name( next_search } + +impl<'ast, 'tcx> RustcContext<'ast, 'tcx> { + fn check_implements_trait(&'ast self, _ty: mid::ty::Ty<'tcx>, trait_ref: mid::ty::TraitRef<'tcx>) -> bool { + use rustc_middle::ty::ToPredicate; + use rustc_trait_selection::infer::TyCtxtInferExt; + use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt; + use rustc_trait_selection::traits::EvaluationResult; + + let infcx = self.rustc_cx.infer_ctxt().build(); + + let obligation = Obligation { + cause: ObligationCause::dummy(), + // TODO: This might be a hack, try what happens with too many generics + param_env: mid::ty::ParamEnv::empty(), + recursion_depth: 0, + predicate: mid::ty::Binder::dummy(trait_ref).to_predicate(self.rustc_cx), + }; + + // Let the record show: I'm not proud of this!!! + + infcx + .evaluate_obligation(&obligation) + .is_ok_and(EvaluationResult::must_apply_modulo_regions) + } +} diff --git a/marker_rustc_driver/src/conversion/rustc/common.rs b/marker_rustc_driver/src/conversion/rustc/common.rs index 4b96a33b..5949812e 100644 --- a/marker_rustc_driver/src/conversion/rustc/common.rs +++ b/marker_rustc_driver/src/conversion/rustc/common.rs @@ -166,7 +166,7 @@ impl<'ast, 'tcx> RustcConverter<'ast, 'tcx> { } #[must_use] - pub fn to_driver_ty_id(&self, id: DriverTyId) -> mid::ty::Ty<'tcx> { + pub fn to_mid_ty(&self, id: DriverTyId) -> mid::ty::Ty<'tcx> { // FIXME(xFrednet): This is theoretically unsound, but should be fine. // See comment in `MarkerConverterInner::to_driver_ty_id` transmute_id!(DriverTyId as mid::ty::Ty<'tcx> = id) diff --git a/marker_rustc_driver/src/lib.rs b/marker_rustc_driver/src/lib.rs index 92692be1..0e4c0339 100644 --- a/marker_rustc_driver/src/lib.rs +++ b/marker_rustc_driver/src/lib.rs @@ -30,6 +30,7 @@ extern crate rustc_middle; extern crate rustc_session; extern crate rustc_span; extern crate rustc_target; +extern crate rustc_trait_selection; pub mod context; pub mod conversion; diff --git a/marker_uilints/src/sem.rs b/marker_uilints/src/sem.rs index 09a60eb0..ab031f1d 100644 --- a/marker_uilints/src/sem.rs +++ b/marker_uilints/src/sem.rs @@ -1,7 +1,7 @@ use marker_api::{diagnostic::DiagnosticBuilder, prelude::*, sem::UserDefinedTraitRef}; pub fn check_expr<'ast>(cx: &'ast MarkerContext<'ast>, expr: ExprKind<'ast>) { - test_ty_impls_trait(cx, expr) + test_ty_impls_trait(cx, expr); } fn test_ty_impls_trait<'ast>(cx: &'ast MarkerContext<'ast>, input_expr: ExprKind<'ast>) { @@ -20,8 +20,11 @@ fn test_ty_impls_trait<'ast>(cx: &'ast MarkerContext<'ast>, input_expr: ExprKind cx.emit_lint(super::TEST_TY_IMPLS_TRAIT, expr, "checking trait impls:") .decorate(|diag| { let ty: sem::TyKind<'_> = expr.ty(); + test_implements_trait(diag, ty, "std::marker::Sized"); + test_implements_trait(diag, ty, "std::marker::Send"); test_implements_trait(diag, ty, "std::clone::Clone"); - // diag.note(&format!("Snippet: {:#?}", expr.span().snippet_or("<..>"))); + test_implements_trait(diag, ty, "std::default::Default"); + test_implements_trait(diag, ty, "unknown::Trait"); }); } diff --git a/marker_uilints/tests/ui/expr/ty_impls_trait.rs b/marker_uilints/tests/ui/expr/ty_impls_trait.rs index f2fda4b3..e95e1afc 100644 --- a/marker_uilints/tests/ui/expr/ty_impls_trait.rs +++ b/marker_uilints/tests/ui/expr/ty_impls_trait.rs @@ -1,5 +1,18 @@ #![warn(marker::marker_uilints::test_ty_impls_trait)] +fn check_generic(check_traits_from_generic: &T) { + let _ = check_traits_from_generic.clone(); +} + +fn return_impl_clone() -> impl Clone { + "I'm a beautiful string, that implements `Clone`" +} + +fn check_impl_ret() { + let check_traits_for_impl_clone = return_impl_clone(); + let _ = check_traits_for_impl_clone; +} + fn main() { let check_traits = 1; let _ = check_traits; diff --git a/marker_uilints/tests/ui/expr/ty_impls_trait.stderr b/marker_uilints/tests/ui/expr/ty_impls_trait.stderr index 89cafcb4..052c407a 100644 --- a/marker_uilints/tests/ui/expr/ty_impls_trait.stderr +++ b/marker_uilints/tests/ui/expr/ty_impls_trait.stderr @@ -1,15 +1,43 @@ warning: checking trait impls: - --> $DIR/ty_impls_trait.rs:5:13 + --> $DIR/ty_impls_trait.rs:4:13 | -5 | let _ = check_traits; - | ^^^^^^^^^^^^ +4 | let _ = check_traits_from_generic.clone(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: Implements: `std::clone::Clone`: false + = note: Implements: `std::marker::Sized`: true + = note: Implements: `std::marker::Send`: false + = note: Implements: `std::clone::Clone`: true + = note: Implements: `std::default::Default`: false + = note: Implements: `unknown::Trait`: false note: the lint level is defined here --> $DIR/ty_impls_trait.rs:1:9 | 1 | #![warn(marker::marker_uilints::test_ty_impls_trait)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -warning: 1 warning emitted +warning: checking trait impls: + --> $DIR/ty_impls_trait.rs:13:13 + | +13 | let _ = check_traits_for_impl_clone; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: Implements: `std::marker::Sized`: true + = note: Implements: `std::marker::Send`: true + = note: Implements: `std::clone::Clone`: true + = note: Implements: `std::default::Default`: false + = note: Implements: `unknown::Trait`: false + +warning: checking trait impls: + --> $DIR/ty_impls_trait.rs:18:13 + | +18 | let _ = check_traits; + | ^^^^^^^^^^^^ + | + = note: Implements: `std::marker::Sized`: true + = note: Implements: `std::marker::Send`: true + = note: Implements: `std::clone::Clone`: true + = note: Implements: `std::default::Default`: true + = note: Implements: `unknown::Trait`: false + +warning: 3 warnings emitted From 9c7e1c6342945eb77845a23ed357f548cd39206f Mon Sep 17 00:00:00 2001 From: xFrednet Date: Tue, 28 Nov 2023 21:29:56 +0100 Subject: [PATCH 3/5] Rustc: Supporting trait testing on generics --- marker_api/src/common/id.rs | 2 +- marker_rustc_driver/src/context.rs | 14 ++- marker_rustc_driver/src/conversion/common.rs | 8 ++ .../src/conversion/marker/common.rs | 22 +++- .../src/conversion/marker/sem/ty.rs | 2 +- .../src/conversion/rustc/common.rs | 10 +- marker_uilints/src/sem.rs | 23 ++-- .../tests/ui/expr/ty_impls_trait.rs | 29 ++++- .../tests/ui/expr/ty_impls_trait.stderr | 104 +++++++++++++----- 9 files changed, 163 insertions(+), 51 deletions(-) diff --git a/marker_api/src/common/id.rs b/marker_api/src/common/id.rs index 8705c8a3..576d7683 100644 --- a/marker_api/src/common/id.rs +++ b/marker_api/src/common/id.rs @@ -148,7 +148,7 @@ new_id! { /// This id is used by the driver to lint the semantic type representation, back to the /// driver type representation, if needed. #[cfg_attr(feature = "driver-api", visibility::make(pub))] - pub(crate) DriverTyId: u64 + pub(crate) DriverTyId: u128 } new_id! { diff --git a/marker_rustc_driver/src/context.rs b/marker_rustc_driver/src/context.rs index 399ab0a7..cc484611 100644 --- a/marker_rustc_driver/src/context.rs +++ b/marker_rustc_driver/src/context.rs @@ -202,7 +202,7 @@ impl<'ast, 'tcx: 'ast> MarkerContextDriver<'ast> for RustcContext<'ast, 'tcx> { // // TODO: This might be a hack, try what happens with too many generics // mid::ty::ParamEnv::empty() - let ty = self.rustc_converter.to_mid_ty(api_ty.data().driver_id()); + let (ty, param_env_src) = self.rustc_converter.to_mid_ty(api_ty.data().driver_id()); let ty = self.rustc_cx.erase_regions(ty); #[allow(clippy::manual_assert)] if ty.has_escaping_bound_vars() { @@ -211,6 +211,7 @@ impl<'ast, 'tcx: 'ast> MarkerContextDriver<'ast> for RustcContext<'ast, 'tcx> { // return false; } + // tcx.generics_of(id) trait_ref .trait_ids() .iter() @@ -225,7 +226,7 @@ impl<'ast, 'tcx: 'ast> MarkerContextDriver<'ast> for RustcContext<'ast, 'tcx> { // TODO Handle generic arguments let test_ref = mid::ty::TraitRef::new(self.rustc_cx, id, std::iter::once(mid::ty::GenericArg::from(ty))); - self.check_implements_trait(ty, test_ref) + self.check_implements_trait(ty, test_ref, self.rustc_cx.param_env(param_env_src)) }) } @@ -331,7 +332,12 @@ fn select_children_with_name( } impl<'ast, 'tcx> RustcContext<'ast, 'tcx> { - fn check_implements_trait(&'ast self, _ty: mid::ty::Ty<'tcx>, trait_ref: mid::ty::TraitRef<'tcx>) -> bool { + fn check_implements_trait( + &'ast self, + _ty: mid::ty::Ty<'tcx>, + trait_ref: mid::ty::TraitRef<'tcx>, + param_env: mid::ty::ParamEnv<'tcx>, + ) -> bool { use rustc_middle::ty::ToPredicate; use rustc_trait_selection::infer::TyCtxtInferExt; use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt; @@ -342,7 +348,7 @@ impl<'ast, 'tcx> RustcContext<'ast, 'tcx> { let obligation = Obligation { cause: ObligationCause::dummy(), // TODO: This might be a hack, try what happens with too many generics - param_env: mid::ty::ParamEnv::empty(), + param_env, recursion_depth: 0, predicate: mid::ty::Binder::dummy(trait_ref).to_predicate(self.rustc_cx), }; diff --git a/marker_rustc_driver/src/conversion/common.rs b/marker_rustc_driver/src/conversion/common.rs index 0a8e9e59..0fb2cb17 100644 --- a/marker_rustc_driver/src/conversion/common.rs +++ b/marker_rustc_driver/src/conversion/common.rs @@ -1,3 +1,5 @@ +use marker_api::common::ItemId; + #[repr(C)] pub struct DefIdLayout { pub krate: u32, @@ -44,3 +46,9 @@ macro_rules! transmute_id { } }; } + +#[repr(C)] +pub struct DriverTyIdLayout { + pub rustc_ty: u64, + pub environment: ItemId, +} diff --git a/marker_rustc_driver/src/conversion/marker/common.rs b/marker_rustc_driver/src/conversion/marker/common.rs index 45f0027a..b1655abd 100644 --- a/marker_rustc_driver/src/conversion/marker/common.rs +++ b/marker_rustc_driver/src/conversion/marker/common.rs @@ -8,7 +8,7 @@ use marker_api::{ use rustc_hir as hir; use rustc_middle as mid; -use crate::conversion::common::{BodyIdLayout, DefIdLayout, ExpnIdLayout, HirIdLayout}; +use crate::conversion::common::{BodyIdLayout, DefIdLayout, DriverTyIdLayout, ExpnIdLayout, HirIdLayout}; use crate::transmute_id; use super::MarkerConverterInner; @@ -165,13 +165,29 @@ impl<'ast, 'tcx> MarkerConverterInner<'ast, 'tcx> { } #[must_use] - pub fn to_driver_ty_id(&self, ty: mid::ty::Ty<'tcx>) -> DriverTyId { + pub fn to_driver_ty_id_for_current_body(&self, ty: mid::ty::Ty<'tcx>) -> DriverTyId { + self.to_driver_ty_id( + ty, + self.rustc_cx.hir().body_owner_def_id( + self.rustc_body + .borrow() + .expect("should always be some, while converting semantic types"), + ), + ) + } + + #[must_use] + pub fn to_driver_ty_id(&self, ty: mid::ty::Ty<'tcx>, environment: impl Into) -> DriverTyId { // FIXME(xFrednet): This is another theoretically unsafe conversion, since // `mid::ty::Ty` doesn't have `#[repr(..)]`. However, it should be fine, as // long as we only access it in the driver, which has the same ABI for all // types. This specific one, is even a wrapper around rustc's `Interned` typed // which should remain valid for `'tcx` and continue to have a small memory footprint. - transmute_id!(mid::ty::Ty as DriverTyId = ty) + let layout = DriverTyIdLayout { + rustc_ty: transmute_id!(mid::ty::Ty<'tcx> as u64 = ty), + environment: self.to_item_id(environment.into()), + }; + transmute_id!(DriverTyIdLayout as DriverTyId = layout) } } diff --git a/marker_rustc_driver/src/conversion/marker/sem/ty.rs b/marker_rustc_driver/src/conversion/marker/sem/ty.rs index e3937ecb..8cf0c3b9 100644 --- a/marker_rustc_driver/src/conversion/marker/sem/ty.rs +++ b/marker_rustc_driver/src/conversion/marker/sem/ty.rs @@ -13,7 +13,7 @@ impl<'ast, 'tcx> MarkerConverterInner<'ast, 'tcx> { #[must_use] pub fn to_sem_ty(&self, rustc_ty: mid::ty::Ty<'tcx>) -> TyKind<'ast> { let data = sem::CommonTyData::builder() - .driver_id(self.to_driver_ty_id(rustc_ty)) + .driver_id(self.to_driver_ty_id_for_current_body(rustc_ty)) .build(); // Semantic types could be cached, the question is if they should and at diff --git a/marker_rustc_driver/src/conversion/rustc/common.rs b/marker_rustc_driver/src/conversion/rustc/common.rs index 5949812e..9633889a 100644 --- a/marker_rustc_driver/src/conversion/rustc/common.rs +++ b/marker_rustc_driver/src/conversion/rustc/common.rs @@ -9,7 +9,7 @@ use marker_api::{ use rustc_hir as hir; use rustc_middle as mid; -use crate::conversion::common::{BodyIdLayout, DefIdInfo, DefIdLayout, ExpnIdLayout, HirIdLayout}; +use crate::conversion::common::{BodyIdLayout, DefIdInfo, DefIdLayout, DriverTyIdLayout, ExpnIdLayout, HirIdLayout}; use crate::transmute_id; use super::RustcConverter; @@ -166,10 +166,14 @@ impl<'ast, 'tcx> RustcConverter<'ast, 'tcx> { } #[must_use] - pub fn to_mid_ty(&self, id: DriverTyId) -> mid::ty::Ty<'tcx> { + pub fn to_mid_ty(&self, id: DriverTyId) -> (mid::ty::Ty<'tcx>, hir::def_id::DefId) { // FIXME(xFrednet): This is theoretically unsound, but should be fine. // See comment in `MarkerConverterInner::to_driver_ty_id` - transmute_id!(DriverTyId as mid::ty::Ty<'tcx> = id) + let layout = transmute_id!(DriverTyId as DriverTyIdLayout = id); + ( + transmute_id!(u64 as mid::ty::Ty<'tcx> = layout.rustc_ty), + self.to_def_id(layout.environment), + ) } #[must_use] diff --git a/marker_uilints/src/sem.rs b/marker_uilints/src/sem.rs index ab031f1d..b29ecd00 100644 --- a/marker_uilints/src/sem.rs +++ b/marker_uilints/src/sem.rs @@ -20,18 +20,27 @@ fn test_ty_impls_trait<'ast>(cx: &'ast MarkerContext<'ast>, input_expr: ExprKind cx.emit_lint(super::TEST_TY_IMPLS_TRAIT, expr, "checking trait impls:") .decorate(|diag| { let ty: sem::TyKind<'_> = expr.ty(); - test_implements_trait(diag, ty, "std::marker::Sized"); - test_implements_trait(diag, ty, "std::marker::Send"); - test_implements_trait(diag, ty, "std::clone::Clone"); - test_implements_trait(diag, ty, "std::default::Default"); - test_implements_trait(diag, ty, "unknown::Trait"); + test_implements_trait(diag, ty, "std::marker::Sized", ""); + test_implements_trait(diag, ty, "std::marker::Send", ""); + test_implements_trait(diag, ty, "std::clone::Clone", ""); + test_implements_trait(diag, ty, "std::default::Default", ""); + test_implements_trait(diag, ty, "std::cmp::Ord", ""); + test_implements_trait(diag, ty, "unknown::Trait", "Should always be false"); + test_implements_trait(diag, ty, "crate::SimpleTestTrait", ""); + test_implements_trait(diag, ty, "crate::GenericTestTrait", "Path without generics"); + test_implements_trait(diag, ty, "crate::AssocTyTestTrait", "Path without type"); }); } -fn test_implements_trait(diag: &mut DiagnosticBuilder<'_>, ty: sem::TyKind<'_>, path: impl Into) { +fn test_implements_trait( + diag: &mut DiagnosticBuilder<'_>, + ty: sem::TyKind<'_>, + path: impl Into, + comment: &str, +) { let path = path.into(); diag.note(format!( - "Implements: `{path}`: {}", + "Implements: `{path}`: {} ({comment})", ty.implements_trait(&UserDefinedTraitRef::new(path.clone())) )); } diff --git a/marker_uilints/tests/ui/expr/ty_impls_trait.rs b/marker_uilints/tests/ui/expr/ty_impls_trait.rs index e95e1afc..b98116ac 100644 --- a/marker_uilints/tests/ui/expr/ty_impls_trait.rs +++ b/marker_uilints/tests/ui/expr/ty_impls_trait.rs @@ -1,9 +1,31 @@ #![warn(marker::marker_uilints::test_ty_impls_trait)] +use std::fmt::Debug; + +trait SimpleTestTrait {} +impl SimpleTestTrait for String {} + +trait GenericTestTrait {} +impl GenericTestTrait for String {} + +trait AssocTyTestTrait { + type Assoc: Debug; +} +impl AssocTyTestTrait for String { + type Assoc=i32; +} fn check_generic(check_traits_from_generic: &T) { let _ = check_traits_from_generic.clone(); } +fn check_more_generics(check_traits_ord: T, y: T, z: T) { + if check_traits_ord > y { + todo!("a"); + } else if z < y { + todo!("b"); + } +} + fn return_impl_clone() -> impl Clone { "I'm a beautiful string, that implements `Clone`" } @@ -14,6 +36,9 @@ fn check_impl_ret() { } fn main() { - let check_traits = 1; - let _ = check_traits; + let check_traits_i32 = 1; + let _ = check_traits_i32; + + let check_traits_string = String::new(); + let _ = check_traits_string; } \ No newline at end of file diff --git a/marker_uilints/tests/ui/expr/ty_impls_trait.stderr b/marker_uilints/tests/ui/expr/ty_impls_trait.stderr index 052c407a..223cea0c 100644 --- a/marker_uilints/tests/ui/expr/ty_impls_trait.stderr +++ b/marker_uilints/tests/ui/expr/ty_impls_trait.stderr @@ -1,43 +1,87 @@ warning: checking trait impls: - --> $DIR/ty_impls_trait.rs:4:13 - | -4 | let _ = check_traits_from_generic.clone(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: Implements: `std::marker::Sized`: true - = note: Implements: `std::marker::Send`: false - = note: Implements: `std::clone::Clone`: true - = note: Implements: `std::default::Default`: false - = note: Implements: `unknown::Trait`: false + --> $DIR/ty_impls_trait.rs:18:13 + | +18 | let _ = check_traits_from_generic.clone(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: Implements: `std::marker::Sized`: true () + = note: Implements: `std::marker::Send`: false () + = note: Implements: `std::clone::Clone`: true () + = note: Implements: `std::default::Default`: false () + = note: Implements: `std::cmp::Ord`: false () + = note: Implements: `unknown::Trait`: false (Should always be false) + = note: Implements: `crate::SimpleTestTrait`: false () + = note: Implements: `crate::GenericTestTrait`: false (Path without generics) + = note: Implements: `crate::AssocTyTestTrait`: false (Path without type) note: the lint level is defined here - --> $DIR/ty_impls_trait.rs:1:9 - | -1 | #![warn(marker::marker_uilints::test_ty_impls_trait)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + --> $DIR/ty_impls_trait.rs:1:9 + | +1 | #![warn(marker::marker_uilints::test_ty_impls_trait)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ warning: checking trait impls: - --> $DIR/ty_impls_trait.rs:13:13 + --> $DIR/ty_impls_trait.rs:22:8 | -13 | let _ = check_traits_for_impl_clone; +22 | if check_traits_ord > y { + | ^^^^^^^^^^^^^^^^ + | + = note: Implements: `std::marker::Sized`: true () + = note: Implements: `std::marker::Send`: false () + = note: Implements: `std::clone::Clone`: false () + = note: Implements: `std::default::Default`: false () + = note: Implements: `std::cmp::Ord`: true () + = note: Implements: `unknown::Trait`: false (Should always be false) + = note: Implements: `crate::SimpleTestTrait`: false () + = note: Implements: `crate::GenericTestTrait`: false (Path without generics) + = note: Implements: `crate::AssocTyTestTrait`: false (Path without type) + +warning: checking trait impls: + --> $DIR/ty_impls_trait.rs:35:13 + | +35 | let _ = check_traits_for_impl_clone; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: Implements: `std::marker::Sized`: true - = note: Implements: `std::marker::Send`: true - = note: Implements: `std::clone::Clone`: true - = note: Implements: `std::default::Default`: false - = note: Implements: `unknown::Trait`: false + = note: Implements: `std::marker::Sized`: true () + = note: Implements: `std::marker::Send`: true () + = note: Implements: `std::clone::Clone`: true () + = note: Implements: `std::default::Default`: false () + = note: Implements: `std::cmp::Ord`: false () + = note: Implements: `unknown::Trait`: false (Should always be false) + = note: Implements: `crate::SimpleTestTrait`: false () + = note: Implements: `crate::GenericTestTrait`: false (Path without generics) + = note: Implements: `crate::AssocTyTestTrait`: false (Path without type) warning: checking trait impls: - --> $DIR/ty_impls_trait.rs:18:13 + --> $DIR/ty_impls_trait.rs:40:13 + | +40 | let _ = check_traits_i32; + | ^^^^^^^^^^^^^^^^ + | + = note: Implements: `std::marker::Sized`: true () + = note: Implements: `std::marker::Send`: true () + = note: Implements: `std::clone::Clone`: true () + = note: Implements: `std::default::Default`: true () + = note: Implements: `std::cmp::Ord`: true () + = note: Implements: `unknown::Trait`: false (Should always be false) + = note: Implements: `crate::SimpleTestTrait`: false () + = note: Implements: `crate::GenericTestTrait`: false (Path without generics) + = note: Implements: `crate::AssocTyTestTrait`: false (Path without type) + +warning: checking trait impls: + --> $DIR/ty_impls_trait.rs:43:13 | -18 | let _ = check_traits; - | ^^^^^^^^^^^^ +43 | let _ = check_traits_string; + | ^^^^^^^^^^^^^^^^^^^ | - = note: Implements: `std::marker::Sized`: true - = note: Implements: `std::marker::Send`: true - = note: Implements: `std::clone::Clone`: true - = note: Implements: `std::default::Default`: true - = note: Implements: `unknown::Trait`: false + = note: Implements: `std::marker::Sized`: true () + = note: Implements: `std::marker::Send`: true () + = note: Implements: `std::clone::Clone`: true () + = note: Implements: `std::default::Default`: true () + = note: Implements: `std::cmp::Ord`: true () + = note: Implements: `unknown::Trait`: false (Should always be false) + = note: Implements: `crate::SimpleTestTrait`: true () + = note: Implements: `crate::GenericTestTrait`: true (Path without generics) + = note: Implements: `crate::AssocTyTestTrait`: true (Path without type) -warning: 3 warnings emitted +warning: 5 warnings emitted From 1f872f3f0fb26c16b9bc867346a66c503ed5d341 Mon Sep 17 00:00:00 2001 From: xFrednet Date: Fri, 1 Dec 2023 22:27:47 +0100 Subject: [PATCH 4/5] API: Support specifying generics for trait testing --- marker_rustc_driver/src/context.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/marker_rustc_driver/src/context.rs b/marker_rustc_driver/src/context.rs index cc484611..eb43851a 100644 --- a/marker_rustc_driver/src/context.rs +++ b/marker_rustc_driver/src/context.rs @@ -226,6 +226,8 @@ impl<'ast, 'tcx: 'ast> MarkerContextDriver<'ast> for RustcContext<'ast, 'tcx> { // TODO Handle generic arguments let test_ref = mid::ty::TraitRef::new(self.rustc_cx, id, std::iter::once(mid::ty::GenericArg::from(ty))); + // Generic arg creation: + // `From>` self.check_implements_trait(ty, test_ref, self.rustc_cx.param_env(param_env_src)) }) } From 2f2b157489da244b80d2e6729b1c35f41f93dd8d Mon Sep 17 00:00:00 2001 From: xFrednet Date: Fri, 8 Dec 2023 14:32:34 +0100 Subject: [PATCH 5/5] Doc: Documentation is fun, especially at a todo and unstable stage --- marker_adapter/src/context.rs | 4 +- marker_api/src/context.rs | 8 +- marker_api/src/sem/ty.rs | 170 ++++++++++++++++-- marker_rustc_driver/src/context.rs | 9 +- marker_uilints/src/sem.rs | 4 +- .../tests/ui/expr/ty_impls_trait.rs | 2 +- 6 files changed, 164 insertions(+), 33 deletions(-) diff --git a/marker_adapter/src/context.rs b/marker_adapter/src/context.rs index eeff7deb..03b47cfb 100644 --- a/marker_adapter/src/context.rs +++ b/marker_adapter/src/context.rs @@ -146,7 +146,7 @@ pub trait MarkerContextDriver<'ast> { fn resolve_ty_ids(&'ast self, path: &str) -> &'ast [TyDefId]; fn expr_ty(&'ast self, expr: ExprId) -> marker_api::sem::TyKind<'ast>; - fn ty_implements_trait(&'ast self, ty: sem::TyKind<'ast>, trait_ref: &sem::FfiUserDefinedTraitRef<'_>) -> bool; + fn ty_implements_trait(&'ast self, ty: sem::TyKind<'ast>, trait_ref: &sem::FfiTestTraitRef<'_>) -> bool; fn span(&'ast self, owner: SpanId) -> &'ast Span<'ast>; fn span_snippet(&'ast self, span: &Span<'_>) -> Option<&'ast str>; fn span_source(&'ast self, span: &Span<'_>) -> SpanSource<'ast>; @@ -178,7 +178,7 @@ extern "C" fn expr_ty<'ast>(data: &'ast MarkerContextData, expr: ExprId) -> mark extern "C" fn ty_implements_trait<'ast>( data: &'ast MarkerContextData, ty: sem::TyKind<'ast>, - trait_ref: &sem::FfiUserDefinedTraitRef<'_>, + trait_ref: &sem::FfiTestTraitRef<'_>, ) -> bool { unsafe { as_driver(data) }.ty_implements_trait(ty, trait_ref) } diff --git a/marker_api/src/context.rs b/marker_api/src/context.rs index fcd48b38..e87e6804 100644 --- a/marker_api/src/context.rs +++ b/marker_api/src/context.rs @@ -259,11 +259,7 @@ impl<'ast> MarkerContext<'ast> { pub(crate) fn expr_ty(&self, expr: ExprId) -> TyKind<'ast> { self.callbacks.call_expr_ty(expr) } - pub(crate) fn ty_implements_trait( - &self, - ty: sem::TyKind<'ast>, - trait_ref: &sem::FfiUserDefinedTraitRef<'_>, - ) -> bool { + pub(crate) fn ty_implements_trait(&self, ty: sem::TyKind<'ast>, trait_ref: &sem::FfiTestTraitRef<'_>) -> bool { (self.callbacks.ty_implements_trait)(self.callbacks.data, ty, trait_ref) } @@ -330,7 +326,7 @@ struct MarkerContextCallbacks<'ast> { // Internal utility pub expr_ty: extern "C" fn(&'ast MarkerContextData, ExprId) -> TyKind<'ast>, pub ty_implements_trait: - extern "C" fn(&'ast MarkerContextData, sem::TyKind<'ast>, &sem::FfiUserDefinedTraitRef<'_>) -> bool, + extern "C" fn(&'ast MarkerContextData, sem::TyKind<'ast>, &sem::FfiTestTraitRef<'_>) -> bool, pub span: extern "C" fn(&'ast MarkerContextData, SpanId) -> &'ast Span<'ast>, pub span_snippet: extern "C" fn(&'ast MarkerContextData, &Span<'ast>) -> ffi::FfiOption>, pub span_source: extern "C" fn(&'ast MarkerContextData, &Span<'_>) -> SpanSource<'ast>, diff --git a/marker_api/src/sem/ty.rs b/marker_api/src/sem/ty.rs index 979e1b13..582ae79d 100644 --- a/marker_api/src/sem/ty.rs +++ b/marker_api/src/sem/ty.rs @@ -118,14 +118,23 @@ impl<'ast> TyKind<'ast> { ty } + /// This function tests if a given type implements a trait, specified by + /// the given [`TestTraitRef`]. A [`TraitTestMode`] can be specified as + /// part of the trait reference. #[must_use] - pub fn implements_trait(self, trait_ref: &UserDefinedTraitRef) -> bool { - let ids = match &trait_ref.trait_ref { - TyDefIdSource::Path(path) => with_cx(&self, |cx| cx.resolve_ty_ids(path)), - TyDefIdSource::Id(_id) => todo!(), + pub fn implements_trait(self, trait_ref: &TestTraitRef) -> bool { + let check_with_ids = |ids: &[TyDefId]| { + let ffi = FfiTestTraitRef { + trait_ids: ids.into(), + mode: trait_ref.mode, + }; + with_cx(&self, |cx| cx.ty_implements_trait(self, &ffi)) }; - let ffi = FfiUserDefinedTraitRef { trait_ids: ids.into() }; - with_cx(&self, |cx| cx.ty_implements_trait(self, &ffi)) + + match &trait_ref.trait_ref { + TyDefIdSource::Path(path) => check_with_ids(with_cx(&self, |cx| cx.resolve_ty_ids(path))), + TyDefIdSource::Id(id) => check_with_ids(&[*id]), + } } } @@ -195,37 +204,78 @@ macro_rules! impl_ty_data { } use impl_ty_data; -// TODO docs -// TODO Use a better name +/// This struct describes a trait and its generic arguments. It is used to check +/// if a semantic type implements a trait. +/// +/// See [`TyKind::implements_trait`]. #[derive(Debug)] -pub struct UserDefinedTraitRef { - #[allow(dead_code)] +pub struct TestTraitRef { trait_ref: TyDefIdSource, // TODO generics + mode: TraitTestMode, +} + +impl TestTraitRef { + pub fn builder() -> TestTraitRefBuilder { + TestTraitRefBuilder::new() + } } -impl UserDefinedTraitRef { - // TODO Decide how to construct this object, probably a builder since it's user exposed - pub fn new(trait_ref: impl Into) -> Self { +/// A builder used to construct a [`TestTraitRef`] instance. +#[derive(Debug)] +pub struct TestTraitRefBuilder { + trait_ref: Option, + mode: TraitTestMode, +} + +impl TestTraitRefBuilder { + fn new() -> Self { Self { - trait_ref: trait_ref.into(), + trait_ref: None, + mode: TraitTestMode::default(), + } + } + + pub fn trait_from_path(&mut self, path: impl Into) -> &mut Self { + self.trait_ref = Some(TyDefIdSource::Path(path.into())); + self + } + + pub fn trait_from_id(&mut self, id: impl Into) -> &mut Self { + self.trait_ref = Some(TyDefIdSource::Id(id.into())); + self + } + + pub fn mode(&mut self, mode: TraitTestMode) -> &mut Self { + self.mode = mode; + self + } + + pub fn build(&mut self) -> TestTraitRef { + TestTraitRef { + trait_ref: self.trait_ref.take().expect("the trait was never set"), + mode: self.mode, } } } #[derive(Debug)] #[cfg_attr(feature = "driver-api", visibility::make(pub))] -pub(crate) struct FfiUserDefinedTraitRef<'a> { - #[allow(dead_code)] +pub(crate) struct FfiTestTraitRef<'a> { trait_ids: FfiSlice<'a, TyDefId>, // TODO generics + mode: TraitTestMode, } #[cfg(feature = "driver-api")] -impl<'a> FfiUserDefinedTraitRef<'a> { +impl<'a> FfiTestTraitRef<'a> { pub fn trait_ids(&self) -> &'a [TyDefId] { self.trait_ids.get() } + + pub fn mode(&self) -> TraitTestMode { + self.mode + } } #[derive(Debug)] @@ -247,3 +297,89 @@ impl From for TyDefIdSource { TyDefIdSource::Path(value) } } + +/// This enum defines how strict the [`TyKind::implements_trait`] test should be. +/// The difference is probably best illustrated by examples. For that, let's +/// take the following traits: +/// +/// ``` +/// // Definitions +/// trait SimpleTrait { /* ... */ } +/// trait GenericTrait { /* ... */ } +/// trait TwoGenericTrait { /* ... */ } +/// ``` +/// +/// Now we have a `SimpleType` which implements a few traits: +/// +/// ``` +/// # trait SimpleTrait { /* ... */ } +/// # trait GenericTrait { /* ... */ } +/// # trait TwoGenericTrait { /* ... */ } +/// struct SimpleType; +/// +/// impl SimpleTrait for SimpleType {} +/// impl GenericTrait for SimpleType {} +/// // The second generic argument, defaults to `u8`. +/// impl TwoGenericTrait for SimpleType {} +/// ``` +/// +/// | Semantic type and checked trait | Lossy | Strict | +/// | --------------------------------------------------------- | ----- | ------ | +/// | `SimpleType` implements `SimpleTrait` | ✅ | ✅ | +/// | `SimpleType` implements `GenericTrait` | ✅ | ❌ | +/// | `SimpleType` implements `GenericTrait` | ❌ | ❌ | +/// | `SimpleType` implements `TwoGenericTrait` | ✅ | ❌ | +/// | `SimpleType` implements `TwoGenericTrait` | ✅ | ✅ | +/// | `SimpleType` implements `TwoGenericTrait` | ❌ | ❌ | +/// +/// We can also check traits for types with generic parameter: +/// +/// ``` +/// # trait SimpleTrait { /* ... */ } +/// # trait GenericTrait { /* ... */ } +/// # trait TwoGenericTrait { /* ... */ } +/// struct GenericType(T); +/// +/// impl SimpleTrait for GenericType {} +/// impl GenericTrait for GenericType {} +/// impl TwoGenericTrait for GenericType {} +/// ``` +/// | Semantic type and checked trait | Lossy | Strict | +/// | --------------------------------------------------------- | ----- | ------ | +/// | `GenericType<()>` implements `SimpleTrait` | ✅ | ✅ | +/// | `GenericType` implements `GenericTrait` | ❌ | ❌ | +/// | `GenericType` implements `GenericTrait` | ✅ | ✅ | +/// | `GenericType` implements `TwoGenericTrait` | ✅ | ❌ | +/// | `GenericType` implements `TwoGenericTrait` | ✅ | ❌ | +/// | `GenericType` implements `TwoGenericTrait` | ✅ | ✅ | +#[non_exhaustive] +#[derive(Debug, Copy, Clone, Default)] +pub enum TraitTestMode { + /// The comparison will enforce the given generic arguments and allow any + /// additional ones to be freely matched. This is useful for traits with + /// default types for generic parameters. + /// + /// See the documentation of [`TraitTestMode`] for a comparison of the + /// different test modes. + #[default] + Lossy, + /// The comparison will check that the correct number of generics has been + /// provided and will check that all of them match. + /// + /// See the documentation of [`TraitTestMode`] for a comparison of the + /// different test modes. + Strict, +} + +pub struct UserDefinedGeneric<'a> { + _lifetime: PhantomData<&'a ()>, +} + +#[derive(Debug)] +#[non_exhaustive] +#[cfg_attr(feature = "driver-api", visibility::make(pub))] +enum UserDefinedGenericInner<'a> { + ApiType(TyKind<'a>), + // TODO: Handle the path properly, since `Str` is not ABI safe + Path(String), +} diff --git a/marker_rustc_driver/src/context.rs b/marker_rustc_driver/src/context.rs index eb43851a..8bda466e 100644 --- a/marker_rustc_driver/src/context.rs +++ b/marker_rustc_driver/src/context.rs @@ -197,11 +197,10 @@ impl<'ast, 'tcx: 'ast> MarkerContextDriver<'ast> for RustcContext<'ast, 'tcx> { self.marker_converter.expr_ty(hir_id) } - fn ty_implements_trait(&'ast self, api_ty: sem::TyKind<'ast>, trait_ref: &sem::FfiUserDefinedTraitRef<'_>) -> bool { + fn ty_implements_trait(&'ast self, api_ty: sem::TyKind<'ast>, trait_ref: &sem::FfiTestTraitRef<'_>) -> bool { use rustc_middle::ty::TypeVisitableExt; - // // TODO: This might be a hack, try what happens with too many generics - // mid::ty::ParamEnv::empty() + // TODO: Implement test mode let (ty, param_env_src) = self.rustc_converter.to_mid_ty(api_ty.data().driver_id()); let ty = self.rustc_cx.erase_regions(ty); #[allow(clippy::manual_assert)] @@ -226,8 +225,8 @@ impl<'ast, 'tcx: 'ast> MarkerContextDriver<'ast> for RustcContext<'ast, 'tcx> { // TODO Handle generic arguments let test_ref = mid::ty::TraitRef::new(self.rustc_cx, id, std::iter::once(mid::ty::GenericArg::from(ty))); - // Generic arg creation: - // `From>` + // Generic arg creation: + // `From>` self.check_implements_trait(ty, test_ref, self.rustc_cx.param_env(param_env_src)) }) } diff --git a/marker_uilints/src/sem.rs b/marker_uilints/src/sem.rs index b29ecd00..b3e2db35 100644 --- a/marker_uilints/src/sem.rs +++ b/marker_uilints/src/sem.rs @@ -1,4 +1,4 @@ -use marker_api::{diagnostic::DiagnosticBuilder, prelude::*, sem::UserDefinedTraitRef}; +use marker_api::{diagnostic::DiagnosticBuilder, prelude::*, sem::TestTraitRef}; pub fn check_expr<'ast>(cx: &'ast MarkerContext<'ast>, expr: ExprKind<'ast>) { test_ty_impls_trait(cx, expr); @@ -41,6 +41,6 @@ fn test_implements_trait( let path = path.into(); diag.note(format!( "Implements: `{path}`: {} ({comment})", - ty.implements_trait(&UserDefinedTraitRef::new(path.clone())) + ty.implements_trait(&TestTraitRef::builder().trait_from_path(path.clone()).build()) )); } diff --git a/marker_uilints/tests/ui/expr/ty_impls_trait.rs b/marker_uilints/tests/ui/expr/ty_impls_trait.rs index b98116ac..e9c388bb 100644 --- a/marker_uilints/tests/ui/expr/ty_impls_trait.rs +++ b/marker_uilints/tests/ui/expr/ty_impls_trait.rs @@ -41,4 +41,4 @@ fn main() { let check_traits_string = String::new(); let _ = check_traits_string; -} \ No newline at end of file +}