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

Compiler error when compiling the IfLet expression. #1177

Open
antego opened this issue Apr 26, 2022 · 11 comments
Open

Compiler error when compiling the IfLet expression. #1177

antego opened this issue Apr 26, 2022 · 11 comments
Assignees
Labels

Comments

@antego
Copy link
Contributor

antego commented Apr 26, 2022

related to #1170

Compilation fails for the IfLet expression.

Code

fn main() -> i32 {
    let mut res = 0;

    enum E {
        X(u8),
    }
    let v = E::X(4);
    if let E::X(n) = v {
        res = n;
    }

    0
}

Meta

Error output

FAIL: rust/compile/macro43.rs (internal compiler error: in append_reference_for_def, at rust/resolve/rust-name-resolver.h:227)```
@antego antego added the bug label Apr 26, 2022
@CohenArthur CohenArthur added this to To do in Imports and Visbility via automation Apr 26, 2022
@CohenArthur CohenArthur added this to the Imports and visibility milestone Apr 26, 2022
@CohenArthur CohenArthur moved this from To do to Additional Sprint items in Imports and Visbility Apr 26, 2022
@antego
Copy link
Contributor Author

antego commented May 1, 2022

AST dump

Crate: 
 inner attributes: none
 items: 
  
i32 main()
BlockExpr:

  outer attributes: none
  inner attributes: none
 statements: 
 
  outer attributes: none
 let mut res = 0
 E
 Generic params: none
 Where clause: none
 Items: 
  X(
  outer attributes: none
 u8)

  outer attributes: none
 let v = CallExpr: 
  outer attributes: none
 Function expr: E::X
 Call params:
  4
  ExprStmtWithBlock: 
IfLetExpr: 
    outer attributes: none
 Condition match arm patterns: 
  TupleStructPattern: 
 Path: E::X
 Tuple struct items: 
  n
 Scrutinee expr: v
 If let block expr:    BlockExpr:
   
     outer attributes: none
     inner attributes: none
    statements: 
    ExprStmtWithoutBlock:
     AssignmentExpr: 
 left: res
 right: n
    final expression: none
   
 final expression: 
0

@antego
Copy link
Contributor Author

antego commented May 1, 2022

HIR dump

HIR::Crate: 
 inner attributes: none
 items: 
  private 
i32 main()
BlockExpr:
{
 outer attributes: none
 inner attributes: none
 statements: 
 Outer attributes: none
 let mut res = ( 0 ([C: 0 Nid: 6 Hid: 23]))
 private E
 Generic params: none
 Where clause: none
 Items: 
  X [Tuple variant](outer attributes: none
private u8)
 Outer attributes: none
 let v = E::X::[C: 0 Nid: 24 Hid: 33](( 4 ([C: 0 Nid: 25 Hid: 34])),)::[C: 0 Hid: 35]
  ExprStmtWithBlock: 
none (this should not happen and is an error)
 final expression: 
( 0 ([C: 0 Nid: 55 Hid: 38]))
}::[C: 0 Nid: 57 Hid: 40 Lid: 6]

::[C: 0 Hid: 41]

@antego
Copy link
Contributor Author

antego commented May 1, 2022

@CohenArthur would love to fix this issue but not sure how to proceed.

Looks like the AST lowering didn't correctly lower the if let expression? This part of the HIR is suspicious:

ExprStmtWithBlock: 
none (this should not happen and is an error)

@CohenArthur
Copy link
Member

Yes, that looks like a good assumption. You can look into the AST lowering pass, which is located in gcc/rust/hir/rust-ast-lower*. The code responsible for lowering AST::If* nodes is in rust-ast-lower-expr.h, and it is missing a lowerer for AST::IfLetExpr so that might be a track to follow.

I can assign you this issue and you see how you feel about solving it? We can always fix it together or have someone else fix it if it doesn't interest you anymore at some point :)

@antego
Copy link
Contributor Author

antego commented May 3, 2022

Thanks! I'll take a look.

@philberty
Copy link
Member

Hi @antego would you like me to setup a guide for you on this? You have already found one of the main missing pieces which is HIR lowering.

Though there are several parts that are need resolved when fixing this issue:

  • name resolution
  • hir-lowering
  • type resolution
  • code-generation

Name resolution will ensure we setup the proper conventions for resolveing the new bindings created in the if-let expr but we also need to ensure that the correct scoping is applied here.

HIR lowering you already have a PR out for

Type resolution is about setting up all the relevant type information for this bloc.

Code generation is all about generating the GENERIC for this.

@antego
Copy link
Contributor Author

antego commented May 5, 2022

Yeah, I've created a draft PR with a commit that I can reference here and ask for the further help 😂

Hi @antego would you like me to setup a guide for you on this?

Yes please!

but we also need to ensure that the correct scoping is applied here.

Yeah I've seen that the error happens around the place that iterates through Ribs so I started reading this. But I still feel lost about what is actually happening here.

@antego
Copy link
Contributor Author

antego commented May 6, 2022

@philberty could you please give some pointers for where the code for the name resolution, type resolution and code generation is located? So that I could check how it works for the IfExpr for example.

antego added a commit to antego/gccrs that referenced this issue May 7, 2022
antego added a commit to antego/gccrs that referenced this issue May 7, 2022
@antego
Copy link
Contributor Author

antego commented May 7, 2022

Is that right that the name resolution should be implemented in rust-ast-resolve-expr.cc? And type resolution in rust-hir-type-check-expr.h?

@philberty
Copy link
Member

philberty commented May 9, 2022

Hi, @antego Yep name resolution should be fairly simple here, You should be able to copy how we do name resolution for the if blocks from that file. Though if let statements add a new binding and scope level so you need to declare the pattern so you should be able to see how that is done from rust-ast-resolve-stmt.

It's possible to do each of these things in separate PR's so the HIR lowering piece is definitely something that can be merged now independently.

antego added a commit to antego/gccrs that referenced this issue May 9, 2022
bors bot added a commit that referenced this issue May 10, 2022
1218: Lower IfLet expressions r=CohenArthur a=antego

This PR addresses #1177. This change implements the "hir-lowering" part of the plan outlined here #1177 (comment).

Co-authored-by: antego <antego@users.noreply.github.com>
antego added a commit to antego/gccrs that referenced this issue May 10, 2022
antego added a commit to antego/gccrs that referenced this issue May 10, 2022
antego added a commit to antego/gccrs that referenced this issue May 10, 2022
bors bot added a commit that referenced this issue May 23, 2022
1241: Implement name resolution for the IfLet expression. r=philberty a=antego

Addresses #1177.

Guidance from the ticket #1177:

> You should be able to copy how we do name resolution for the if blocks from that file. Though if let statements add a new binding and scope level so you need to declare the pattern so you should be able to see how that is done from rust-ast-resolve-stmt.

I don't understand how to modify the block expression resolution so that it can handle the `IfLet` expression. For now, I copied the code from the `MatchExpr` resolution. Not sure how to test it either and what is the desired expected result of the name resolution so I just hope that reviewers will spot the errors.

I created this PR in order to get some guidance about how to proceed with it. Thanks!



Co-authored-by: antego <antego@users.noreply.github.com>
bors bot added a commit that referenced this issue May 25, 2022
1279: Implement type checking for the `IfLet` expression. r=philberty a=antego

Addresses #1177.

This change implements the type check for the `IfLet` expression. This is the adapted type checking code from the `MatchExpr`.

Tested on this code
```
enum E {
    X(u8),
}

fn main() -> i32 {
    let mut res = 0;
    let v = E::X(4);
    if let E::X(n) = v {
        res = n;
    }

    0
}
```
Compilation finishes without errors.

Next is the implementation of the code generation but I'll need help to find where it's located.

Co-authored-by: antego <antego@users.noreply.github.com>
@philberty philberty removed this from Additional Sprint items in Imports and Visbility May 25, 2022
@philberty philberty removed this from the Imports and visibility milestone May 25, 2022
@dkm
Copy link
Member

dkm commented Jun 3, 2024

FTR, some work on if-let expr was done in 8664ba4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants