Merge pull request #20642 from ChayimFriedman2/wasm-safe

fix: Make `#[target_feature]` always safe on WASM

Author: ShoyuVanilla

crates/base-db/src/input.rs

@@ -295,8 +295,6 @@ impl CrateDisplayName {
     }
 }
 
-pub type TargetLayoutLoadResult = Result<Arc<str>, Arc<str>>;
-
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
 pub enum ReleaseChannel {
     Stable,
@@ -929,7 +927,7 @@ mod tests {
     use super::{CrateGraphBuilder, CrateName, CrateOrigin, Edition::Edition2018, Env, FileId};
 
     fn empty_ws_data() -> Arc<CrateWorkspaceData> {
-        Arc::new(CrateWorkspaceData { data_layout: Err("".into()), toolchain: None })
+        Arc::new(CrateWorkspaceData { target: Err("".into()), toolchain: None })
     }
 
     #[test]

crates/base-db/src/lib.rs

@@ -6,6 +6,7 @@ pub use salsa_macros;
 // FIXME: Rename this crate, base db is non descriptive
 mod change;
 mod input;
+pub mod target;
 
 use std::{
     cell::RefCell,
@@ -20,8 +21,7 @@ pub use crate::{
         BuiltCrateData, BuiltDependency, Crate, CrateBuilder, CrateBuilderId, CrateDataBuilder,
         CrateDisplayName, CrateGraphBuilder, CrateName, CrateOrigin, CratesIdMap, CratesMap,
         DependencyBuilder, Env, ExtraCrateData, LangCrateOrigin, ProcMacroLoadingError,
-        ProcMacroPaths, ReleaseChannel, SourceRoot, SourceRootId, TargetLayoutLoadResult,
-        UniqueCrateData,
+        ProcMacroPaths, ReleaseChannel, SourceRoot, SourceRootId, UniqueCrateData,
     },
 };
 use dashmap::{DashMap, mapref::entry::Entry};
@@ -359,8 +359,7 @@ impl Nonce {
 /// Crate related data shared by the whole workspace.
 #[derive(Debug, PartialEq, Eq, Hash, Clone)]
 pub struct CrateWorkspaceData {
-    // FIXME: Consider removing this, making HirDatabase::target_data_layout an input query
-    pub data_layout: TargetLayoutLoadResult,
+    pub target: Result<target::TargetData, target::TargetLoadError>,
     /// Toolchain version used to compile the crate.
     pub toolchain: Option<Version>,
 }

crates/base-db/src/target.rs

@@ -0,0 +1,50 @@
+//! Information about the target.
+
+use std::fmt;
+
+use triomphe::Arc;
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub enum Arch {
+    // Only what we need is present here.
+    Wasm32,
+    Wasm64,
+    Other,
+}
+
+#[derive(Debug, PartialEq, Eq, Hash, Clone)]
+pub struct TargetData {
+    pub data_layout: Box<str>,
+    pub arch: Arch,
+}
+
+#[derive(Clone, PartialEq, Eq, Hash)]
+pub struct TargetLoadError(Arc<str>);
+
+impl fmt::Debug for TargetLoadError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        fmt::Debug::fmt(&self.0, f)
+    }
+}
+
+impl fmt::Display for TargetLoadError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        fmt::Display::fmt(&self.0, f)
+    }
+}
+
+impl std::error::Error for TargetLoadError {}
+
+impl From<String> for TargetLoadError {
+    fn from(value: String) -> Self {
+        Self(value.into())
+    }
+}
+
+impl From<&str> for TargetLoadError {
+    fn from(value: &str) -> Self {
+        Self(value.into())
+    }
+}
+
+pub type TargetLoadResult = Result<TargetData, TargetLoadError>;

crates/hir-def/src/nameres/tests/incremental.rs

@@ -84,7 +84,7 @@ pub const BAZ: u32 = 0;
                     )
                     .unwrap(),
                 ),
-                Arc::new(CrateWorkspaceData { data_layout: Err("".into()), toolchain: None }),
+                Arc::new(CrateWorkspaceData { target: Err("".into()), toolchain: None }),
             )
         };
         let a = add_crate("a", 0);

crates/hir-ty/src/db.rs

@@ -2,6 +2,7 @@
 //! type inference-related queries.
 
 use base_db::Crate;
+use base_db::target::TargetLoadError;
 use hir_def::{
     AdtId, BlockId, CallableDefId, ConstParamId, DefWithBodyId, EnumVariantId, FunctionId,
     GeneralConstId, GenericDefId, ImplId, LifetimeParamId, LocalFieldId, StaticId, TraitId,
@@ -108,7 +109,7 @@ pub trait HirDatabase: DefDatabase + std::fmt::Debug {
     ) -> Result<Arc<Layout>, LayoutError>;
 
     #[salsa::invoke(crate::layout::target_data_layout_query)]
-    fn target_data_layout(&self, krate: Crate) -> Result<Arc<TargetDataLayout>, Arc<str>>;
+    fn target_data_layout(&self, krate: Crate) -> Result<Arc<TargetDataLayout>, TargetLoadError>;
 
     #[salsa::invoke(crate::dyn_compatibility::dyn_compatibility_of_trait_query)]
     fn dyn_compatibility_of_trait(&self, trait_: TraitId) -> Option<DynCompatibilityViolation>;

crates/hir-ty/src/diagnostics/unsafe_check.rs

@@ -15,8 +15,9 @@ use hir_def::{
 use span::Edition;
 
 use crate::{
-    InferenceResult, Interner, TargetFeatures, TyExt, TyKind, db::HirDatabase,
-    utils::is_fn_unsafe_to_call,
+    InferenceResult, Interner, TargetFeatures, TyExt, TyKind,
+    db::HirDatabase,
+    utils::{is_fn_unsafe_to_call, target_feature_is_safe_in_target},
 };
 
 #[derive(Debug, Default)]
@@ -144,6 +145,9 @@ struct UnsafeVisitor<'db> {
     def_target_features: TargetFeatures,
     // FIXME: This needs to be the edition of the span of each call.
     edition: Edition,
+    /// On some targets (WASM), calling safe functions with `#[target_feature]` is always safe, even when
+    /// the target feature is not enabled. This flag encodes that.
+    target_feature_is_safe: bool,
 }
 
 impl<'db> UnsafeVisitor<'db> {
@@ -159,7 +163,12 @@ impl<'db> UnsafeVisitor<'db> {
             DefWithBodyId::FunctionId(func) => TargetFeatures::from_attrs(&db.attrs(func.into())),
             _ => TargetFeatures::default(),
         };
-        let edition = resolver.module().krate().data(db).edition;
+        let krate = resolver.module().krate();
+        let edition = krate.data(db).edition;
+        let target_feature_is_safe = match &krate.workspace_data(db).target {
+            Ok(target) => target_feature_is_safe_in_target(target),
+            Err(_) => false,
+        };
         Self {
             db,
             infer,
@@ -172,6 +181,7 @@ impl<'db> UnsafeVisitor<'db> {
             callback: unsafe_expr_cb,
             def_target_features,
             edition,
+            target_feature_is_safe,
         }
     }
 
@@ -184,7 +194,13 @@ impl<'db> UnsafeVisitor<'db> {
     }
 
     fn check_call(&mut self, node: ExprId, func: FunctionId) {
-        let unsafety = is_fn_unsafe_to_call(self.db, func, &self.def_target_features, self.edition);
+        let unsafety = is_fn_unsafe_to_call(
+            self.db,
+            func,
+            &self.def_target_features,
+            self.edition,
+            self.target_feature_is_safe,
+        );
         match unsafety {
             crate::utils::Unsafety::Safe => {}
             crate::utils::Unsafety::Unsafe => {

crates/hir-ty/src/layout/target.rs

@@ -1,6 +1,6 @@
 //! Target dependent parameters needed for layouts
 
-use base_db::Crate;
+use base_db::{Crate, target::TargetLoadError};
 use hir_def::layout::TargetDataLayout;
 use rustc_abi::{AddressSpace, AlignFromBytesError, TargetDataLayoutErrors};
 use triomphe::Arc;
@@ -10,9 +10,9 @@ use crate::db::HirDatabase;
 pub fn target_data_layout_query(
     db: &dyn HirDatabase,
     krate: Crate,
-) -> Result<Arc<TargetDataLayout>, Arc<str>> {
-    match &krate.workspace_data(db).data_layout {
-        Ok(it) => match TargetDataLayout::parse_from_llvm_datalayout_string(it, AddressSpace::ZERO) {
+) -> Result<Arc<TargetDataLayout>, TargetLoadError> {
+    match &krate.workspace_data(db).target {
+        Ok(target) => match TargetDataLayout::parse_from_llvm_datalayout_string(&target.data_layout, AddressSpace::ZERO) {
             Ok(it) => Ok(Arc::new(it)),
             Err(e) => {
                 Err(match e {

crates/hir-ty/src/layout/tests.rs

@@ -1,3 +1,4 @@
+use base_db::target::TargetData;
 use chalk_ir::{AdtId, TyKind};
 use either::Either;
 use hir_def::db::DefDatabase;
@@ -18,8 +19,8 @@ use crate::{
 
 mod closure;
 
-fn current_machine_data_layout() -> String {
-    project_model::toolchain_info::target_data_layout::get(
+fn current_machine_target_data() -> TargetData {
+    project_model::toolchain_info::target_data::get(
         QueryConfig::Rustc(&Sysroot::empty(), &std::env::current_dir().unwrap()),
         None,
         &FxHashMap::default(),
@@ -32,7 +33,8 @@ fn eval_goal(
     minicore: &str,
 ) -> Result<Arc<Layout>, LayoutError> {
     let _tracing = setup_tracing();
-    let target_data_layout = current_machine_data_layout();
+    let target_data = current_machine_target_data();
+    let target_data_layout = target_data.data_layout;
     let ra_fixture = format!(
         "//- target_data_layout: {target_data_layout}\n{minicore}//- /main.rs crate:test\n{ra_fixture}",
     );
@@ -104,7 +106,8 @@ fn eval_expr(
     minicore: &str,
 ) -> Result<Arc<Layout>, LayoutError> {
     let _tracing = setup_tracing();
-    let target_data_layout = current_machine_data_layout();
+    let target_data = current_machine_target_data();
+    let target_data_layout = target_data.data_layout;
     let ra_fixture = format!(
         "//- target_data_layout: {target_data_layout}\n{minicore}//- /main.rs crate:test\nfn main(){{let goal = {{{ra_fixture}}};}}",
     );

crates/hir-ty/src/lib.rs

@@ -132,7 +132,10 @@ pub use mapping::{
 pub use method_resolution::check_orphan_rules;
 pub use target_feature::TargetFeatures;
 pub use traits::TraitEnvironment;
-pub use utils::{Unsafety, all_super_traits, direct_super_traits, is_fn_unsafe_to_call};
+pub use utils::{
+    Unsafety, all_super_traits, direct_super_traits, is_fn_unsafe_to_call,
+    target_feature_is_safe_in_target,
+};
 pub use variance::Variance;
 
 pub use chalk_ir::{

crates/hir-ty/src/mir/eval.rs

@@ -3,6 +3,7 @@
 use std::{borrow::Cow, cell::RefCell, fmt::Write, iter, mem, ops::Range};
 
 use base_db::Crate;
+use base_db::target::TargetLoadError;
 use chalk_ir::{Mutability, cast::Cast};
 use either::Either;
 use hir_def::{
@@ -337,7 +338,7 @@ impl Address {
 pub enum MirEvalError {
     ConstEvalError(String, Box<ConstEvalError>),
     LayoutError(LayoutError, Ty),
-    TargetDataLayoutNotAvailable(Arc<str>),
+    TargetDataLayoutNotAvailable(TargetLoadError),
     /// Means that code had undefined behavior. We don't try to actively detect UB, but if it was detected
     /// then use this type of error.
     UndefinedBehavior(String),