Rustc: Avoid caching by tracking lint crates and env vars

marker_adapter/Cargo.toml

@@ -16,3 +16,4 @@ marker_utils = { path = "../marker_utils", version = "0.1.1" }
 
 cfg-if     = "1.0.0"
 libloading = "0.8.0"
+thiserror  = "1.0.44"

marker_adapter/src/lib.rs

@@ -6,10 +6,9 @@
 
 pub mod context;
 mod loader;
+pub use loader::LintCrateInfo;
+use loader::{LintCrateRegistry, LoadingError};
 
-use std::{cell::RefCell, ops::ControlFlow};
-
-use loader::LintCrateRegistry;
 use marker_api::{
     ast::{
         expr::ExprKind,
@@ -21,9 +20,26 @@ use marker_api::{
     LintPass, LintPassInfo,
 };
 use marker_utils::visitor::{self, Visitor};
+use std::{cell::RefCell, ops::ControlFlow};
+use thiserror::Error;
+
+pub const LINT_CRATES_ENV: &str = "MARKER_LINT_CRATES";
+
+#[derive(Debug, Error)]
+pub enum AdapterError {
+    #[error("the `{LINT_CRATES_ENV}` environment value is not set")]
+    LintCratesEnvUnset,
+    /// The format of the environment value is defined in the `README.md` of
+    /// the `marker_adapter` crate.
+    #[error("the content of the `{LINT_CRATES_ENV}` environment value is malformed")]
+    LintCratesEnvMalformed,
+    #[error("error while loading the lint crate: {0:#?}")]
+    LoadingError(#[from] LoadingError),
+}
 
 /// This struct is the interface used by lint drivers to load lint crates, pass
 /// `marker_api` objects to external lint passes and all other magic you can think of.
+#[derive(Debug)]
 pub struct Adapter {
     /// [`LintPass`] functions are called with a mutable `self` parameter as the
     /// first argument. This `RefCell` acts as a wrapper to hide the internal
@@ -34,17 +50,23 @@ pub struct Adapter {
     inner: RefCell<AdapterInner>,
 }
 
+#[derive(Debug)]
 struct AdapterInner {
     external_lint_crates: LintCrateRegistry,
 }
 
 impl Adapter {
-    #[must_use]
-    pub fn new_from_env() -> Self {
-        let external_lint_crates = LintCrateRegistry::new_from_env();
-        Self {
+    /// This creates a new [`Adapter`] instance
+    ///
+    /// # Errors
+    ///
+    /// This function will return an error if an error occurs during the lint
+    /// loading process.
+    pub fn new(lint_crates: &[LintCrateInfo]) -> Result<Self, AdapterError> {
+        let external_lint_crates = LintCrateRegistry::new(lint_crates)?;
+        Ok(Self {
             inner: RefCell::new(AdapterInner { external_lint_crates }),
-        }
+        })
     }
 
     #[must_use]

marker_adapter/src/loader.rs

@@ -1,10 +1,12 @@
 use cfg_if::cfg_if;
 use libloading::Library;
-
 use marker_api::{interface::LintCrateBindings, AstContext};
-use marker_api::{LintPass, LintPassInfo};
-
+use marker_api::{LintPass, LintPassInfo, MARKER_API_VERSION};
 use std::ffi::{OsStr, OsString};
+use std::path::PathBuf;
+use thiserror::Error;
+
+use super::{AdapterError, LINT_CRATES_ENV};
 
 /// Splits [`OsStr`] by an ascii character
 fn split_os_str(s: &OsStr, c: u8) -> Vec<OsString> {
@@ -40,53 +42,58 @@ fn windows_split_os_str(s: &OsStr, c: u8) -> Vec<OsString> {
     bytes.split(|v| *v == u16::from(c)).map(OsString::from_wide).collect()
 }
 
-/// This struct loads external lint crates into memory and provides a safe API
-/// to call the respective methods on all of them.
-#[derive(Default)]
-pub struct LintCrateRegistry {
-    passes: Vec<LoadedLintCrate>,
+/// A struct describing a lint crate that can be loaded
+#[derive(Debug, Clone)]
+pub struct LintCrateInfo {
+    /// The absolute path of the compiled dynamic library, which can be loaded as a lint crate.
+    pub path: PathBuf,
 }
 
-impl LintCrateRegistry {
+impl LintCrateInfo {
+    /// This function tries to load the list of [`LintCrateInfo`]s from the
+    /// [`LINT_CRATES_ENV`] environment value.
+    ///
     /// # Errors
-    /// This can return errors if the library couldn't be found or if the
-    /// required symbols weren't provided.
-    fn load_external_lib(lib_path: &OsStr) -> Result<LoadedLintCrate, LoadingError> {
-        let lib: &'static Library = Box::leak(Box::new(
-            unsafe { Library::new(lib_path) }.map_err(|_| LoadingError::FileNotFound)?,
-        ));
+    ///
+    /// This function will return an error if the value can't be read or the
+    /// content is malformed. The `README.md` of this adapter contains the
+    /// format definition.
+    pub fn list_from_env() -> Result<Vec<LintCrateInfo>, AdapterError> {
+        let Some(env_str) = std::env::var_os(LINT_CRATES_ENV) else {
+            return Err(AdapterError::LintCratesEnvUnset);
+        };
 
-        let pass = LoadedLintCrate::try_from_lib(lib)?;
+        let mut list = vec![];
+        for entry_str in split_os_str(&env_str, b';') {
+            if entry_str.is_empty() {
+                return Err(AdapterError::LintCratesEnvMalformed);
+            }
 
-        // FIXME: Create issue for lifetimes and fix dropping and pointer decl stuff
+            list.push(LintCrateInfo {
+                path: PathBuf::from(entry_str),
+            });
+        }
 
-        Ok(pass)
+        Ok(list)
     }
+}
 
-    /// # Panics
-    ///
-    /// Panics if a lint in the environment couldn't be loaded.
-    pub fn new_from_env() -> Self {
-        let mut new_self = Self::default();
-
-        let Some((_, lint_crates_lst)) = std::env::vars_os().find(|(name, _val)| name == "MARKER_LINT_CRATES") else {
-            panic!("Adapter tried to find `MARKER_LINT_CRATES` env variable, but it was not present");
-        };
-
-        for lib in split_os_str(&lint_crates_lst, b';') {
-            if lib.is_empty() {
-                continue;
-            }
+/// This struct loads external lint crates into memory and provides a safe API
+/// to call the respective methods on all of them.
+#[derive(Debug, Default)]
+pub struct LintCrateRegistry {
+    passes: Vec<LoadedLintCrate>,
+}
 
-            let lib = match Self::load_external_lib(&lib) {
-                Ok(v) => v,
-                Err(err) => panic!("Unable to load `{}`, reason: {err:?}", lib.to_string_lossy()),
-            };
+impl LintCrateRegistry {
+    pub fn new(lint_crates: &[LintCrateInfo]) -> Result<Self, LoadingError> {
+        let mut new_self = Self::default();
 
-            new_self.passes.push(lib);
+        for krate in lint_crates {
+            new_self.passes.push(LoadedLintCrate::try_from_info(krate.clone())?);
         }
 
-        new_self
+        Ok(new_self)
     }
 
     pub(super) fn set_ast_context<'ast>(&self, cx: &'ast AstContext<'ast>) {
@@ -153,36 +160,64 @@ impl LintPass for LintCrateRegistry {
 
 struct LoadedLintCrate {
     _lib: &'static Library,
+    info: LintCrateInfo,
     bindings: LintCrateBindings,
 }
 
+#[allow(clippy::missing_fields_in_debug)]
+impl std::fmt::Debug for LoadedLintCrate {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("LoadedLintCrate").field("info", &self.info).finish()
+    }
+}
+
 impl LoadedLintCrate {
-    fn try_from_lib(lib: &'static Library) -> Result<Self, LoadingError> {
+    fn try_from_info(info: LintCrateInfo) -> Result<Self, LoadingError> {
+        let lib: &'static Library = Box::leak(Box::new(unsafe { Library::new(&info.path) }?));
+
+        let pass = LoadedLintCrate::try_from_lib(lib, info)?;
+
+        Ok(pass)
+    }
+
+    fn try_from_lib(lib: &'static Library, info: LintCrateInfo) -> Result<Self, LoadingError> {
         // Check API version for verification
         let get_api_version = {
             unsafe {
                 lib.get::<unsafe extern "C" fn() -> &'static str>(b"marker_api_version\0")
-                    .map_err(|_| LoadingError::MissingLintDeclaration)?
+                    .map_err(|_| LoadingError::MissingApiSymbol)?
             }
         };
-        if unsafe { get_api_version() } != marker_api::MARKER_API_VERSION {
-            return Err(LoadingError::IncompatibleVersion);
+        let krate_api_version = unsafe { get_api_version() };
+        if krate_api_version != MARKER_API_VERSION {
+            return Err(LoadingError::IncompatibleVersion {
+                krate_version: krate_api_version.to_string(),
+            });
         }
 
         // Load bindings
         let get_lint_crate_bindings = unsafe {
             lib.get::<extern "C" fn() -> LintCrateBindings>(b"marker_lint_crate_bindings\0")
-                .map_err(|_| LoadingError::MissingLintDeclaration)?
+                .map_err(|_| LoadingError::MissingBindingSymbol)?
         };
         let bindings = get_lint_crate_bindings();
 
-        Ok(Self { _lib: lib, bindings })
+        Ok(Self {
+            _lib: lib,
+            info,
+            bindings,
+        })
     }
 }
 
-#[derive(Debug)]
+#[derive(Error, Debug)]
 pub enum LoadingError {
-    FileNotFound,
-    IncompatibleVersion,
-    MissingLintDeclaration,
+    #[error("the lint crate could not be loaded: {0:#?}")]
+    LibLoading(#[from] libloading::Error),
+    #[error("the loaded crated doesn't contain the `marker_api_version` symbol")]
+    MissingApiSymbol,
+    #[error("the loaded crated doesn't contain the `marker_lint_crate_bindings` symbol")]
+    MissingBindingSymbol,
+    #[error("incompatible api version:\n- lint-crate api: {krate_version}\n- driver api: {MARKER_API_VERSION}")]
+    IncompatibleVersion { krate_version: String },
 }

marker_rustc_driver/src/lint_pass.rs

@@ -1,6 +1,6 @@
-use std::cell::LazyCell;
+use std::cell::OnceCell;
 
-use marker_adapter::Adapter;
+use marker_adapter::{Adapter, AdapterError, LintCrateInfo};
 use marker_api::lint::Lint;
 
 use crate::context::{storage::Storage, RustcContext};
@@ -18,17 +18,26 @@ thread_local! {
     /// Storing the [`Adapter`] in a `thread_local` is safe, since rustc is currently
     /// only single threaded. This cell will therefore only be constructed once, and
     /// this driver will always use the same adapter.
-    static ADAPTER: LazyCell<Adapter> = LazyCell::new(|| {
-        Adapter::new_from_env()
-    });
+    static ADAPTER: OnceCell<Adapter> = OnceCell::new();
 }
 
 pub struct RustcLintPass;
 
 impl RustcLintPass {
+    pub fn init_adapter(lint_crates: &[LintCrateInfo]) -> Result<(), AdapterError> {
+        ADAPTER.with(move |cell| {
+            if cell.get().is_none() {
+                cell.set(Adapter::new(lint_crates)?).unwrap();
+            };
+            Ok(())
+        })
+    }
+
     pub fn marker_lints() -> Vec<&'static Lint> {
         ADAPTER.with(|adapter| {
             adapter
+                .get()
+                .unwrap()
                 .lint_pass_infos()
                 .iter()
                 .flat_map(marker_api::LintPassInfo::lints)
@@ -43,7 +52,7 @@ rustc_lint_defs::impl_lint_pass!(RustcLintPass => []);
 impl<'tcx> rustc_lint::LateLintPass<'tcx> for RustcLintPass {
     fn check_crate(&mut self, rustc_cx: &rustc_lint::LateContext<'tcx>) {
         ADAPTER.with(|adapter| {
-            process_crate(rustc_cx, adapter);
+            process_crate(rustc_cx, adapter.get().unwrap());
         });
     }
 }