Repository navigation
Alternatives to rustc_plugin::registry for (Servo’s) custom lints? #62868
Description
Continuing from #62727 (comment):
Servo’s memory safety unfortunately still depends on a custom lint that checks that GC pointers on the stack are properly rooted: https://github.com/servo/servo/tree/cef98d2e5179/components/script_plugins
I would love to find another solution, this plugin is regularly giving us headaches. But until we do, if the plugin API is removed Servo will be unable to upgrade and will be stuck on an old Rust Nightly.So there has been no movement since last time we discussed this. That's unfortunate. At some point however, I think it stops being reasonable for rustc to indefinitely maintain the plugin interface solely for the benefit of servo. What would it take to get rid of your reliance on this?
It would take building an alternative.
This thread is to discuss what alternative mechanisms already exist or could be added.
Requirements
-
Having custom attributes in source code (e.g. onstructdefinitions), ignored without warning by rustc.Currently we rely onrustc_plugin::registry::Registry::register_attribute.My reading of Tracking issue: RFC 2103 - attributes for tools #44690 is that therustfmt::andclippy::attribute namespaces are hard-coded for now, and that the RFC-proposed mechanism for registering more such namespaces / tools is not implemented yet.Edit: This is now taken care of by
#![register_tool]: Tracking issue for#![register_tool]#66079 -
The ability to write custom “lints”: arbitrary code that analyses a given crate and can reject it based on:
- Access to everything in a crate: type definitions, expressions and sub-expressions
- Access to span information, to report the location of an error when there is one
- Access to full type information. For example in
let x = foo.bar();the lint wants to find out whether the struct definition for the concrete type ofxhas one of the previously mentioned custom attributes, even ifbaris a trait method that returns an associated type.
-
The lint only runs for selected crates. Or at least it has access to the crate name (and can exit early based on its own allow-list.)
-
Running the lint after (or during) a normal
cargo build -p script(wherescriptis the crate being linted) does not require recompiling dependencies, nor duplicating much of Cargo’s logic.As far as I understand, resolving types requires metadata for dependency crates. It’s fine if the lint runs separately from rustc, but reverse-engineering Cargo to find out which of multiple
libfoo*.rlibwith different hashes in their name is the correct one would be fragile. -
Using Rust Nightly through rustup.
Many years ago Servo had custom builds of rustc. It was a pain.
Nice to have
-
Access to rustc’s mechanism for emitting nicely-formatted diagnostics (not just access to line and column numbers)
-
Running the lint after either
cargo buildorcargo checkdoes not require recompiling/rechecking dependencies. Consider allowing reuse of metadata betweencargo checkandcargo buildcargo#3501 may be relevant.- The lint being part of normal compilation / checking is ideal.
-
Reducing reliance on rustc-internal APIs that change often would be nice, to help reduce the frequency of PRs like this: Upgrade to rustc 1.38.0-nightly (273f42b59 2019-07-21) servo/servo#23822. (We’re averaging at 20 days between Nightly upgrades in the last year, mostly because of this.)
I do realize this is in contradiction with many of the other points.
- Would it be crazy for RLS to have a non-interactive mode where it dumps everything it knows about a crate into a giant JSON file?
@jdm, @Manishearth, @nox: anything to add?
Activity
eddyb commentedon Aug 11, 2019
However, nowadays you could just link against
rustc_interfaceand inject your lint, without building the whole compiler, only a small executable.bstrie commentedon Nov 4, 2019
My knowledge is long out of date, but the lints themselves are only seen as a stopgap solution until Rust provides some sort of first-class way to integrate with an external garbage collector, yes? If we're considering what alternative mechanisms could be added to solve this problem, then it makes sense to also consider what it would take to get rid of Servo's lints entirely. Does Servo have an issue for tracking such a thing?
asajeffrey commentedon Nov 4, 2019
@bstrie It would be nice to move over to something like Josephine (https://github.com/asajeffrey/josephine) which integrates JS's GC into the Rust lifetime system. Josephine is a proof-of-concept, and we hit the point of validating the principal of the library, but not a plan of how to scale it to something like Servo, and what the migration path would look like.
The main problem is that Josephine follows SpiderMonkey in passing around a
JSContextobject, and using Rust type information to make sure that it, and any JS objects, are used safely. Unfortunately this clashes with Servo, which stores the JS context in a global variable with fixed type. So it's a non-trivial effort to move Servo's code over to explicitly passing a JS context object.The issue for this (though it's pretty sparse) is servo/servo#8732
Manishearth commentedon Nov 5, 2019
We were part of a discussion with the lang team folks about GC design. Eventually, the discussion petered out because "servo is fine with its lints" (we were!).
I documented the state of our work there in https://manishearth.github.io/blog/2016/08/18/gc-support-in-rust-api-design/ . It's a hard problem to solve and will take much more work.
Josephine is promising but is also a huge refactor.
We're mostly happy using the unstable plugin API here.
eddyb commentedon Nov 6, 2019
What about non-Josephine solutions, though, such as encoding the lint conditions in
traits (auto traits?) and adding some perma-unstable feature torustcto trigger diagnostics?I keep suggesting something like this once in a while but I haven't had the chance to delve deep into this with someone familiar with the current architecture of the lints in question.
Manishearth commentedon Nov 6, 2019
It could also just be an attribute. If rustc wants to absorb the must_root attribute we can make changes to make it a bit more general
SimonSapin commentedon Nov 17, 2019
#![register_tool]now replacesRegistry::register_attribute: #66079CC @pnkfelix
pnkfelix commentedon Nov 25, 2019
I'm going to try to spend some time investigating this. I've had some 1:1 conversations with @SimonSapin and think there are plausible paths to replace the last remaining use of plugins.
SimonSapin commentedon Feb 7, 2020
@Zoxc, you were asking about this on IRC. Unfortunately we have no news that is not already described in this issue. If the plugin system is removed from Rust Nightly without a migration path, Servo will be left with a choice of not upgrading past an old Nightly that still has it or abandoning our GC rooting soundness lint.
Mark-Simulacrum commentedon Jan 22, 2024
Closing as Servo has moved off plugins (servo/servo#30508) and plugin support has been removed (#116412).