-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Only insert nodes which changes lint levels in the LintLevelMap #58176
Conversation
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@bors try |
⌛ Trying commit 76b688633cd55ff5ee282a70fc1790d229f539b1 with merge 0677dc2b768296023b6cb338d6afbdc5ab0be878... |
☀️ Test successful - checks-travis |
@rust-timer build 0677dc2b768296023b6cb338d6afbdc5ab0be878 |
Success: Queued 0677dc2b768296023b6cb338d6afbdc5ab0be878 with parent b2c6b8c, comparison URL. |
Finished benchmarking try commit 0677dc2b768296023b6cb338d6afbdc5ab0be878 |
1d97ce4
to
59c6c5a
Compare
@bors try |
[WIP] Only insert nodes which changes lint levels in the LintLevelMap r? @eddyb
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@bors try |
Only insert nodes which changes lint levels in the LintLevelMap r? @eddyb
☀️ Test successful - checks-travis |
@rust-timer build f94acd0 |
Success: Queued f94acd0 with parent 65e647c, comparison URL. |
Finished benchmarking try commit f94acd0 |
@bors try |
⌛ Trying commit 776e2eacdd50eae29db3d72949bada4ea43659fe with merge 74c0b1340d42de21d4d37f561338cbab04a9e6e2... |
@oli-obk I don't know how to run clippy locally. I just get |
@bors try |
Only insert nodes which changes lint levels in the LintLevelMap r? @eddyb
How did you run it? |
I set the |
☀️ Try build successful - checks-travis |
@rust-timer build f2f65c8 |
Success: Queued f2f65c8 with parent f22dca0, comparison URL. |
Finished benchmarking try commit f2f65c8 |
perf is mixed, but max-rss seems mostly positive |
Btw, one change I'd like to eventually make is to allow lints to declare that they're pure, i.e. they don't cache any state between
|
filed #59024 |
cc @estebank in case you know stuff about lints |
@oli-obk Are you comfortable approving this? |
Implementation wise yes, but I'm not happy with the perf regressions. Since this enables actually making lints incremental, which again will be a perf bonus outstripping most of the regressions, I'm going to r+ @bors r+ |
📌 Commit f07ce55 has been approved by |
@oli-obk We might be able to get rid of the regressions by storing |
In order to do that properly we'd need to have different diagnostic systems for early lint passes and late lint passes (as early lint passes don't have |
I meant to say storing the HIR lint location in the serialized lints, so emitting a lint won't depend on the lint level map. |
Only insert nodes which changes lint levels in the LintLevelMap r? @eddyb
☀️ Test successful - checks-travis, status-appveyor |
r? @eddyb