Stream: t-compiler/wg-parallel-rustc

Topic: lint store


Zoxc (Oct 08 2019 at 05:01, on Zulip):

The lock around LintStore can be removed by making a query which calls the plugins and returns a plain LintStore which includes the plugin lints.

The list of lints can be made immutable by using lists of constructor functions instead of lint objects. This is currently done for late per-module lints, but in a pretty hacky way where you can construct more lint objects from an existing one.

I have been waiting on TyCtxt to be created earlier to clean this up and also allow lints to refer to 'tcx.

cc @simulacrum

nikomatsakis (Oct 08 2019 at 09:54, on Zulip):

The lock around LintStore can be removed by making a query which calls the plugins and returns a plain LintStore which includes the plugin lints.

Ah, nice -- we were discussing adding a query, but not having it query the plugins. This doesn't seem like it's blocked on creating the TyCtxt earlier, right?

nikomatsakis (Oct 08 2019 at 09:54, on Zulip):

The list of lints can be made immutable by using lists of constructor functions instead of lint objects. This is currently done for late per-module lints, but in a pretty hacky way where you can construct more lint objects from an existing one.

yeah, we were talking about doing this

simulacrum (Oct 08 2019 at 10:52, on Zulip):

The second part (constructor functions) I have done. I can try the first part (probably with a rustc_interface query) but the most that I can do is probably making the lint store a Once instead of a Lock, since we need it on session I suspect. But maybe I can thread it through to the relevant places without that.

simulacrum (Oct 08 2019 at 12:06, on Zulip):

https://github.com/rust-lang/rust/pull/65193

nikomatsakis (Oct 08 2019 at 12:25, on Zulip):

The second part (constructor functions) I have done. I can try the first part (probably with a rustc_interface query) but the most that I can do is probably making the lint store a Once instead of a Lock, since we need it on session I suspect. But maybe I can thread it through to the relevant places without that.

do we really?

Zoxc (Oct 08 2019 at 12:28, on Zulip):

I'd just wait until it can be a proper TyCtxt query

simulacrum (Oct 08 2019 at 12:37, on Zulip):

@nikomatsakis we might not need it on session -- it's possible we could thread it into everything pre-tyctxt manually, and then just use it in a fully initialized state via TyCtxt

simulacrum (Oct 08 2019 at 12:38, on Zulip):

@Zoxc We're going to be waiting a while for that, though, right? Like... months, I'd assume? It'd be good to make progress where we can

Zoxc (Oct 08 2019 at 12:48, on Zulip):

You can probably make the register_plugins rustc_interface query create Lrc<LintStore> and then pass it on to TyCtxt.

Zoxc (Oct 08 2019 at 12:49, on Zulip):

Moving it from Session to GlobalCtxt

simulacrum (Oct 08 2019 at 12:52, on Zulip):

yes, that's my plan -- I was uncertain if we ran any lints before we registered plugins, if that's the case we might not be able to make such a transformation

simulacrum (Oct 08 2019 at 12:52, on Zulip):

but I suspect we can either not do so or do some minor restructuring to stick plugin registration in

nikomatsakis (Oct 08 2019 at 12:56, on Zulip):

remind me -- the "early lints" run before tcx creation?

nikomatsakis (Oct 08 2019 at 12:56, on Zulip):

(as far as timing goes, I think it's worth trying to progressively simplify Session in small ways where we can)

simulacrum (Oct 08 2019 at 12:59, on Zulip):

yes, they do

simulacrum (Oct 08 2019 at 13:00, on Zulip):

check_ast_crate is the function to grep for if you want

simulacrum (Oct 08 2019 at 13:00, on Zulip):

or at least I assume they do -- there's no TyCtxt around in any of those functions

simulacrum (Oct 08 2019 at 13:00, on Zulip):

I think that's sort of "the difference"

simulacrum (Oct 08 2019 at 13:01, on Zulip):

OTOH, I don't know _why_ we care - it seems not hugely helpful. I guess IDEs want results earlier?

Zoxc (Oct 08 2019 at 13:07, on Zulip):

We only run lints after registering plugins

nikomatsakis (Oct 08 2019 at 14:25, on Zulip):

OTOH, I don't know _why_ we care - it seems not hugely helpful. I guess IDEs want results earlier?

I think it might be that they operate on the AST, and we want to throw that way? I dimly recall something like that

simulacrum (Oct 08 2019 at 16:01, on Zulip):

hm, maybe -- I guess TyCtxt does contain the HIR so it is post lowering

nikomatsakis (Oct 22 2019 at 20:02, on Zulip):

@simulacrum I forget, did we talk about trying to document how the lint store works in the rustc-guide?

nikomatsakis (Oct 22 2019 at 20:03, on Zulip):

(I'm reviewing your PR now fyi)

simulacrum (Oct 22 2019 at 20:15, on Zulip):

Yeah I was planning to do so once we r+

simulacrum (Oct 22 2019 at 20:16, on Zulip):

I've been thinking about it passively though haven't actually written anything just yet

nikomatsakis (Oct 22 2019 at 20:21, on Zulip):

@simulacrum ok I left r+ -- this PR seems like a solid step forward whatever we do. It's very gratifying to see the interior mutability just go away. =)

nikomatsakis (Oct 22 2019 at 20:21, on Zulip):

I was pondering what @Zoxc wrote earlier -- I like the idea of (eventually?) having the LintStore construction be a query that executes once, which gathers up the lints at that time -- but it seems like that is dependent on a much broader refactoring, so I think we ought to land this for now.

simulacrum (Oct 22 2019 at 20:23, on Zulip):

Yeah, I ... feel a bit ambiguous about that kind of query

simulacrum (Oct 22 2019 at 20:23, on Zulip):

I think it makes sense

simulacrum (Oct 22 2019 at 20:24, on Zulip):

but it feels like it doesn't _quite_ fit into current query infrastructure

nikomatsakis (Oct 22 2019 at 20:25, on Zulip):

in salsa, it would be a "volatile" query I think -- well, one that does a "synthetic read"

nikomatsakis (Oct 22 2019 at 20:25, on Zulip):

but anyway

Last update: Nov 17 2019 at 07:20UTC