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

Fix variable shadowing in late resolution 2.0 #3226

Merged
merged 1 commit into from
Oct 27, 2024

Conversation

powerboat9
Copy link
Contributor

No description provided.

Copy link
Member

@P-E-P P-E-P left a comment

Choose a reason for hiding this comment

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

Ok, I had trouble understanding why the order was important here. For reference in the future here is an extract of what this PR is fixing:
issue-1930.rs:

// { dg-options "-w" }
#[lang = "sized"]
pub trait Sized {}

fn test<T>(x: *mut T) {
    let x = x as *mut u8;
}

issue-1813.rs:

fn main() {
    let a = 15u8;
    let a = &a;
    match a {
        &15 => {}
        &14 => {}
        _ => {}
    }
}

error:

issue-1813.rs:3:14: error: cannot find value ‘a’ in this scope [E0425]
    3 |     let a = &a;
      |              ^
crab1: internal compiler error: in TyVar, at rust/typecheck/rust-tyty-util.cc:31

Both those issues shadow a variable and visiting the pattern before the expression makes the new name resolution ICE because the identifier refers to the new binding being created and not the one being shadowed.

Whilst I do think it is indeed the correct fix, I'm unsure we should change the default visitor for this. This problem is solely tied to name resolution, I think we need to overload the default resolver in the name resolution visitor so no other part of the code will be affected by this change (I guess people expect the default visitor to visit the items in order as in the code ?) and put a comment to show future contributor the intents of this weird order so it isn't too confusing.

gcc/rust/ChangeLog:

	* resolve/rust-late-name-resolver-2.0.cc
	(Late::visit): Visit the initialization expressions of let
	statements before visiting their patterns.

gcc/testsuite/ChangeLog:

	* rust/compile/nr2/exclude: Remove entries.

Signed-off-by: Owen Avery <[email protected]>
@powerboat9 powerboat9 changed the title Adjust default AST::LetStmt visitor Fix variable shadowing in late resolution 2.0 Oct 27, 2024
@powerboat9
Copy link
Contributor Author

Alright, should be addressed

@powerboat9 powerboat9 requested a review from P-E-P October 27, 2024 18:43
@P-E-P P-E-P added this pull request to the merge queue Oct 27, 2024
Merged via the queue into Rust-GCC:master with commit e52fa08 Oct 27, 2024
12 checks passed
@powerboat9 powerboat9 deleted the reorder-let branch October 28, 2024 03:04
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.

2 participants