Desugar IfLet* expr to match

[dkm]

Replace the "regular" AST->HIR lowering for IfLet* with a desugaring into a MatchExpr. Desugar a simple if let:

   if let Some(y) = some_value {
     bar();
   }

into:

   match some_value {
     Some(y) => {bar();},
     _ => ()
   }

Same applies for IfLetExprConseqElse (if let with an else block).

Desugar:

   if let Some(y) = some_value {
     bar();
   } else {
     baz();
   }

into:

   match some_value {
     Some(y) => {bar();},
     _ => {baz();}
   }

Fixes https://github.com/Rust-GCC/gccrs/issues/1177

[dkm]

@CohenArthur I think this is now ready for review (but of course, no hurry).

[dkm]

Looks like the alpine builder had a network issue: [7] Could not connect to server (Failed to connect to static.crates.io port 443 after 0 ms: Could not connect to server)

[dkm]

Also, I'm not really sure how this impacts error reporting... What if we report an error after the HIR lowering. Do we already have a mechanism to make sure we're not reporting something confusing (e.g. a diagnostic mentioning a match error pointing to the "if let").

[P-E-P]

Also, I'm not really sure how this impacts error reporting... What if we report an error after the HIR lowering. Do we already have a mechanism to make sure we're not reporting something confusing (e.g. a diagnostic mentioning a match error pointing to the "if let").

Mmh good point, I think there is no mention of if let at this stage in the compile process, but the location should still point to the initial if let statement, if an error arise it should be correctly annotated anyway.

[dkm]

Also, I'm not really sure how this impacts error reporting... What if we report an error after the HIR lowering. Do we already have a mechanism to make sure we're not reporting something confusing (e.g. a diagnostic mentioning a match error pointing to the "if let").

Mmh good point, I think there is no mention of if let at this stage in the compile process, but the location should still point to the initial if let statement, if an error arise it should be correctly annotated anyway.

Sure, but I'm afraid we may produce nonsensical diagnostic at some point... I'll try to push the compiler to emit errors around this... But maybe it's not possible by construction... at least for the moment. But we probably need something to identify synthetic construction from the ones that come from the source.

[dkm]

Something like:

• https://github.com/rust-lang/rustc-dev-guide/blob/d39f68ba831f87144b7d2f49d35886a90e562b69/src/diagnostics.md?plain=1#L955 for rustc
• https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/ada/einfo.ads;h=2fb45703a4fb77218637c4f885ede85510326586;hb=HEAD#l663 for GCC Ada.

[dkm]

@CohenArthur I thought the FAIL that this PR has in nr2 test was caused by my change... but it seems it's caused by my new test case.

crab1: internal compiler error: in visit, at rust/resolve/rust-late-name-resolver-2.0.cc:207
0x259fde4 Rust::Resolver2_0::Late::visit(Rust::AST::PathInExpression&)
        ../../gcc/rust/resolve/rust-late-name-resolver-2.0.cc:207
0x237a889 Rust::AST::DefaultASTVisitor::visit(Rust::AST::TupleStructPattern&)                                                                                                 
        ../../gcc/rust/ast/rust-ast-visitor.cc:1231                                                                                                                           
0x2357099 Rust::AST::TupleStructPattern::accept_vis(Rust::AST::ASTVisitor&)       
        ../../gcc/rust/ast/rust-pattern.cc:487
0x237dc25 void Rust::AST::DefaultASTVisitor::visit<Rust::AST::Pattern>(std::unique_ptr<Rust::AST::Pattern, std::default_delete<Rust::AST::Pattern> >&)
        ../../gcc/rust/ast/rust-ast-visitor.h:409                          
0x2377cc0 Rust::AST::DefaultASTVisitor::visit(Rust::AST::IfLetExpr&)                                                                                                          
        ../../gcc/rust/ast/rust-ast-visitor.cc:623
0x2377d63 Rust::AST::DefaultASTVisitor::visit(Rust::AST::IfLetExprConseqElse&)
        ../../gcc/rust/ast/rust-ast-visitor.cc:631          
0x2277c51 Rust::AST::IfLetExprConseqElse::accept_vis(Rust::AST::ASTVisitor&)
        ../../gcc/rust/ast/rust-ast.cc:4645 
0x237c2d9 void Rust::AST::DefaultASTVisitor::visit<Rust::AST::Expr>(Rust::AST::Expr&)
        ../../gcc/rust/ast/rust-ast-visitor.h:405
0x23773bb Rust::AST::DefaultASTVisitor::visit(Rust::AST::BlockExpr&)
        ../../gcc/rust/ast/rust-ast-visitor.cc:461
0x25803a4 operator()

In :

void
Late::visit (AST::PathInExpression &expr)
{
  // TODO: How do we have a nice error with `can't capture dynamic environment
  // in a function item` error here?
  // do we emit it in `get<Namespace::Labels>`?

  rust_debug ("[ARTHUR]: %s", expr.as_simple_path ().as_string ().c_str ());
  auto value = ctx.values.resolve_path (expr.get_segments ());
  if (!value.has_value ())
    rust_unreachable (); // Should have been resolved earlier

Test case :

enum MyOption {
    Some(i32),
    None,
}

pub fn toto(i : MyOption) -> i32 {
    if let MyOption::Some(v) = i {
        v
    } else {
        23i32
    }
}

Seen in https://rust.godbolt.org/z/1dYfWGY5T

[dkm]

@P-E-P do you know if there's a way to mark a test XFAIL only for a given target (nr2)? I don't think I can debug this issue as I know nearly nothing about the name resolution in gccrs, but I think this change could go in as it's not adding any issue, it's simply testing something that's currently already broken in nr2.

[P-E-P]

@P-E-P do you know if there's a way to mark a test XFAIL only for a given target (nr2)? I don't think I can debug this issue as I know nearly nothing about the name resolution in gccrs, but I think this change could go in as it's not adding any issue, it's simply testing something that's currently already broken in nr2.

There is an exclude file containing a list of test that do not pass with nr2 here https://github.com/Rust-GCC/gccrs/blob/master/gcc%2Ftestsuite%2Frust%2Fcompile%2Fnr2%2Fexclude

You can add your failing test, we'll remove it from the list when nr2 will be more complete.

[dkm]

You can add your failing test, we'll remove it from the list when nr2 will be more complete.

oh nice, thanks!