From 143203b7133a810c068e44c657fc6c5ee978023b Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Fri, 8 Dec 2023 10:47:36 +0100 Subject: [PATCH 1/2] Make TraitEnvironment's constructor private --- crates/hir-ty/src/consteval.rs | 2 +- crates/hir-ty/src/display.rs | 5 ++--- crates/hir-ty/src/infer/expr.rs | 6 ++--- crates/hir-ty/src/lib.rs | 2 +- crates/hir-ty/src/lower.rs | 9 ++------ crates/hir-ty/src/mir.rs | 3 +-- crates/hir-ty/src/traits.rs | 22 +++++++++++++++---- crates/hir/src/lib.rs | 39 +++++++++++++++------------------ 8 files changed, 46 insertions(+), 42 deletions(-) diff --git a/crates/hir-ty/src/consteval.rs b/crates/hir-ty/src/consteval.rs index ddeb9f14b5dc7..9792d945eb8f5 100644 --- a/crates/hir-ty/src/consteval.rs +++ b/crates/hir-ty/src/consteval.rs @@ -137,7 +137,7 @@ pub fn intern_const_ref( ty: Ty, krate: CrateId, ) -> Const { - let layout = db.layout_of_ty(ty.clone(), Arc::new(TraitEnvironment::empty(krate))); + let layout = db.layout_of_ty(ty.clone(), TraitEnvironment::empty(krate)); let bytes = match value { LiteralConstRef::Int(i) => { // FIXME: We should handle failure of layout better. diff --git a/crates/hir-ty/src/display.rs b/crates/hir-ty/src/display.rs index a324129b351cc..d81926f7c9762 100644 --- a/crates/hir-ty/src/display.rs +++ b/crates/hir-ty/src/display.rs @@ -448,9 +448,8 @@ fn render_const_scalar( ) -> Result<(), HirDisplayError> { // FIXME: We need to get krate from the final callers of the hir display // infrastructure and have it here as a field on `f`. - let trait_env = Arc::new(TraitEnvironment::empty( - *f.db.crate_graph().crates_in_topological_order().last().unwrap(), - )); + let trait_env = + TraitEnvironment::empty(*f.db.crate_graph().crates_in_topological_order().last().unwrap()); match ty.kind(Interner) { TyKind::Scalar(s) => match s { Scalar::Bool => write!(f, "{}", if b[0] == 0 { false } else { true }), diff --git a/crates/hir-ty/src/infer/expr.rs b/crates/hir-ty/src/infer/expr.rs index 0c3c725a7c743..24026202b74d6 100644 --- a/crates/hir-ty/src/infer/expr.rs +++ b/crates/hir-ty/src/infer/expr.rs @@ -18,7 +18,6 @@ use hir_def::{ use hir_expand::name::{name, Name}; use stdx::always; use syntax::ast::RangeOp; -use triomphe::Arc; use crate::{ autoderef::{builtin_deref, deref_by_trait, Autoderef}, @@ -40,7 +39,8 @@ use crate::{ traits::FnTrait, utils::{generics, Generics}, Adjust, Adjustment, AdtId, AutoBorrow, Binders, CallableDefId, FnPointer, FnSig, FnSubst, - Interner, Rawness, Scalar, Substitution, TraitRef, Ty, TyBuilder, TyExt, TyKind, + Interner, Rawness, Scalar, Substitution, TraitEnvironment, TraitRef, Ty, TyBuilder, TyExt, + TyKind, }; use super::{ @@ -1291,7 +1291,7 @@ impl InferenceContext<'_> { let g = self.resolver.update_to_inner_scope(self.db.upcast(), self.owner, expr); let prev_env = block_id.map(|block_id| { let prev_env = self.table.trait_env.clone(); - Arc::make_mut(&mut self.table.trait_env).block = Some(block_id); + TraitEnvironment::with_block(&mut self.table.trait_env, block_id); prev_env }); diff --git a/crates/hir-ty/src/lib.rs b/crates/hir-ty/src/lib.rs index 5a3e423f152ae..cf174feed24b8 100644 --- a/crates/hir-ty/src/lib.rs +++ b/crates/hir-ty/src/lib.rs @@ -122,7 +122,7 @@ pub type TyKind = chalk_ir::TyKind; pub type TypeFlags = chalk_ir::TypeFlags; pub type DynTy = chalk_ir::DynTy; pub type FnPointer = chalk_ir::FnPointer; -// pub type FnSubst = chalk_ir::FnSubst; +// pub type FnSubst = chalk_ir::FnSubst; // a re-export so we don't lose the tuple constructor pub use chalk_ir::FnSubst; pub type ProjectionTy = chalk_ir::ProjectionTy; pub type AliasTy = chalk_ir::AliasTy; diff --git a/crates/hir-ty/src/lower.rs b/crates/hir-ty/src/lower.rs index 2a6d69e7fc624..c86fe9adff866 100644 --- a/crates/hir-ty/src/lower.rs +++ b/crates/hir-ty/src/lower.rs @@ -1468,7 +1468,7 @@ pub(crate) fn trait_environment_for_body_query( ) -> Arc { let Some(def) = def.as_generic_def_id() else { let krate = def.module(db.upcast()).krate(); - return Arc::new(TraitEnvironment::empty(krate)); + return TraitEnvironment::empty(krate); }; db.trait_environment(def) } @@ -1528,12 +1528,7 @@ pub(crate) fn trait_environment_query( let env = chalk_ir::Environment::new(Interner).add_clauses(Interner, clauses); - Arc::new(TraitEnvironment { - krate, - block: None, - traits_from_clauses: traits_in_scope.into_boxed_slice(), - env, - }) + TraitEnvironment::new(krate, None, traits_in_scope.into_boxed_slice(), env) } /// Resolve the where clause(s) of an item with generics. diff --git a/crates/hir-ty/src/mir.rs b/crates/hir-ty/src/mir.rs index 2e6fe59d3bd80..f1795e71d945c 100644 --- a/crates/hir-ty/src/mir.rs +++ b/crates/hir-ty/src/mir.rs @@ -40,7 +40,6 @@ pub use monomorphization::{ use rustc_hash::FxHashMap; use smallvec::{smallvec, SmallVec}; use stdx::{impl_from, never}; -use triomphe::Arc; use super::consteval::{intern_const_scalar, try_const_usize}; @@ -147,7 +146,7 @@ impl ProjectionElem { base = normalize( db, // FIXME: we should get this from caller - Arc::new(TraitEnvironment::empty(krate)), + TraitEnvironment::empty(krate), base, ); } diff --git a/crates/hir-ty/src/traits.rs b/crates/hir-ty/src/traits.rs index 467b94a2662a1..b6bc76bc98d53 100644 --- a/crates/hir-ty/src/traits.rs +++ b/crates/hir-ty/src/traits.rs @@ -48,18 +48,32 @@ pub struct TraitEnvironment { pub krate: CrateId, pub block: Option, // FIXME make this a BTreeMap - pub(crate) traits_from_clauses: Box<[(Ty, TraitId)]>, + traits_from_clauses: Box<[(Ty, TraitId)]>, pub env: chalk_ir::Environment, } impl TraitEnvironment { - pub fn empty(krate: CrateId) -> Self { - TraitEnvironment { + pub fn empty(krate: CrateId) -> Arc { + Arc::new(TraitEnvironment { krate, block: None, traits_from_clauses: Box::default(), env: chalk_ir::Environment::new(Interner), - } + }) + } + + pub fn new( + krate: CrateId, + block: Option, + traits_from_clauses: Box<[(Ty, TraitId)]>, + env: chalk_ir::Environment, + ) -> Arc { + Arc::new(TraitEnvironment { krate, block, traits_from_clauses, env }) + } + + // pub fn with_block(self: &mut Arc, block: BlockId) { + pub fn with_block(this: &mut Arc, block: BlockId) { + Arc::make_mut(this).block = Some(block); } pub fn traits_in_scope_from_clauses(&self, ty: Ty) -> impl Iterator + '_ { diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 5137bff055a4b..ca838c7a51e10 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -3564,10 +3564,9 @@ impl TraitRef { resolver: &Resolver, trait_ref: hir_ty::TraitRef, ) -> TraitRef { - let env = resolver.generic_def().map_or_else( - || Arc::new(TraitEnvironment::empty(resolver.krate())), - |d| db.trait_environment(d), - ); + let env = resolver + .generic_def() + .map_or_else(|| TraitEnvironment::empty(resolver.krate()), |d| db.trait_environment(d)); TraitRef { env, trait_ref } } @@ -3707,15 +3706,14 @@ impl Type { resolver: &Resolver, ty: Ty, ) -> Type { - let environment = resolver.generic_def().map_or_else( - || Arc::new(TraitEnvironment::empty(resolver.krate())), - |d| db.trait_environment(d), - ); + let environment = resolver + .generic_def() + .map_or_else(|| TraitEnvironment::empty(resolver.krate()), |d| db.trait_environment(d)); Type { env: environment, ty } } pub(crate) fn new_for_crate(krate: CrateId, ty: Ty) -> Type { - Type { env: Arc::new(TraitEnvironment::empty(krate)), ty } + Type { env: TraitEnvironment::empty(krate), ty } } pub fn reference(inner: &Type, m: Mutability) -> Type { @@ -3731,10 +3729,9 @@ impl Type { fn new(db: &dyn HirDatabase, lexical_env: impl HasResolver, ty: Ty) -> Type { let resolver = lexical_env.resolver(db.upcast()); - let environment = resolver.generic_def().map_or_else( - || Arc::new(TraitEnvironment::empty(resolver.krate())), - |d| db.trait_environment(d), - ); + let environment = resolver + .generic_def() + .map_or_else(|| TraitEnvironment::empty(resolver.krate()), |d| db.trait_environment(d)); Type { env: environment, ty } } @@ -4304,10 +4301,10 @@ impl Type { let canonical = hir_ty::replace_errors_with_variables(&self.ty); let krate = scope.krate(); - let environment = scope.resolver().generic_def().map_or_else( - || Arc::new(TraitEnvironment::empty(krate.id)), - |d| db.trait_environment(d), - ); + let environment = scope + .resolver() + .generic_def() + .map_or_else(|| TraitEnvironment::empty(krate.id), |d| db.trait_environment(d)); method_resolution::iterate_method_candidates_dyn( &canonical, @@ -4361,10 +4358,10 @@ impl Type { let canonical = hir_ty::replace_errors_with_variables(&self.ty); let krate = scope.krate(); - let environment = scope.resolver().generic_def().map_or_else( - || Arc::new(TraitEnvironment::empty(krate.id)), - |d| db.trait_environment(d), - ); + let environment = scope + .resolver() + .generic_def() + .map_or_else(|| TraitEnvironment::empty(krate.id), |d| db.trait_environment(d)); method_resolution::iterate_path_candidates( &canonical, From 71337f6682c27233d9c7bf9c5e3383da451ba604 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Fri, 8 Dec 2023 11:26:22 +0100 Subject: [PATCH 2/2] fix: Fix `concat_bytes!` expansion --- .../macro_expansion_tests/builtin_fn_macro.rs | 4 +-- .../macro_expansion_tests/mbe/regression.rs | 26 +++++++++++++++++++ crates/hir-expand/src/builtin_fn_macro.rs | 22 ++++++++++++++-- crates/mbe/src/token_map.rs | 16 +++++++++--- 4 files changed, 61 insertions(+), 7 deletions(-) diff --git a/crates/hir-def/src/macro_expansion_tests/builtin_fn_macro.rs b/crates/hir-def/src/macro_expansion_tests/builtin_fn_macro.rs index 106ead83fad76..514219ee71505 100644 --- a/crates/hir-def/src/macro_expansion_tests/builtin_fn_macro.rs +++ b/crates/hir-def/src/macro_expansion_tests/builtin_fn_macro.rs @@ -468,12 +468,12 @@ macro_rules! concat_bytes {} fn main() { concat_bytes!(b'A', b"BC", [68, b'E', 70]); } "##, - expect![[r##" + expect![[r#" #[rustc_builtin_macro] macro_rules! concat_bytes {} fn main() { [b'A', 66, 67, 68, b'E', 70]; } -"##]], +"#]], ); } diff --git a/crates/hir-def/src/macro_expansion_tests/mbe/regression.rs b/crates/hir-def/src/macro_expansion_tests/mbe/regression.rs index 2886b2a366c04..9010050ee6788 100644 --- a/crates/hir-def/src/macro_expansion_tests/mbe/regression.rs +++ b/crates/hir-def/src/macro_expansion_tests/mbe/regression.rs @@ -1004,3 +1004,29 @@ fn main() { "##]], ); } + +#[test] +fn eager_concat_bytes_panic() { + check( + r#" +#[rustc_builtin_macro] +#[macro_export] +macro_rules! concat_bytes {} + +fn main() { + let x = concat_bytes!(2); +} + +"#, + expect![[r#" +#[rustc_builtin_macro] +#[macro_export] +macro_rules! concat_bytes {} + +fn main() { + let x = /* error: unexpected token in input */[]; +} + +"#]], + ); +} diff --git a/crates/hir-expand/src/builtin_fn_macro.rs b/crates/hir-expand/src/builtin_fn_macro.rs index 903c21c84eec9..c8f04bfee54f1 100644 --- a/crates/hir-expand/src/builtin_fn_macro.rs +++ b/crates/hir-expand/src/builtin_fn_macro.rs @@ -6,6 +6,7 @@ use base_db::{ }; use cfg::CfgExpr; use either::Either; +use itertools::Itertools; use mbe::{parse_exprs_with_sep, parse_to_token_tree}; use syntax::{ ast::{self, AstToken}, @@ -491,8 +492,25 @@ fn concat_bytes_expand( } } } - let ident = tt::Ident { text: bytes.join(", ").into(), span }; - ExpandResult { value: quote!(span =>[#ident]), err } + let value = tt::Subtree { + delimiter: tt::Delimiter { open: span, close: span, kind: tt::DelimiterKind::Bracket }, + token_trees: { + Itertools::intersperse_with( + bytes.into_iter().map(|it| { + tt::TokenTree::Leaf(tt::Leaf::Literal(tt::Literal { text: it.into(), span })) + }), + || { + tt::TokenTree::Leaf(tt::Leaf::Punct(tt::Punct { + char: ',', + spacing: tt::Spacing::Alone, + span, + })) + }, + ) + .collect() + }, + }; + ExpandResult { value, err } } fn concat_bytes_expand_subtree( diff --git a/crates/mbe/src/token_map.rs b/crates/mbe/src/token_map.rs index 28b39b4f1eec4..7d15812f8cb60 100644 --- a/crates/mbe/src/token_map.rs +++ b/crates/mbe/src/token_map.rs @@ -2,7 +2,7 @@ use std::hash::Hash; -use stdx::itertools::Itertools; +use stdx::{always, itertools::Itertools}; use syntax::{TextRange, TextSize}; use tt::Span; @@ -21,13 +21,23 @@ impl SpanMap { /// Finalizes the [`SpanMap`], shrinking its backing storage and validating that the offsets are /// in order. pub fn finish(&mut self) { - assert!(self.spans.iter().tuple_windows().all(|(a, b)| a.0 < b.0)); + always!( + self.spans.iter().tuple_windows().all(|(a, b)| a.0 < b.0), + "spans are not in order" + ); self.spans.shrink_to_fit(); } /// Pushes a new span onto the [`SpanMap`]. pub fn push(&mut self, offset: TextSize, span: S) { - debug_assert!(self.spans.last().map_or(true, |&(last_offset, _)| last_offset < offset)); + if cfg!(debug_assertions) { + if let Some(&(last_offset, _)) = self.spans.last() { + assert!( + last_offset < offset, + "last_offset({last_offset:?}) must be smaller than offset({offset:?})" + ); + } + } self.spans.push((offset, span)); }