Skip to content

nr2.0: Fix multiple bindings #3702

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

Jump to bottom
Merged
merged 8 commits into from
Apr 24, 2025
Merged

nr2.0: Fix multiple bindings #3702

merged 8 commits into from
Apr 24, 2025
+311 −20

Conversation

P-E-P
Copy link
Member

@P-E-P P-E-P commented Apr 7, 2025

Fixes #3478

@P-E-P P-E-P requested review from CohenArthur and powerboat9 April 7, 2025 08:29
@P-E-P P-E-P changed the title Nr2: Fix multiple bindings nr2: Fix multiple bindings Apr 7, 2025
@P-E-P P-E-P changed the title nr2: Fix multiple bindings nr2.0: Fix multiple bindings Apr 7, 2025
@P-E-P P-E-P force-pushed the nr2-fix-missing-lang-items branch from c55a6c2 to 26ff412 Compare April 7, 2025 10:50
powerboat9
powerboat9 reviewed Apr 7, 2025
View reviewed changes
@P-E-P P-E-P force-pushed the nr2-fix-missing-lang-items branch 9 times, most recently from 7c13a57 to dcc89d8 Compare April 19, 2025 11:51
@P-E-P P-E-P marked this pull request as ready for review April 19, 2025 12:19
@P-E-P
Copy link
Member Author

P-E-P commented Apr 19, 2025 •
edited
Loading

@CohenArthur @philberty shall I drop e169a88 ? I'm unsure about it.

On one hand it allows us to avoid mistakes and reduce maintenance cost (we don't have to duplicate this visit function code, only one small part). But on the other hand it introduces a new kind of interface to the default visitor that is not as straightforward as the current one.

We could also change is to something like visit(std::vector<Param>) instead, that would be more "in line" with the rest of the visitor, but the "intermediate" aspect would be less obvious and that would prevent us from using this kind of intermediate visitor step for subsets of fields that are not in the same container (unless we use multiple arguments ?).

@powerboat9
Copy link
Collaborator

powerboat9 commented Apr 19, 2025 •
edited
Loading

@CohenArthur @philberty shall I drop e169a88 ? I'm unsure about it.

On one hand it allows us to avoid mistakes and reduce maintenance cost (we don't have to duplicate this visit function code, only one small part). But on the other hand it introduces a new kind of interface to the default visitor that is not as straightforward as the current one.

We could also change is to something like visit(std::vector<Param>) instead, that would be more "in line" with the rest of the visitor, but the "intermediate" aspect would be less obvious and that would prevent us from using this kind of intermediate visitor step for subsets of fields that are not in the same container (unless we use multiple arguments ?).

It's a bit odd, but e169a88 seems like the best solution for now

@P-E-P P-E-P requested a review from CohenArthur April 23, 2025 12:45
P-E-P added 6 commits April 24, 2025 15:04
@P-E-P
Add fn_once and Sized lang items to the test
4709749 
gcc/testsuite/ChangeLog:

	* rust/compile/multiple_bindings1.rs: Add missing lang items.

Signed-off-by: Pierre-Emmanuel Patry <pierre-emmanuel.patry@embecosm.com>
@P-E-P
Add equality operator for identifiers
974a706 
gcc/rust/ChangeLog:

	* ast/rust-ast.h: Add equality operator.

Signed-off-by: Pierre-Emmanuel Patry <pierre-emmanuel.patry@embecosm.com>
@P-E-P
Add hash function for Identifiers
228a971 
gcc/rust/ChangeLog:

	* ast/rust-ast.h: Add hash function.

Signed-off-by: Pierre-Emmanuel Patry <pierre-emmanuel.patry@embecosm.com>
@P-E-P
Add binding context class
dbc1ea1 
We need to differentiate bindings types, so the same binding cannot be
reused multiple time in a product binding.

gcc/rust/ChangeLog:

	* resolve/rust-name-resolution-context.h (struct Binding): Add Binding
	struct to differentiate Or and Product bindings in patterns.
	(enum class): Add Binding kind.
	(class BindingContext): Add binding context with Binding stack.

Signed-off-by: Pierre-Emmanuel Patry <pierre-emmanuel.patry@embecosm.com>
@P-E-P
Add pattern bindings
c2a0871 
gcc/rust/ChangeLog:

	* resolve/rust-late-name-resolver-2.0.cc (Late::visit): Add binding
	creation in visitor.
	* resolve/rust-late-name-resolver-2.0.h: Add function prototypes.
	* resolve/rust-name-resolution-context.h: Add binding context.

Signed-off-by: Pierre-Emmanuel Patry <pierre-emmanuel.patry@embecosm.com>
@P-E-P
Use stacked context for nested bindings.
4056b8c 
Binding context may be stacked when a new binding group is introduced
within a const expression.

gcc/rust/ChangeLog:

	* resolve/rust-name-resolution-context.h: Use BindingLayer instead.
	* resolve/rust-name-resolution-context.cc (BindingLayer::BindingLayer):
	Add new constructor for binding layer.
	(BindingLayer::bind_test): Add a function to test a binding constraint.
	(BindingLayer::push): Push a new binding group.
	(BindingLayer::and_binded): Add function to test and-binding
	constraint.
	(BindingLayer::or_binded): Add function to test or-binding constraints.
	(BindingLayer::insert_ident): Insert a new identifier in the current
	binding group.
	(BindingLayer::merge): Merge current binding group with it's parent.
	(BindingLayer::get_source): Get the source of the current binding
	group.
	* resolve/rust-late-name-resolver-2.0.cc: Use stacked context for
	binding group.
	* util/rust-stacked-contexts.h: Add mutable peek function.

Signed-off-by: Pierre-Emmanuel Patry <pierre-emmanuel.patry@embecosm.com>
P-E-P added 2 commits April 24, 2025 15:04
@P-E-P
Remove passing test from exclusion list
8ce5a69 
gcc/testsuite/ChangeLog:

	* rust/compile/nr2/exclude: Remove passing test from exclusion list.

Signed-off-by: Pierre-Emmanuel Patry <pierre-emmanuel.patry@embecosm.com>
@P-E-P
Use specialized param visit function for params
7134a11 
This commit introduce a new public function to visit function parameters
in the default visitor. It allows visitors derived from DefaultVisitor
to override only a small part of the default visitor.

gcc/rust/ChangeLog:

	* ast/rust-ast-visitor.cc (DefaultASTVisitor::visit_function_params):
	Add specialized function to visit function parameters.
	(DefaultASTVisitor::visit): Remove parameter visit and call specialized
	function instead.
	* ast/rust-ast-visitor.h: Add function prototye.
	* resolve/rust-late-name-resolver-2.0.cc (Late::visit): Remove
	function.
	(Late::visit_function_params): Override specialized visit function.
	* resolve/rust-late-name-resolver-2.0.h: Add overriden function
	prototype.

Signed-off-by: Pierre-Emmanuel Patry <pierre-emmanuel.patry@embecosm.com>
@P-E-P P-E-P force-pushed the nr2-fix-missing-lang-items branch from e169a88 to 7134a11 Compare April 24, 2025 13:04
@P-E-P P-E-P added this pull request to the merge queue Apr 24, 2025
Merged via the queue into Rust-GCC:master with commit ee61e89 Apr 24, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@powerboat9 powerboat9 powerboat9 left review comments

@CohenArthur CohenArthur Awaiting requested review from CohenArthur

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

NR2: Multiple definition
3 participants
@P-E-P @powerboat9 @CohenArthur