From 29ae46d81a03fae6b21b1064bc5a1c44ae609681 Mon Sep 17 00:00:00 2001 From: Arthur Cohen Date: Wed, 5 Mar 2025 15:31:56 +0000 Subject: [PATCH 1/6] ast: Add optional diverging else to AST::LetStmt gcc/rust/ChangeLog: * ast/rust-stmt.h (class LetStmt): Add optional expression for diverging else. * ast/rust-ast-builder.cc (Builder::let): Use new API. --- gcc/rust/ast/rust-ast-builder.cc | 3 ++- gcc/rust/ast/rust-stmt.h | 29 ++++++++++++++++++++++++++++- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/gcc/rust/ast/rust-ast-builder.cc b/gcc/rust/ast/rust-ast-builder.cc index 58c37a5aa5f7..2e3685f2b198 100644 --- a/gcc/rust/ast/rust-ast-builder.cc +++ b/gcc/rust/ast/rust-ast-builder.cc @@ -17,6 +17,7 @@ // . #include "rust-ast-builder.h" +#include "optional.h" #include "rust-ast-builder-type.h" #include "rust-ast.h" #include "rust-common.h" @@ -352,7 +353,7 @@ Builder::let (std::unique_ptr &&pattern, std::unique_ptr &&type, { return std::unique_ptr (new LetStmt (std::move (pattern), std::move (init), std::move (type), - {}, loc)); + tl::nullopt, {}, loc)); } std::unique_ptr diff --git a/gcc/rust/ast/rust-stmt.h b/gcc/rust/ast/rust-stmt.h index afe48f271c19..e64fbe4acd0a 100644 --- a/gcc/rust/ast/rust-stmt.h +++ b/gcc/rust/ast/rust-stmt.h @@ -19,6 +19,7 @@ #ifndef RUST_AST_STATEMENT_H #define RUST_AST_STATEMENT_H +#include "optional.h" #include "rust-ast.h" #include "rust-path.h" #include "rust-expr.h" @@ -72,6 +73,8 @@ class LetStmt : public Stmt // bool has_init_expr; std::unique_ptr init_expr; + tl::optional> else_expr; + location_t locus; public: @@ -85,15 +88,18 @@ class LetStmt : public Stmt // Returns whether let statement has an initialisation expression. bool has_init_expr () const { return init_expr != nullptr; } + bool has_else_expr () const { return else_expr.has_value (); } std::string as_string () const override; LetStmt (std::unique_ptr variables_pattern, std::unique_ptr init_expr, std::unique_ptr type, + tl::optional> else_expr, std::vector outer_attrs, location_t locus) : outer_attrs (std::move (outer_attrs)), variables_pattern (std::move (variables_pattern)), - type (std::move (type)), init_expr (std::move (init_expr)), locus (locus) + type (std::move (type)), init_expr (std::move (init_expr)), + else_expr (std::move (else_expr)), locus (locus) {} // Copy constructor with clone @@ -107,6 +113,9 @@ class LetStmt : public Stmt // guard to prevent null dereference (always required) if (other.init_expr != nullptr) init_expr = other.init_expr->clone_expr (); + if (other.else_expr.has_value ()) + else_expr = other.else_expr.value ()->clone_expr (); + if (other.type != nullptr) type = other.type->clone_type (); } @@ -128,6 +137,12 @@ class LetStmt : public Stmt init_expr = other.init_expr->clone_expr (); else init_expr = nullptr; + + if (other.else_expr != nullptr) + else_expr = other.else_expr.value ()->clone_expr (); + else + else_expr = tl::nullopt; + if (other.type != nullptr) type = other.type->clone_type (); else @@ -162,12 +177,24 @@ class LetStmt : public Stmt return *init_expr; } + Expr &get_else_expr () + { + rust_assert (has_else_expr ()); + return *else_expr.value (); + } + std::unique_ptr &get_init_expr_ptr () { rust_assert (has_init_expr ()); return init_expr; } + std::unique_ptr &get_else_expr_ptr () + { + rust_assert (has_else_expr ()); + return else_expr.value (); + } + Pattern &get_pattern () { rust_assert (variables_pattern != nullptr); From 5c6aeb8305e5723419cd69c7db29d886c1fe60b8 Mon Sep 17 00:00:00 2001 From: Arthur Cohen Date: Wed, 5 Mar 2025 15:30:04 +0000 Subject: [PATCH 2/6] parser: Parse let-else statements gcc/rust/ChangeLog: * parse/rust-parse-impl.h (Parser::parse_let_stmt): Add new parsing in case of `else` token. --- gcc/rust/parse/rust-parse-impl.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/gcc/rust/parse/rust-parse-impl.h b/gcc/rust/parse/rust-parse-impl.h index a1b22c0a4168..bd5011329142 100644 --- a/gcc/rust/parse/rust-parse-impl.h +++ b/gcc/rust/parse/rust-parse-impl.h @@ -6163,6 +6163,10 @@ Parser::parse_let_stmt (AST::AttrVec outer_attrs, } } + tl::optional> else_expr = tl::nullopt; + if (maybe_skip_token (ELSE)) + else_expr = parse_block_expr (); + if (restrictions.consume_semi) { // `stmt` macro variables are parsed without a semicolon, but should be @@ -6177,7 +6181,7 @@ Parser::parse_let_stmt (AST::AttrVec outer_attrs, return std::unique_ptr ( new AST::LetStmt (std::move (pattern), std::move (expr), std::move (type), - std::move (outer_attrs), locus)); + std::move (else_expr), std::move (outer_attrs), locus)); } // Parses a type path. From 3c94cda704a0c2e3e9918d48787e9acb2802d152 Mon Sep 17 00:00:00 2001 From: Arthur Cohen Date: Wed, 5 Mar 2025 15:33:31 +0000 Subject: [PATCH 3/6] dump: Handle let-else properly gcc/rust/ChangeLog: * ast/rust-ast-collector.cc (TokenCollector::visit): Add handling for diverging else expression. --- gcc/rust/ast/rust-ast-collector.cc | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/gcc/rust/ast/rust-ast-collector.cc b/gcc/rust/ast/rust-ast-collector.cc index b4d565cadeb5..d0ecedb0c313 100644 --- a/gcc/rust/ast/rust-ast-collector.cc +++ b/gcc/rust/ast/rust-ast-collector.cc @@ -22,6 +22,7 @@ #include "rust-expr.h" #include "rust-item.h" #include "rust-keyword-values.h" +#include "rust-location.h" #include "rust-path.h" #include "rust-system.h" #include "rust-token.h" @@ -2587,6 +2588,13 @@ TokenCollector::visit (LetStmt &stmt) push (Rust::Token::make (EQUAL, UNDEF_LOCATION)); visit (stmt.get_init_expr ()); } + + if (stmt.has_else_expr ()) + { + push (Rust::Token::make (ELSE, UNDEF_LOCATION)); + visit (stmt.get_else_expr ()); + } + push (Rust::Token::make (SEMICOLON, UNDEF_LOCATION)); } From 926331f22165834aac88838364fc83fd39e50da4 Mon Sep 17 00:00:00 2001 From: Arthur Cohen Date: Wed, 5 Mar 2025 15:34:25 +0000 Subject: [PATCH 4/6] name-resolution: Handle let-else properly gcc/rust/ChangeLog: * resolve/rust-ast-resolve-stmt.h: Add handling for diverging else. * resolve/rust-late-name-resolver-2.0.cc (Late::visit): Likewise. --- gcc/rust/resolve/rust-ast-resolve-stmt.h | 7 ++++--- gcc/rust/resolve/rust-late-name-resolver-2.0.cc | 3 +++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/gcc/rust/resolve/rust-ast-resolve-stmt.h b/gcc/rust/resolve/rust-ast-resolve-stmt.h index 5d5891043627..a1261a070c1c 100644 --- a/gcc/rust/resolve/rust-ast-resolve-stmt.h +++ b/gcc/rust/resolve/rust-ast-resolve-stmt.h @@ -73,9 +73,10 @@ class ResolveStmt : public ResolverBase void visit (AST::LetStmt &stmt) override { if (stmt.has_init_expr ()) - { - ResolveExpr::go (stmt.get_init_expr (), prefix, canonical_prefix); - } + ResolveExpr::go (stmt.get_init_expr (), prefix, canonical_prefix); + + if (stmt.has_else_expr ()) + ResolveExpr::go (stmt.get_else_expr (), prefix, canonical_prefix); PatternDeclaration::go (stmt.get_pattern (), Rib::ItemType::Var); if (stmt.has_type ()) diff --git a/gcc/rust/resolve/rust-late-name-resolver-2.0.cc b/gcc/rust/resolve/rust-late-name-resolver-2.0.cc index d1c10e51a082..5fcf0ec27cde 100644 --- a/gcc/rust/resolve/rust-late-name-resolver-2.0.cc +++ b/gcc/rust/resolve/rust-late-name-resolver-2.0.cc @@ -140,6 +140,9 @@ Late::visit (AST::LetStmt &let) visit (let.get_init_expr ()); visit (let.get_pattern ()); + if (let.has_else_expr ()) + visit (let.get_init_expr ()); + // how do we deal with the fact that `let a = blipbloup` should look for a // label and cannot go through function ribs, but `let a = blipbloup()` can? From e7050005c2c370fa951fef83699aea43ce976da5 Mon Sep 17 00:00:00 2001 From: Arthur Cohen Date: Wed, 5 Mar 2025 15:36:19 +0000 Subject: [PATCH 5/6] lower: Handle let-else properly gcc/rust/ChangeLog: * hir/tree/rust-hir-stmt.h (class LetStmt): Add optional diverging else expression. * hir/tree/rust-hir-stmt.cc: Likewise. * hir/rust-ast-lower-stmt.cc (ASTLoweringStmt::visit): Add handling for lowering diverging else. --- gcc/rust/hir/rust-ast-lower-stmt.cc | 14 ++++++++++---- gcc/rust/hir/tree/rust-hir-stmt.cc | 12 +++++++++++- gcc/rust/hir/tree/rust-hir-stmt.h | 16 ++++++++++++++++ 3 files changed, 37 insertions(+), 5 deletions(-) diff --git a/gcc/rust/hir/rust-ast-lower-stmt.cc b/gcc/rust/hir/rust-ast-lower-stmt.cc index 761a638a80e6..0d3947cba2fa 100644 --- a/gcc/rust/hir/rust-ast-lower-stmt.cc +++ b/gcc/rust/hir/rust-ast-lower-stmt.cc @@ -76,20 +76,26 @@ ASTLoweringStmt::visit (AST::LetStmt &stmt) type = std::unique_ptr (ASTLoweringType::translate (stmt.get_type ())); - tl::optional> init_expression = tl::nullopt; + tl::optional> init_expr = tl::nullopt; + tl::optional> else_expr = tl::nullopt; if (stmt.has_init_expr ()) - init_expression = std::unique_ptr ( + init_expr = std::unique_ptr ( ASTLoweringExpr::translate (stmt.get_init_expr ())); + if (stmt.has_else_expr ()) + else_expr = std::unique_ptr ( + ASTLoweringExpr::translate (stmt.get_else_expr ())); + auto crate_num = mappings.get_current_crate (); Analysis::NodeMapping mapping (crate_num, stmt.get_node_id (), mappings.get_next_hir_id (crate_num), UNKNOWN_LOCAL_DEFID); translated = new HIR::LetStmt (mapping, std::unique_ptr (variables), - std::move (init_expression), std::move (type), - stmt.get_outer_attrs (), stmt.get_locus ()); + std::move (init_expr), std::move (else_expr), + std::move (type), stmt.get_outer_attrs (), + stmt.get_locus ()); } void diff --git a/gcc/rust/hir/tree/rust-hir-stmt.cc b/gcc/rust/hir/tree/rust-hir-stmt.cc index 025f67e2c9b1..fd58e29c7f8d 100644 --- a/gcc/rust/hir/tree/rust-hir-stmt.cc +++ b/gcc/rust/hir/tree/rust-hir-stmt.cc @@ -26,11 +26,13 @@ namespace HIR { LetStmt::LetStmt (Analysis::NodeMapping mappings, std::unique_ptr variables_pattern, tl::optional> init_expr, + tl::optional> else_expr, tl::optional> type, AST::AttrVec outer_attrs, location_t locus) : Stmt (std::move (mappings)), outer_attrs (std::move (outer_attrs)), variables_pattern (std::move (variables_pattern)), type (std::move (type)), - init_expr (std::move (init_expr)), locus (locus) + init_expr (std::move (init_expr)), else_expr (std::move (else_expr)), + locus (locus) {} LetStmt::LetStmt (LetStmt const &other) @@ -43,6 +45,8 @@ LetStmt::LetStmt (LetStmt const &other) // guard to prevent null dereference (always required) if (other.has_init_expr ()) init_expr = other.get_init_expr ().clone_expr (); + if (other.has_else_expr ()) + else_expr = other.get_else_expr ().clone_expr (); if (other.has_type ()) type = other.get_type ().clone_type (); @@ -67,6 +71,12 @@ LetStmt::operator= (LetStmt const &other) init_expr = other.get_init_expr ().clone_expr (); else init_expr = nullptr; + + if (other.has_else_expr ()) + else_expr = other.get_else_expr ().clone_expr (); + else + else_expr = tl::nullopt; + if (other.has_type ()) type = other.get_type ().clone_type (); else diff --git a/gcc/rust/hir/tree/rust-hir-stmt.h b/gcc/rust/hir/tree/rust-hir-stmt.h index 1e17f047d3e2..bfa8fe1e15bf 100644 --- a/gcc/rust/hir/tree/rust-hir-stmt.h +++ b/gcc/rust/hir/tree/rust-hir-stmt.h @@ -101,6 +101,7 @@ class LetStmt : public Stmt tl::optional> type; tl::optional> init_expr; + tl::optional> else_expr; location_t locus; @@ -113,12 +114,15 @@ class LetStmt : public Stmt // Returns whether let statement has an initialisation expression. bool has_init_expr () const { return init_expr.has_value (); } + // Returns whether let statement has a diverging else expression. + bool has_else_expr () const { return else_expr.has_value (); } std::string as_string () const override; LetStmt (Analysis::NodeMapping mappings, std::unique_ptr variables_pattern, tl::optional> init_expr, + tl::optional> else_expr, tl::optional> type, AST::AttrVec outer_attrs, location_t locus); @@ -167,6 +171,18 @@ class LetStmt : public Stmt return *init_expr.value (); } + HIR::Expr &get_else_expr () + { + rust_assert (*else_expr); + return *else_expr.value (); + } + + const HIR::Expr &get_else_expr () const + { + rust_assert (*else_expr); + return *else_expr.value (); + } + HIR::Pattern &get_pattern () { return *variables_pattern; } bool is_item () const override final { return false; } From cfd49f30f858e75ec01c36f699e6c451b5dae6b4 Mon Sep 17 00:00:00 2001 From: Arthur Cohen Date: Wed, 5 Mar 2025 15:37:48 +0000 Subject: [PATCH 6/6] tychk: Ensure diverging else of let-else is actually diverging Make sure the `else` expression resolves to the never type. gcc/rust/ChangeLog: * typecheck/rust-hir-type-check-stmt.cc (TypeCheckStmt::visit): Check diverging else against `!`. gcc/testsuite/ChangeLog: * rust/compile/let-else-invalid-type.rs: New test. --- .../typecheck/rust-hir-type-check-stmt.cc | 19 +++++++++++++++++++ .../rust/compile/let-else-invalid-type.rs | 8 ++++++++ 2 files changed, 27 insertions(+) create mode 100644 gcc/testsuite/rust/compile/let-else-invalid-type.rs diff --git a/gcc/rust/typecheck/rust-hir-type-check-stmt.cc b/gcc/rust/typecheck/rust-hir-type-check-stmt.cc index 87a7733848a1..aa50e257374c 100644 --- a/gcc/rust/typecheck/rust-hir-type-check-stmt.cc +++ b/gcc/rust/typecheck/rust-hir-type-check-stmt.cc @@ -17,12 +17,14 @@ // . #include "rust-hir-type-check-stmt.h" +#include "rich-location.h" #include "rust-hir-type-check-type.h" #include "rust-hir-type-check-expr.h" #include "rust-hir-type-check-implitem.h" #include "rust-hir-type-check-item.h" #include "rust-hir-type-check-pattern.h" #include "rust-type-util.h" +#include "rust-tyty.h" namespace Rust { namespace Resolver { @@ -78,6 +80,7 @@ TypeCheckStmt::visit (HIR::LetStmt &stmt) auto &stmt_pattern = stmt.get_pattern (); TyTy::BaseType *init_expr_ty = nullptr; location_t init_expr_locus = UNKNOWN_LOCATION; + if (stmt.has_init_expr ()) { init_expr_locus = stmt.get_init_expr ().get_locus (); @@ -97,6 +100,22 @@ TypeCheckStmt::visit (HIR::LetStmt &stmt) specified_ty_locus = stmt.get_type ().get_locus (); } + if (stmt.has_else_expr ()) + { + auto &else_expr = stmt.get_else_expr (); + auto else_expr_ty = TypeCheckExpr::Resolve (else_expr); + + if (else_expr_ty->get_kind () != TyTy::TypeKind::NEVER) + { + rust_error_at (else_expr.get_locus (), + "% clause of let-else does not diverge"); + return; + } + + // FIXME: Is that enough? Do we need to do something like + // `append_reference` here as well? + } + // let x:i32 = 123; if (specified_ty != nullptr && init_expr_ty != nullptr) { diff --git a/gcc/testsuite/rust/compile/let-else-invalid-type.rs b/gcc/testsuite/rust/compile/let-else-invalid-type.rs new file mode 100644 index 000000000000..6679e5763949 --- /dev/null +++ b/gcc/testsuite/rust/compile/let-else-invalid-type.rs @@ -0,0 +1,8 @@ +enum FakeOption { + Some(i32), + None, +} + +fn main() { + let FakeOption::Some(a) = FakeOption::None else { FakeOption::Some(15) }; // { dg-error "does not diverge" } +}