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

Allow use before items being used continued #3150

Merged
merged 30 commits into from
Sep 26, 2024
Merged

Allow use before items being used continued #3150

merged 30 commits into from
Sep 26, 2024

Conversation

P-E-P
Copy link
Member

@P-E-P P-E-P commented Sep 4, 2024

Name resolution 2.0 continued.

This PR add fixes and add some work on top of #2940

@P-E-P P-E-P requested a review from CohenArthur September 4, 2024 11:56
@P-E-P P-E-P self-assigned this Sep 4, 2024
@P-E-P P-E-P added this to the Name Resolution 2.0 rework milestone Sep 4, 2024
@P-E-P P-E-P force-pushed the allow-use-before-items-being-used-continued branch 4 times, most recently from 3e15f81 to dd3b3d1 Compare September 5, 2024 15:59
@CohenArthur CohenArthur force-pushed the allow-use-before-items-being-used-continued branch from 04301d7 to fac42c6 Compare September 16, 2024 14:59
@P-E-P P-E-P marked this pull request as ready for review September 17, 2024 12:03
@P-E-P P-E-P force-pushed the allow-use-before-items-being-used-continued branch from fac42c6 to 1f67f70 Compare September 17, 2024 12:06
@P-E-P P-E-P enabled auto-merge September 17, 2024 12:21
@P-E-P P-E-P disabled auto-merge September 17, 2024 12:45
@P-E-P P-E-P force-pushed the allow-use-before-items-being-used-continued branch 2 times, most recently from b1928ac to 28fd0a5 Compare September 17, 2024 14:57
@powerboat9 powerboat9 mentioned this pull request Sep 18, 2024
Merged
@P-E-P P-E-P force-pushed the allow-use-before-items-being-used-continued branch from 28fd0a5 to 02f3600 Compare September 19, 2024 14:19
CohenArthur and others added 13 commits September 26, 2024 23:44
@CohenArthur @P-E-P
nr2.0: default-visitor: Conditionally visit type in self parameters.
7347cb0 
This could trigger an assertions as `get_type` on `SelfParam` asserts that
the self param does have a given type, which is not always the case.

gcc/rust/ChangeLog:

	* resolve/rust-default-resolver.cc (DefaultResolver::visit): Do not
	visit self's type if it does not have one.
@CohenArthur @P-E-P
toplevel: Add note for resolving use declarations
fbe034d 
gcc/rust/ChangeLog:

	* resolve/rust-toplevel-name-resolver-2.0.cc (TopLevel::handle_use_dec):
	Add notes on the problem.
	* resolve/rust-toplevel-name-resolver-2.0.h: Likewise.
@CohenArthur @P-E-P
toplevel: Build list of imports for Early to resolve
d226286 
gcc/rust/ChangeLog:

	* resolve/rust-toplevel-name-resolver-2.0.cc: Comment out handle_use
	call and error emission.
	* resolve/rust-toplevel-name-resolver-2.0.h: Create ImportKind class.
@CohenArthur @P-E-P
early: Resolve imports and create import mappings
b655a51 
gcc/rust/ChangeLog:

	* resolve/rust-early-name-resolver-2.0.cc (Early::resolve_glob_import): New function.
	(Early::resolve_simple_import): Likewise.
	(Early::resolve_rebind_import): Likewise.
	(Early::build_import_mapping): Likewise.
	* resolve/rust-early-name-resolver-2.0.h: Add declarations and list of imports to
	resolve.
	* resolve/rust-toplevel-name-resolver-2.0.cc (TopLevel::handle_use_glob): Remove function,
	which is now being handled by the Early name resolver.
	(TopLevel::handle_use_dec): Likewise.
	(TopLevel::handle_rebind): Likewise.
	* resolve/rust-toplevel-name-resolver-2.0.h: Likewise, and add functions for creating
	import list and fetching it.
@CohenArthur @P-E-P
imports: Add FinalizeImports class
7abd0c4 
gcc/rust/ChangeLog:

	* Make-lang.in: Add new object file.
	* ast/rust-item.h: Constify method.
	* resolve/rust-early-name-resolver-2.0.cc (Early::go): Call into
	the imports finalizer.
	(Early::resolve_glob_import): Remove old resolution.
	(Early::resolve_rebind_import): Likewise.
	* resolve/rust-toplevel-name-resolver-2.0.cc (GlobbingVisitor::go):
	New function.
	(GlobbingVisitor::visit): Likewise.
	(TopLevel::visit): Do not call into handle_use_* functions anymore.
	* resolve/rust-toplevel-name-resolver-2.0.h (class GlobbingVisitor):
	New.
	* resolve/rust-finalize-imports-2.0.cc: New file.
	* resolve/rust-finalize-imports-2.0.h: New file.
@CohenArthur @P-E-P
imports: Create ImportData class and use it in import_mappings
a5a4a1d 
gcc/rust/ChangeLog:

	* resolve/rust-early-name-resolver-2.0.cc (Early::resolve_glob_import):
	Use ImportData class.
	(Early::resolve_simple_import): Likewise.
	(Early::resolve_rebind_import): Likewise.
	(Early::build_import_mapping): Likewise.
	* resolve/rust-early-name-resolver-2.0.h: Likewise.
	* resolve/rust-finalize-imports-2.0.cc (finalize_simple_import): Likewise.
	(finalize_glob_import): Likewise.
	(finalize_rebind_import): Likewise.
	(FinalizeImports::go): Likewise.
	* resolve/rust-finalize-imports-2.0.h: Likewise.
	* resolve/rust-name-resolution-context.h: Likewise.
	* resolve/rust-rib.h: Define ImportData class.
@CohenArthur @P-E-P
imports: Start storing Ribs in ImportKind
51e89e5 
gcc/rust/ChangeLog:

	* resolve/rust-late-name-resolver-2.0.cc (Late::visit): Add debug call.
	* resolve/rust-toplevel-name-resolver-2.0.cc (TopLevel::visit): Store
	imports using the new classes.
	* resolve/rust-toplevel-name-resolver-2.0.h: Use new classes.
@CohenArthur @P-E-P
imports: Make FinalizeImports a resolver visitor as well
3ee7238 
gcc/rust/ChangeLog:

	* resolve/rust-finalize-imports-2.0.cc (FinalizeImports::go): Turn
	static method into method.
	(FinalizeImports::visit): New.
	* resolve/rust-finalize-imports-2.0.h (class FinalizeImports): Make
	FinalizeImports a visitor.
	* resolve/rust-early-name-resolver-2.0.cc (Early::go): Use new FinalizeImports API.
	(Early::resolve_glob_import): Use new API.
	(Early::resolve_simple_import): Likewise.
	(Early::resolve_rebind_import): Likewise.
	(Early::build_import_mapping): Likewise.
	* resolve/rust-early-name-resolver-2.0.h: Likewise.
	* resolve/rust-toplevel-name-resolver-2.0.cc (TopLevel::visit): Likewise.
	* resolve/rust-toplevel-name-resolver-2.0.h: Likewise.
@CohenArthur @P-E-P
early: Do not emit errors for unresolved imports, store them instead
d08788f 
gcc/rust/ChangeLog:

	* resolve/rust-early-name-resolver-2.0.cc (Early::visit_attributes):
	Store errors for later.
@CohenArthur @P-E-P
Insert imports in all namespaces they were resolved in
4d8fda3 
gcc/rust/ChangeLog:

	* resolve/rust-early-name-resolver-2.0.cc (Early::resolve_simple_import):
	Insert import in all namespaces where they were resolved.
	(Early::resolve_rebind_import): Likewise.
	* resolve/rust-early-name-resolver-2.0.h: Improve APIs, make them
	accept multiple resolutions.
	* resolve/rust-finalize-imports-2.0.cc: Handle multiple resolutions.
	* resolve/rust-name-resolution-context.h (resolve_path): Remove function.
@CohenArthur @P-E-P
nr2.0: Cleanup import mappings and factor into a class.
c636cdf 
gcc/rust/ChangeLog:

	* resolve/rust-early-name-resolver-2.0.h: New class for imports.
	* resolve/rust-finalize-imports-2.0.cc (finalize_simple_import): Use
	the new API.
	(finalize_glob_import): Likewise.
	(finalize_rebind_import): Likewise.
	(FinalizeImports::FinalizeImports): Likewise.
	(FinalizeImports::visit): Likewise.
	* resolve/rust-finalize-imports-2.0.h: Likewise.
	* resolve/rust-early-name-resolver-2.0.cc (Early::resolve_glob_import): Likewise.
	(Early::resolve_simple_import): Likewise.
	(Early::resolve_rebind_import): Likewise.
@P-E-P
Fix missing error on duplicated nodes
26df0d8 
When we tried to insert a shadowable node and another shadowable node has
been inserted before, we didn't emit any error if the node has already
been inserted previously and failed silently.

gcc/rust/ChangeLog:

	* resolve/rust-rib.cc (Rib::insert): Emit an error when trying to
	insert an already inserted node.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Loop on expansion if a new export has been defined
05e097d 
When a use statement requires a reexported item it cannot find it in
the same pass, an additional pass shall be performed. This means we need
to detect whether a new item has been reexported and resolve until the
end.

gcc/rust/ChangeLog:

	* resolve/rust-early-name-resolver-2.0.cc (Early::Early): Add dirty
	flag initialization.
	(Early::go): Set dirty flag using top level resolver.
	* resolve/rust-early-name-resolver-2.0.h: Add dirty flag.
	* resolve/rust-toplevel-name-resolver-2.0.cc (TopLevel::TopLevel):
	Initialize dirty flag.
	(TopLevel::insert_or_error_out): Set dirty flag on successful
	namespace modification.
	* resolve/rust-toplevel-name-resolver-2.0.h: Add dirty flag.
	* rust-session-manager.cc (Session::expansion): Modify fixed point
	condition to include name resolution modifications.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
P-E-P added 10 commits September 26, 2024 23:57
@P-E-P
Mark virtual function override in default resolver
3e1992c 
Those function prototype were not marked as override and throwing
warning.

gcc/rust/ChangeLog:

	* resolve/rust-default-resolver.h: Make most visit function override.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Remove empty visit functions
cc4ba51 
We can let the default visitor visit those nodes anyway so we're sure
all nodes can be reached.

gcc/rust/ChangeLog:

	* resolve/rust-default-resolver.cc (DefaultResolver::visit): Remove
	empty visit function implementations.
	* resolve/rust-default-resolver.h: Remove corresponding prototypes.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Remove regular visit code
91ea827 
Regular visit code can be replaced with default visit functions.

gcc/rust/ChangeLog:

	* resolve/rust-default-resolver.cc (DefaultResolver::visit): Remove
	default visit code and replace it with call to default visitor.
	* resolve/rust-default-resolver.h: Remove removed function's
	prototypes.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Change lambda content with default visitor call
25f219b 
We can reduce code duplication by using the default visitor functions
from within the scoped lambda function.

gcc/rust/ChangeLog:

	* resolve/rust-default-resolver.cc (DefaultResolver::visit): Use
	default visitor instead.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Add default resolver parent functions by default
03a9a24 
gcc/rust/ChangeLog:

	* resolve/rust-finalize-imports-2.0.h: Add parent member functions
	from default resolver.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Make AST default visitor visit functions public
858e308 
Make those functions public so they can be used within a lambda on GCC
4.8.

gcc/rust/ChangeLog:

	* ast/rust-ast-visitor.h: Make visit functions public.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Move failing test to xfail
9ee6537 
We want to begin experimenting with this new name resolution 2.0
algorithm as soon as possible. This test highlight a problem where the
compiler should emit an error and should be fixed soon.

gcc/testsuite/ChangeLog:

	* rust/compile/name_resolution21.rs: Move to...
	* rust/compile/xfail/name_resolution21.rs: ...here.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Move bir builder function implementation
1c3cf3f 
Move function implementation to their own file.

gcc/rust/ChangeLog:

	* Make-lang.in: Add new rust-bir-builder-pattern file.
	* checks/errors/borrowck/rust-bir-builder-pattern.h: Remove
	implementation.
	* checks/errors/borrowck/rust-bir-builder-pattern.cc: New file.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Do not assert insertion result
d357b2a 
We might have some duplicated name in some pattern and we should
therefore not assert the non duplication of identifiers.

gcc/rust/ChangeLog:

	* resolve/rust-late-name-resolver-2.0.cc (Late::visit): Remove
	assertion and explicitely tells why we ignore the insertion result.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Update exclude list with working tests
838e2c8 
gcc/testsuite/ChangeLog:

	* rust/compile/nr2/exclude: Remove working tests from nr2 exclude list.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P P-E-P force-pushed the allow-use-before-items-being-used-continued branch from 48d4cc2 to 8a2f112 Compare September 26, 2024 22:02
@P-E-P
Change resolved type segment
a795621 
gcc/rust/ChangeLog:

	* resolve/rust-late-name-resolver-2.0.h: Add visit function prototype.
	* resolve/rust-late-name-resolver-2.0.cc (Late::visit): Change resolved
	type segment.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Make node id getter const.
df3b02a 
gcc/rust/ChangeLog:

	* ast/rust-ast.h: Node id getter could be const.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
This test requires the standard library
fab6c61 
It requires the standard library and Copy to work correctly which we
cannot provide. Stopping the compiler before the name resolution allow us
to prevent an error whilst resolving Copy and keep the test's goal.

gcc/testsuite/ChangeLog:

	* rust/compile/functions_without_body.rs: Add compile step argument.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Add box definition to avoid error
f1518be 
Box definition is part of the standard library and cannot be found during
name resolution. This simple definition prevent any error from being
emitted.

gcc/testsuite/ChangeLog:

	* rust/compile/box_syntax_feature_gate.rs: Add box land item
	definition.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Resolve TypeParam with name resolution 2.0
25d4ed7 
Resolve TypeParam unless it is Self.

gcc/rust/ChangeLog:

	* resolve/rust-toplevel-name-resolver-2.0.cc (TopLevel::visit): Resolve
	TypeParam.
	* resolve/rust-toplevel-name-resolver-2.0.h: Add visit function
	prototype.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Postpone break on error after name resolution
4c3f7d5 
We need the top level to run at least once before breaking because it
will be required by the other name resolution steps.

gcc/rust/ChangeLog:

	* rust-session-manager.cc (Session::expansion): Break on error after
	top level name resolution.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Remove some passing test from nr2 passing list
2843ceb 
gcc/testsuite/ChangeLog:

	* rust/compile/nr2/exclude: Remove cast_generics.rs, issue-1131.rs,
	issue-1383.rs and unsafe10.rs

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P P-E-P force-pushed the allow-use-before-items-being-used-continued branch from 8a2f112 to 2843ceb Compare September 26, 2024 22:13
@P-E-P P-E-P added this pull request to the merge queue Sep 26, 2024
Merged via the queue into Rust-GCC:master with commit 423122c Sep 26, 2024
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CohenArthur CohenArthur Awaiting requested review from CohenArthur

Assignees

@P-E-P P-E-P

Labels
name resolution
Projects
name resolution 2.0
Status: Done
Milestone
Name Resolution 2.0 rework
Development

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