pattern analysis #3115

tamaroning · 2024-08-02T12:03:58Z

This patch adds a pattern analysis pass which detects non-exhaustive match patterns and finds witnesses (counterexamples).
The algorithm I implemented is based on one described in rustc documents. The current implementation lacks some patterns (OR, literals, ranges, etc.), which is handled by converting them into wildcard pattern.

gccrs: Implement initial pattern analysis pass.
gcc/rust/ChangeLog:

	* Make-lang.in: Add rust-hir-pattern-analysis.o.
	* rust-session-manager.cc (Session::compile_crate):
		Add pattern analysis pass.
	* typecheck/rust-hir-type-check-pattern.cc (TypeCheckPattern::visit):
		Do typecheck for subpatterns.
	* checks/errors/rust-hir-pattern-analysis.cc: New file.
	* checks/errors/rust-hir-pattern-analysis.h: New file.

gcc/testsuite/ChangeLog:

	* rust/compile/exhaustiveness1.rs: New test.
	* rust/compile/exhaustiveness2.rs: New test.
	* rust/compile/exhaustiveness3.rs: New test.

tamaroning · 2024-08-07T23:55:15Z

Build fails due to warnings. Let me fix it.

tamaroning · 2024-08-08T03:14:44Z

it seems that warnings are not related my code.

tamaroning · 2024-08-22T14:45:56Z

I found ci complained about structured bindings in my code which are not supported by gcc48.
Now i fixed and pushed the code.

P-E-P · 2024-08-29T13:21:02Z

@tamaroning is this ready for review ?

tamaroning · 2024-08-29T14:19:14Z

@P-E-P yes!

tamaroning · 2024-08-29T14:25:48Z

I leave some comments here for review

tamaroning · 2024-08-29T14:30:43Z

gcc/rust/checks/errors/rust-hir-pattern-analysis.cc

+// Entry point for computing match usefulness and check exhaustiveness
+void
+check_match_usefulness (Resolver::TypeCheckContext *ctx,
+			TyTy::BaseType *scrutinee_ty, HIR::MatchExpr &expr)
+{
+  // Lower the arms to a more convenient representation.
+  std::vector<MatrixRow> rows;
+  for (auto &arm : expr.get_match_cases ())
+    {
+      PatStack pats;
+      MatchArm lowered = lower_arm (ctx, arm, scrutinee_ty);
+      PatOrWild pat = PatOrWild::make_pattern (lowered.get_pat ());
+      pats.push (pat);
+      rows.push_back (MatrixRow (pats, lowered.has_guard ()));
+    }
+
+  std::vector<PlaceInfo> place_infos = {{PlaceInfo (scrutinee_ty)}};
+  Matrix matrix{rows, place_infos};
+
+  WitnessMatrix witness = compute_exhaustiveness_and_usefulness (ctx, matrix);
+
+  emit_exhaustiveness_error (ctx, expr, witness);
+}


This is an entry point function that is called for each match expression.
It converts all arms in a match expr to a matrix for which we compute witness (counterexamples of exhaustiveness).

tamaroning · 2024-08-29T14:36:11Z

gcc/rust/checks/errors/rust-hir-pattern-analysis.cc

+static DeconstructedPat
+lower_pattern (Resolver::TypeCheckContext *ctx, HIR::Pattern *pattern,
+	       TyTy::BaseType *scrutinee_ty)


All patterns in the match expr are lowered to DeconstructedPat.
We support struct and enum-variant patterns for now. Other unimplemented patterns are handled by converting them to wildcard.

tamaroning · 2024-08-29T14:39:10Z

gcc/rust/checks/errors/rust-hir-pattern-analysis.cc

+// The core of the algorithm. It computes the usefulness and exhaustiveness of a
+// given matrix recursively.
+// TODO: calculate usefulness
+static WitnessMatrix
+compute_exhaustiveness_and_usefulness (Resolver::TypeCheckContext *ctx,
+				       Matrix &matrix)


almost same as https://doc.rust-lang.org/nightly/nightly-rustc/rustc_pattern_analysis/usefulness/fn.compute_exhaustiveness_and_usefulness.html

tamaroning · 2024-08-29T14:39:24Z

gcc/rust/checks/errors/rust-hir-pattern-analysis.cc

+Matrix
+Matrix::specialize (const Constructor &ctor) const


Almost same as https://doc.rust-lang.org/nightly/nightly-rustc/rustc_pattern_analysis/usefulness/struct.Matrix.html#method.specialize_constructor

P-E-P

This is great work!
I am sorry, the review took quite some time. I don't know this part of the codebase as much as I'd like to and I had to read through rustc's exhaustiveness mechanism to understand most of it. Your comments on the PR were really helpful, thank you!

I've highlighted a few nits that you could change but this shall not overshade your impressive work.

P-E-P · 2024-09-06T13:11:37Z

gcc/rust/checks/errors/rust-hir-pattern-analysis.cc

+{}
+
+void
+PatternChecker::visit (BorrowExpr &expr)


We really need a default Hir visitor somewhere, this file could be half as long with it. I do not think this should happen in this PR as it probably requires a huge amount of refactor.

P-E-P · 2024-09-10T13:42:23Z

gcc/rust/checks/errors/rust-hir-pattern-analysis.h

+    struct
+    {
+      int64_t lo;
+      int64_t hi;
+    } int_range;


We'll have to support more variants (floating point ranges are going away but we need to support them), we should probably use a struct with a fully qualified name in this union.

gcc/rust/checks/errors/rust-hir-pattern-analysis.h

gcc/rust/ChangeLog: * Make-lang.in: Add rust-hir-pattern-analysis.o. * rust-session-manager.cc (Session::compile_crate): Add pattern analysis pass. * typecheck/rust-hir-type-check-pattern.cc (TypeCheckPattern::visit): Do typecheck for subpatterns. * checks/errors/rust-hir-pattern-analysis.cc: New file. * checks/errors/rust-hir-pattern-analysis.h: New file. gcc/testsuite/ChangeLog: * rust/compile/exhaustiveness1.rs: New test. * rust/compile/exhaustiveness2.rs: New test. * rust/compile/exhaustiveness3.rs: New test. Signed-off-by: Raiki Tamura <tamaron1203@gmail.com>

tamaroning · 2024-09-11T12:46:19Z

Thank you for your review! I have fixed the code.

P-E-P

Great work!

tamaroning force-pushed the feat/pattern-analysis branch 3 times, most recently from 554b585 to e17919f Compare August 7, 2024 17:27

tamaroning changed the title ~~pattern analysis (wip)~~ pattern analysis Aug 7, 2024

tamaroning marked this pull request as ready for review August 7, 2024 17:38

P-E-P self-requested a review August 7, 2024 20:44

tamaroning force-pushed the feat/pattern-analysis branch from e17919f to 2961dea Compare August 7, 2024 23:55

tamaroning force-pushed the feat/pattern-analysis branch from 2961dea to 94e6fa2 Compare August 22, 2024 14:43

tamaroning commented Aug 29, 2024

View reviewed changes

P-E-P approved these changes Sep 10, 2024

View reviewed changes

tamaroning force-pushed the feat/pattern-analysis branch from 94e6fa2 to 9020cc0 Compare September 11, 2024 12:42

P-E-P approved these changes Sep 11, 2024

View reviewed changes

P-E-P added this pull request to the merge queue Sep 11, 2024

Merged via the queue into Rust-GCC:master with commit 9941d6f Sep 11, 2024
11 of 12 checks passed

This was referenced Sep 14, 2024

Change HIR visitor pattern #3160

Open

Exhaustiveness check #3172

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pattern analysis #3115

pattern analysis #3115

tamaroning commented Aug 2, 2024 •

edited

Loading

tamaroning commented Aug 7, 2024

tamaroning commented Aug 8, 2024

tamaroning commented Aug 22, 2024 •

edited

Loading

P-E-P commented Aug 29, 2024

tamaroning commented Aug 29, 2024

tamaroning commented Aug 29, 2024

tamaroning Aug 29, 2024 •

edited

Loading

tamaroning Aug 29, 2024

tamaroning Aug 29, 2024

tamaroning Aug 29, 2024 •

edited

Loading

P-E-P left a comment

P-E-P Sep 6, 2024

tamaroning Sep 11, 2024

P-E-P Sep 10, 2024

tamaroning commented Sep 11, 2024

P-E-P left a comment

pattern analysis #3115

pattern analysis #3115

Conversation

tamaroning commented Aug 2, 2024 • edited Loading

tamaroning commented Aug 7, 2024

tamaroning commented Aug 8, 2024

tamaroning commented Aug 22, 2024 • edited Loading

P-E-P commented Aug 29, 2024

tamaroning commented Aug 29, 2024

tamaroning commented Aug 29, 2024

tamaroning Aug 29, 2024 • edited Loading

Choose a reason for hiding this comment

tamaroning Aug 29, 2024

Choose a reason for hiding this comment

tamaroning Aug 29, 2024

Choose a reason for hiding this comment

tamaroning Aug 29, 2024 • edited Loading

Choose a reason for hiding this comment

P-E-P left a comment

Choose a reason for hiding this comment

P-E-P Sep 6, 2024

Choose a reason for hiding this comment

tamaroning Sep 11, 2024

Choose a reason for hiding this comment

P-E-P Sep 10, 2024

Choose a reason for hiding this comment

tamaroning commented Sep 11, 2024

P-E-P left a comment

Choose a reason for hiding this comment

tamaroning commented Aug 2, 2024 •

edited

Loading

tamaroning commented Aug 22, 2024 •

edited

Loading

tamaroning Aug 29, 2024 •

edited

Loading

tamaroning Aug 29, 2024 •

edited

Loading