Skip to content

nr2.0: Add more checks for alternate patterns #3829

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

nr2.0: Add more checks for alternate patterns #3829

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions gcc/rust/resolve/rust-late-name-resolver-2.0.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ Late::visit (AST::LetStmt &let)
static void
visit_identifier_as_pattern (NameResolutionContext &ctx,
const Identifier &ident, location_t locus,
NodeId node_id)
NodeId node_id, bool is_ref, bool is_mut)
{
// do we insert in labels or in values
// but values does not allow shadowing... since functions cannot shadow
Expand All @@ -232,7 +232,7 @@ visit_identifier_as_pattern (NameResolutionContext &ctx,
return;
}

ctx.bindings.peek ().insert_ident (ident);
ctx.bindings.peek ().insert_ident (ident.as_string (), locus, is_ref, is_mut);

if (ctx.bindings.peek ().is_or_bound (ident))
{
Expand All @@ -258,7 +258,9 @@ Late::visit (AST::IdentifierPattern &identifier)

visit_identifier_as_pattern (ctx, identifier.get_ident (),
identifier.get_locus (),
identifier.get_node_id ());
identifier.get_node_id (),
Copy link
Member

@P-E-P P-E-P Jun 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

At this point we'd better pass the identifier down the function instead of it's fields separately.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That would require some modifications to AST::StructPatternFieldIdent -- which I suppose I could do

Copy link
Member

@P-E-P P-E-P Jun 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I've taken a closer look and I'm not sure it's worth the changes.

identifier.get_is_ref (),
identifier.get_is_mut ());
}

void
Expand Down Expand Up @@ -289,7 +291,8 @@ void
Late::visit (AST::StructPatternFieldIdent &field)
{
visit_identifier_as_pattern (ctx, field.get_identifier (), field.get_locus (),
field.get_node_id ());
field.get_node_id (), field.is_ref (),
field.is_mut ());
}

void
Expand Down
59 changes: 53 additions & 6 deletions gcc/rust/resolve/rust-name-resolution-context.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ BindingLayer::bind_test (Identifier ident, Binding::Kind kind)
{
for (auto &bind : bindings)
{
if (bind.set.find (ident) != bind.set.cend () && bind.kind == kind)
if (bind.idents.find (ident.as_string ()) != bind.idents.cend ()
&& bind.kind == kind)
{
return true;
}
Expand All @@ -60,20 +61,66 @@ BindingLayer::is_or_bound (Identifier ident)
}

void
BindingLayer::insert_ident (Identifier ident)
BindingLayer::insert_ident (std::string ident, location_t locus, bool is_ref,
bool is_mut)
{
bindings.back ().set.insert (ident);
bindings.back ().idents.emplace (
std::move (ident), std::make_pair (locus, IdentifierMode (is_ref, is_mut)));
}

void
BindingLayer::merge ()
{
auto last_binding = bindings.back ();
auto last_binding = std::move (bindings.back ());
bindings.pop_back ();
for (auto &value : last_binding.set)

if (bindings.back ().has_expected_bindings)
{
bindings.back ().set.insert (value);
for (auto &value : bindings.back ().idents)
{
auto ident = value.first;
if (last_binding.idents.find (ident) == last_binding.idents.end ())
{
location_t locus = value.second.first;
rust_error_at (locus, ErrorCode::E0408,
"variable %qs is not bound in all patterns",
ident.c_str ());
}
}
}

for (auto &value : last_binding.idents)
{
auto res = bindings.back ().idents.emplace (value);
if (res.second)
{
if (bindings.back ().has_expected_bindings)
{
auto &ident = value.first;
location_t locus = value.second.first;
rust_error_at (locus, ErrorCode::E0408,
"variable %qs is not bound in all patterns",
ident.c_str ());
}
}
else
{
auto this_mode = value.second.second;
auto other_mode = res.first->second.second;
if (this_mode != other_mode)
{
auto &ident = value.first;
location_t locus = value.second.first;
rust_error_at (locus, ErrorCode::E0409,
"variable %qs is bound inconsistently across "
"pattern alternatives",
ident.c_str ());
}
}
}

if (bindings.back ().kind == Binding::Kind::Or)
bindings.back ().has_expected_bindings = true;
}

BindingSource
Expand Down
26 changes: 23 additions & 3 deletions gcc/rust/resolve/rust-name-resolution-context.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,22 @@ class Definition
NodeId id;
};

struct IdentifierMode
{
bool is_ref;
bool is_mut;

IdentifierMode (bool is_ref, bool is_mut) : is_ref (is_ref), is_mut (is_mut)
{}

bool operator== (const IdentifierMode &other)
{
return other.is_ref == is_ref && other.is_mut == is_mut;
}

bool operator!= (const IdentifierMode &other) { return !(*this == other); }
};

struct Binding
{
enum class Kind
Expand All @@ -166,9 +182,12 @@ struct Binding
Or,
} kind;

std::unordered_set<Identifier> set;
// used to check the correctness of or-bindings
bool has_expected_bindings;

std::unordered_map<std::string, std::pair<location_t, IdentifierMode>> idents;

Binding (Binding::Kind kind) : kind (kind) {}
Binding (Binding::Kind kind) : kind (kind), has_expected_bindings (false) {}
};

/**
Expand Down Expand Up @@ -208,7 +227,8 @@ class BindingLayer
*/
bool is_or_bound (Identifier ident);

void insert_ident (Identifier ident);
void insert_ident (std::string ident, location_t locus, bool is_ref,
bool is_mut);

void merge ();

Expand Down
1 change: 0 additions & 1 deletion gcc/testsuite/rust/compile/nr2/exclude
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
issue-3315-2.rs
torture/alt_patterns1.rs
issue-1487.rs
issue-2015.rs
issue-3454.rs
Expand Down