Allow `marker_adapter` to accept statically linked lints

[smoelius]

I have been experimenting with idea of running Marker lints under a Dylint driver, and I have a PoC. The PoC is essentially a Dylint library that loads other Marker libraries (see #257 for additional details). Ideally, the additional loading would not be necessary. Rather, the Marker lints would be linked directly into the Dylint library.

To elaborate, the PoC Dylint library has a check_crate implementation that looks like this:

    fn check_crate(&mut self, cx: &LateContext<'tcx>) {
        let adapter = Adapter::new(&self.lint_crates()).unwrap();
        lint_pass::process_crate(cx, &adapter);
    }

self.lint_crates() is a vector of LintCrateInfo, i.e., the names and locations of the Marker libraries the adapter should load.

What I would prefer would be an interface like the following:

static LINT_CRATE_HANDLES: &[LintCrateHandle] = &[foo::handle(), bar::handle()];
...
    fn check_crate(&mut self, cx: &LateContext<'tcx>) {
        let adapter = Adapter::new(LINT_CRATE_HANDLES).unwrap();
        lint_pass::process_crate(cx, &adapter);
    }

where:

• foo and bar are Marker lint crates, named in the Dylint library's Cargo.toml file.
• handle is a const-evaluable function that returns a structure with the information the adapter needs to call into the crate.

Do you think such an interface could be possible? Does it sound too onerous to implement?

[xFrednet]

I remember that a suggestion like that also came up in a discussion about potentially using rust-analyzer as a driver. There, the main motivation was performance, if I recall correctly. Not sure how interesting the discussion is for you, but here is the issue, if you want to read up on it: https://github.com/rust-marker/marker/issues/32

Right now, I have nothing against it. Using dynamic libraries was just the easiest solution for how I envisioned the standalone version Marker, we currently have. Supporting something like this in the future, was one reason for extracting this logic into the marker_adapter crate.

Another contributor, @Veetaha, also asked about a similar feature. I believe there are a few things which need to be discussed in this regard, like how these lint crates are statically linked? Do we load them all as one dynamic library, or do we compile them with the driver? Where do we store the driver if it contains static lints? etc..

[smoelius]

Where do we store the driver if it contains static lints?

What I am proposing would be an additional means of using Marker, not a replacement for the current one. (I should have been more clear about this.) The drivers might be changed (slightly) internally. But from a user's perspective, there would be no change, including where/what things are stored on the file system.

Essentially, I'm suggesting that the "loading" be moved out of the adapter.

Marker's code does two things conceptually:

1. provide a stable linting interface
2. provide drivers to load an run crates that use that stable interface

My proposal might be thought of as drawing a cleaner line between 1 and 2.

how these lint crates are statically linked? Do we load them all as one dynamic library, or do we compile them with the driver?

Let me try to answer this with some sample code. Currently, marker_rustc_driver initializes the adapter like this: https://github.com/rust-marker/marker/blob/4bd433f2b344a34224d05cd8e031dc2162bd4173/marker_rustc_driver/src/lint_pass.rs#L28-L31 If my suggestion were adopted, then I imagine that code might look like this:

ADAPTER.with(move |cell| { 
    let lint_crate_handles: &[LintCrateHandle] = load_lint_crates(lint_crates)?;
    cell.get_or_try_init(|| Adapter::new(lint_crate_handles))?; 
    Ok(()) 
}) 

The crucial idea is: a LintCrateHandle could come from a either a dynamic or statically linked library. For a Marker driver, the former would apply. For a Dylint library, the latter would.

[xFrednet]

What I am proposing would be an additional means of using Marker, not a replacement for the current one. (I should have been more clear about this.) The drivers might be changed (slightly) internally. But from a user's perspective, there would be no change, including where/what things are stored on the file system.

Oh right, my head was still focussing on the way Marker deals with the driver. (By default, we store the driver in the toolchain folder, which might not be ideal, if the lint crates are statically linked)

Marker's code does two things conceptually:

1. provide a stable linting interface
2. provide drivers to load an run crates that use that stable interface

My proposal might be thought of as drawing a cleaner line between 1 and 2.

Point 1 and 2 are split into different crates in Marker. Rustc's driver only does the conversion to the stable interface, and passes that to the adapter. I still would like the driver to focus on the actual conversion part and leave the lint crate loading things etc to the adapter.

We can do something like you suggested here:

ADAPTER.with(move |cell| { 
    let lint_crate_handles: &[LintCrateHandle] = load_lint_crates(lint_crates)?;
    cell.get_or_try_init(|| Adapter::new(lint_crate_handles))?; 
    Ok(()) 
}) 

Though, I would like the load_lint_crates util to come from the Adapter. The only reason that we moved some knowledge of the loaded lint crates into the driver, was to support the marker = "<lint-crate>" cfg test.

---

Another option, we could think about, would be to generate a dummy crate that has the lints we want to statically load as dependencies. This dummy crate would be used as a dependency for the driver, kind of like this:

The arrows (x --> Y) mean that X depends on Y. The code would then look something like this:

ADAPTER.with(move |cell| { 
    let lint_crate_handles: &[LintCrateHandle] = static_lint_crates::get_handles()?;
    cell.get_or_try_init(|| Adapter::new(lint_crate_handles))?; 
    Ok(()) 
}) 

What I like about this approach, is that static_lint_crates as a generated dummy crate, would be super simple to use by other drivers. We could even change Marker's normal setup, to compile all lint crates into this new dummy crate, and only have that one dynamically loaded. (I'm uncertain if this would be the best for the rustc driver, but it's something worth considering)

---

My short answer would be: If we find a nice interface for this, I'm not apposed to the idea of statically linking lint crates :)