Alternatives to rustc_plugin::registry for (Servo’s) custom lints?

[SimonSapin]

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. on struct definitions), ignored without warning by rustc.

  Currently we rely on rustc_plugin::registry::Registry::register_attribute.

  My reading of Tracking issue: RFC 2103 - attributes for tools #44690 is that the rustfmt:: and clippy:: 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 of x has one of the previously mentioned custom attributes, even if bar is 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 (where script is 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*.rlib with 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 build or cargo check does not require recompiling/rechecking dependencies. Consider allowing reuse of metadata between cargo check and cargo build cargo#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?

[eddyb]

Many years ago Servo had custom builds of rustc. It was a pain.

However, nowadays you could just link against rustc_interface and inject your lint, without building the whole compiler, only a small executable.

[bstrie]

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]

@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 JSContext object, 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]

If we're considering what alternative mechanisms could be added to solve this problem

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]

What about non-Josephine solutions, though, such as encoding the lint conditions in traits (auto traits?) and adding some perma-unstable feature to rustc to 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]

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]

#![register_tool] now replaces Registry::register_attribute: #66079

CC @pnkfelix