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.

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

Fix nr2 unit struct #3299

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Fix nr2 unit struct #3299

wants to merge 10 commits into from

Conversation

P-E-P
Copy link
Member

@P-E-P P-E-P commented Dec 10, 2024

No description provided.

@P-E-P P-E-P force-pushed the fix-nr2-unit-struct branch 2 times, most recently from b490ffd to 9d29160 Compare December 11, 2024 12:45
@powerboat9
Copy link
Contributor

powerboat9 commented Dec 11, 2024 •
edited
Loading

Label scope is for ex: break 'a right?

Edit: ah, I see

@powerboat9
Copy link
Contributor

Most of the edits here look like they only apply to nr1

@P-E-P
Copy link
Member Author

P-E-P commented Dec 12, 2024

Most of the edits here look like they only apply to nr1

This PR isn't finished yet. Basically I stumbled on a problem for NR2, and discovered that this problem should apply on NR1. So I fixed NR1 then I changed the behavior for both NR.

@P-E-P P-E-P force-pushed the fix-nr2-unit-struct branch 2 times, most recently from eb1f1c7 to b2bf11e Compare January 6, 2025 17:22
@P-E-P P-E-P mentioned this pull request Jan 8, 2025
Merged
P-E-P added 8 commits January 11, 2025 18:28
@P-E-P
Labels shall be pushed within label namespace
c4b460c 
Labels were using the wrong namespace.

gcc/rust/ChangeLog:

	* resolve/rust-ast-resolve-expr.cc (ResolveExpr::visit): Change label
	push function from type rib to label rib.
	* resolve/rust-ast-resolve-item.cc (ResolveTraitItems::visit):
	Likewise.
	(ResolveItem::visit): Likewise.
	(ResolveExternItem::visit): Likewise.
	* resolve/rust-ast-resolve-stmt.h: Likewise.
	* resolve/rust-ast-resolve.cc (NameResolution::go): Likewise.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Add debug dump to old name resolver
d751b2e 
It might be necessary to compare both name resolution' internal states
during the transition. This new debug representation could help with
that.

gcc/rust/ChangeLog:

	* resolve/rust-name-resolver.h: Add new degug dump for old name
	resolver.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Add unit struct to name namespace in old resolver
17571f2 
We missed the name namespace for unit struct in the old resolver.

gcc/rust/ChangeLog:

	* resolve/rust-ast-resolve-toplevel.h: Add struct to name namespace.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Remove query mode on CompileItem
ac30ed1 
Query mode was a hack to catch up some compile errors early, it was
deemed to be removed at some time. Recent changes to NR1 highlighted
an incompatibility with it hence it's removal.

gcc/rust/ChangeLog:

	* backend/rust-compile-item.h: Remove query mode.
	* backend/rust-compile-resolve-path.cc (HIRCompileBase::query_compile):
	Likewise.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Remove some tests from nr2 exclusion file
f71402b 
Those test are now passing.

gcc/testsuite/ChangeLog:

	* rust/compile/nr2/exclude: Remove passing tests.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Clone expr instead of taking it
a40fcde 
We're reusing the value, it could therefore not be taken be should be
cloned.

gcc/rust/ChangeLog:

	* typecheck/rust-hir-type-check-enumitem.cc (TypeCheckEnumItem::visit):
	Clone expr instead of taking it.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Make foreverstack debug string const
864598d 
Those function should not change anything within the foreverstack, it
can therefore be made const.

gcc/rust/ChangeLog:

	* resolve/rust-forever-stack.h: Make debug functions const.
	* resolve/rust-forever-stack.hxx: Likewise.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Resolved item type shall be differentiated later
d9726c1 
We need to query all namespaces and error out at a later stage if the
retrieved item is wrong.

gcc/rust/ChangeLog:

	* typecheck/rust-hir-trait-resolve.cc (TraitResolver::resolve_path_to_trait):
	Query all namespaces.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P P-E-P force-pushed the fix-nr2-unit-struct branch from b2bf11e to bd1b05f Compare January 11, 2025 21:35
@powerboat9
Copy link
Contributor

A lot of these commits seem like they're ready to be merged, if put in separate pull request(s)

@P-E-P P-E-P marked this pull request as ready for review January 13, 2025 17:28
@P-E-P
Copy link
Member Author

P-E-P commented Jan 13, 2025 •
edited
Loading

A lot of these commits seem like they're ready to be merged, if put in separate pull request(s)

I think it is nearly ready to be merged anyway. I'll keep it like that for now.

@P-E-P
Copy link
Member Author

P-E-P commented Jan 13, 2025

@philberty could you please take a look at b646b25 ? There is a mention of ResolvePathType::Compile earlier in a comment but that type must have disappeared since I can't find it. Not 100% convinced this is the right way of fixing unit struct compile issue.

@P-E-P P-E-P requested a review from philberty January 13, 2025 17:30
@P-E-P
Copy link
Member Author

P-E-P commented Jan 14, 2025

If accepted as is I could technically remove ac30ed1.

P-E-P added 2 commits January 14, 2025 11:49
@P-E-P
Compile unit struct with constructor
a30a5c8 
gcc/rust/ChangeLog:

	* backend/rust-compile-resolve-path.cc (ResolvePathRef::resolve): Do
	not use query system for unit struct but compile it's constructor
	instead.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Remove some passing test from exclude file
93a788b 
Those tests are now passing.

gcc/testsuite/ChangeLog:

	* rust/compile/nr2/exclude: Remove some tests.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P P-E-P force-pushed the fix-nr2-unit-struct branch from b646b25 to 93a788b Compare January 14, 2025 11:04
@P-E-P P-E-P requested a review from CohenArthur January 16, 2025 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@philberty philberty Awaiting requested review from philberty

@CohenArthur CohenArthur Awaiting requested review from CohenArthur

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

Successfully merging this pull request may close these issues.
2 participants
@P-E-P @powerboat9