Skip to content
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

internal: Update match checking algorithm #10484

Merged
merged 4 commits into from Dec 20, 2021

Conversation

iDawer
Copy link
Contributor

@iDawer iDawer commented Oct 7, 2021

Sync match checking algorithm with rust-lang/rust f31622a50 2021-11-12 (rust-lang/rust#90813)

This update brings huge simplification to the match checking and introduces an easy to use machinery for pattern destructuring and also:

  1. Add a function to do post-inference normalization hir_ty::infer::normalize(...).
  2. Store binding modes in InferenceResult.

Todo:

@iDawer
Copy link
Contributor Author

iDawer commented Nov 19, 2021

I'll probably continue on this before the extraction into a library rust-lang/rust#89570 lands. Anyway this represents the current rustc nightly, except some recent fixes I want to catch up.

@iDawer
Copy link
Contributor Author

iDawer commented Dec 14, 2021

Tested 6942dd4 (outdated):

  • for diagnostics output regression on [self, ripgrep, webrender, diesel]: no difference (action run).
  • analysis-stats on self (action run):
    Run ./target/rust-analyzer-baseline-4566414789310acb2617543f4b50beab4bb48e06 -q analysis-stats --memory-usage .
    Database loaded:     1.06s, 118mb (metadata 677.17ms, 791kb; build 201.30ms, -180kb)
      crates: 39, mods: 824, decls: 18627, fns: 13894
    Item Collection:     17.52s, 394mb
      exprs: 381416, ??ty: 388 (0%), ?ty: 275 (0%), !ty: 145
    Inference:           69.37s, 652mb
    Total:               86.89s, 1046mb
    
    Run ./target/rust-analyzer-bench-6942dd44d0a9a0ff77fdf4ac3fcf1e986cda763d -q analysis-stats --memory-usage .
    Database loaded:     968.60ms, 118mb (metadata 623.11ms, 791kb; build 179.41ms, -180kb)
      crates: 39, mods: 824, decls: 18627, fns: 13894
    Item Collection:     16.21s, 394mb
      exprs: 381416, ??ty: 388 (0%), ?ty: 275 (0%), !ty: 145
    Inference:           73.88s, 653mb
    Total:               90.09s, 1047mb
    
  • checked for diagnostic panics on test data from rust-lang/rust/src/test/:
    ui/pattern/
    ui/or-patterns/
    ui/rfc-2008-non-exhaustive/
    ui/match/
    ui/binding/
    ui/rfc-2005-default-binding-mode/
    ui/rfcs/rfc-2005-default-binding-mode/
    
ui test details
diff --git a/crates/ide_diagnostics/src/tests.rs b/crates/ide_diagnostics/src/tests.rs
index a2b92c4ff..59d0d9eb3 100644
--- a/crates/ide_diagnostics/src/tests.rs
+++ b/crates/ide_diagnostics/src/tests.rs
@@ -1,4 +1,13 @@
 mod sourcegen;
+mod ui {
+    mod pattern;
+    mod or_patterns;
+    mod rfc_2008;
+    mod match_;
+    mod binding;
+    mod rfc_2005;
+    mod rfcs_rfc_2005;
+}
 
 use expect_test::Expect;
 use ide_db::{
@@ -126,6 +135,20 @@ pub(crate) fn check_diagnostics_with_config(config: DiagnosticsConfig, ra_fixtur
     }
 }
 
+#[track_caller]
+pub(crate) fn scan_diagnostics(ra_fixture: &str) {
+    let fixture = format!(
+        r"
+//- minicore: option, sized
+{}
+",
+        ra_fixture
+    );
+
+    let (db, file) = RootDatabase::with_single_file(&fixture);
+    super::diagnostics(&db, &DiagnosticsConfig::default(), &AssistResolveStrategy::All, file);
+}
+
 #[test]
 fn test_disabled_diagnostics() {
     let mut config = DiagnosticsConfig::default();
diff --git a/crates/test_utils/src/fixture.rs b/crates/test_utils/src/fixture.rs
index 3d303a123..1f07538d0 100644
--- a/crates/test_utils/src/fixture.rs
+++ b/crates/test_utils/src/fixture.rs
@@ -150,7 +150,7 @@ impl Fixture {
                     && !line.contains(".")
                     && line.chars().all(|it| !it.is_uppercase())
                 {
-                    panic!("looks like invalid metadata line: {:?}", line);
+                    // panic!("looks like invalid metadata line: {:?}", line);
                 }
 
                 if let Some(entry) = res.last_mut() {
diff --git a/ui b/ui
new file mode 100644
index 000000000..97ecea6cd
--- /dev/null
+++ b/ui
@@ -0,0 +1,23 @@
+ui/pattern/
+ui/or-patterns/
+ui/rfc-2008-non-exhaustive/
+ui/match/
+ui/binding/
+ui/rfc-2005-default-binding-mode/
+ui/rfcs/rfc-2005-default-binding-mode/
+
+
+#!/bin/bash
+echo "use crate::tests::scan_diagnostics;"
+for file in $(find $1 -name "*.rs")
+do
+   test_name=__$(basename "$file" ".rs" | sed -e 's/[^a-zA-Z0-9_]/_/g')
+   echo "\
+#[test]
+fn $test_name() {
+   scan_diagnostics(include_str!(
+       \"$file\"
+   ));
+}
+"
+done

@iDawer iDawer changed the title [Experiment] Update match checking algorithm internal: Update match checking algorithm Dec 14, 2021
@iDawer
Copy link
Contributor Author

iDawer commented Dec 17, 2021

Tested on rebased a1e67d4 (update: rebased again a9ad7be, results should be the same)

  • for diagnostics output regression on [self, ripgrep, webrender, diesel]: no difference
  • analysis-stats on self (action run):
    Run ./target/rust-analyzer-baseline-9efa355a629badd9eb680b0ae9067757bff05f96 -q analysis-stats --memory-usage .
    Database loaded:     1.07s, 119mb (metadata 679.71ms, 803kb; build 200.00ms, -188kb)
      crates: 39, mods: 833, decls: 18856, fns: 14061
    Item Collection:     16.31s, 381mb
      exprs: 386921, ??ty: 395 (0%), ?ty: 288 (0%), !ty: 146
    Inference:           80.52s, 646mb
    Total:               96.83s, 1027mb
    
    Run ./target/rust-analyzer-bench-a1e67d441d94fef2d4505e88606a15bf36ecd5c9 -q analysis-stats --memory-usage .
    Database loaded:     1.08s, 119mb (metadata 682.11ms, 803kb; build 201.92ms, -197kb)
      crates: 39, mods: 833, decls: 18856, fns: 14061
    Item Collection:     16.30s, 381mb
      exprs: 386921, ??ty: 395 (0%), ?ty: 288 (0%), !ty: 146
    Inference:           81.03s, 647mb
    Total:               97.33s, 1028mb
    
  • rechecked for diagnostic panics on test data from rust-lang/rust/src/test/

@iDawer iDawer marked this pull request as ready for review December 17, 2021 12:03
@lnicola
Copy link
Member

lnicola commented Dec 19, 2021

@iDawer can you please rebase? There's been a couple of mechanical changes related to chalk.

@iDawer
Copy link
Contributor Author

iDawer commented Dec 19, 2021

@iDawer can you please rebase? There's been a couple of mechanical changes related to chalk.

Done! It is simply &Interner => Interner changes, right. I believe the previous test is still applicable

(Slice(self_slice), Slice(other_slice))
if self_slice.arity() != other_slice.arity() =>
{
unimplemented!()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can people hit this currently? We don't wanna panic if that is the case I'd say

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, Slice is void struct. It is here just to preserve semantics of catchall arms

/// A constructor for array and slice patterns.
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub(super) struct Slice {
    _unimplemented: Void,
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To 'preserve' in relation of rustc

@lnicola
Copy link
Member

lnicola commented Dec 20, 2021

Thanks! Let's merge this to avoid further conflicts.

bors r+

@bors
Copy link
Contributor

bors bot commented Dec 20, 2021

@bors bors bot merged commit 48d6cef into rust-lang:master Dec 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants