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

Name resolution globbing #2796

Merged
merged 44 commits into from
Mar 26, 2024
Merged

Name resolution globbing #2796

merged 44 commits into from
Mar 26, 2024

Conversation

P-E-P
Copy link
Member

@P-E-P P-E-P commented Jan 15, 2024

Late name resolution was not used and did not handle globbing.

@P-E-P P-E-P added this to the GCC 14.1 release milestone Jan 15, 2024
@P-E-P P-E-P self-assigned this Jan 15, 2024
@powerboat9
Copy link
Contributor

@CohenArthur how much of this can be merged as-is?

@P-E-P P-E-P mentioned this pull request Feb 28, 2024
Draft
@P-E-P P-E-P force-pushed the name-res-globbing-from-existing-base branch 4 times, most recently from b5c44d1 to 18fc6f5 Compare March 13, 2024 16:38
@P-E-P P-E-P force-pushed the name-res-globbing-from-existing-base branch 6 times, most recently from 29b4a0c to 546a8da Compare March 22, 2024 14:09
@CohenArthur CohenArthur force-pushed the name-res-globbing-from-existing-base branch from 546a8da to 176b059 Compare March 22, 2024 16:22
@CohenArthur
nr2.0: Add new ImmutableNameResolutionCtx class.
8af64d2 
gcc/rust/ChangeLog:

	* Make-lang.in: Compile it.
	* resolve/rust-immutable-name-resolution-context.cc: New file.
	* resolve/rust-immutable-name-resolution-context.h: New file.
@CohenArthur
sesh: Add late name resolution 2.0
0d5c3f1 
gcc/rust/ChangeLog:

	* rust-session-manager.cc (Session::compile_crate): Create name resolution
	context for Session::expansion and subsequent name resolution passes.
	(Session::expansion): Take name resolution context as a parameter
	instead.
	* rust-session-manager.h (Session::expansion): Fix declaration.
@P-E-P P-E-P marked this pull request as ready for review March 22, 2024 18:05
@P-E-P P-E-P requested a review from CohenArthur March 22, 2024 18:06
@P-E-P
Copy link
Member Author

P-E-P commented Mar 22, 2024

I still need to reorder some commits and fix the changelog but this is nearly done 🎉

@P-E-P P-E-P requested a review from philberty March 22, 2024 18:07
@P-E-P P-E-P force-pushed the name-res-globbing-from-existing-base branch 8 times, most recently from 209d7be to aaead7e Compare March 24, 2024 22:21
P-E-P and others added 22 commits March 26, 2024 10:37
@P-E-P
Change error message to match test
c484363 
Error message did not match the test from the previous name resolver when
a given path cannot be resolved.

gcc/rust/ChangeLog:

	* typecheck/rust-hir-type-check-path.cc (TypeCheckExpr::resolve_root_path):
	Change error message to match old resolver and test case.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Visit function return type in default resolver
c3b5f23 
Function return type was not properly visited in the default resolver
visitor pattern.

gcc/rust/ChangeLog:

	* resolve/rust-default-resolver.cc (DefaultResolver::visit): Visit
	function return type.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Visit constant item type in default resolver
b080ecb 
The type of constant item expression was not properly visited in the
default resolver.

gcc/rust/ChangeLog:

	* resolve/rust-default-resolver.cc (DefaultResolver::visit): Visit
	constant item's types.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Raw pointer type visitor didn't require overload
c10d15d 
This overload did not dispatch the visitor to sub members of a raw
pointer like the default one. It is therefore useless as pointed type
shall be visited to be resolved correctly.

gcc/rust/ChangeLog:

	* resolve/rust-default-resolver.cc (DefaultResolver::visit): Remove
	function implementation.
	* resolve/rust-default-resolver.h: Remove function prototype.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Values shall be inserted in the value namespace
4caad73 
Values were inserted in the label namespace instead of the value
namespace this lead to several bugs.

gcc/rust/ChangeLog:

	* resolve/rust-late-name-resolver-2.0.cc (Late::visit): Change the
	namespace for values from "label" to "values".

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Unit struct constructor shall be resolved
1bb562e 
Unit struct have a special constructor that should be added to the struct
namespace in order to be resolved later when called. As it is a function
it should be added in the value namespace.

gcc/rust/ChangeLog:

	* resolve/rust-toplevel-name-resolver-2.0.cc (GlobbingVisitor::visit):
	Add the struct constructor when the struct is a unit.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Add tuple struct to the type namespace
95a0547 
Only tuple struct constructor was added to the resolver.

gcc/rust/ChangeLog:

	* resolve/rust-toplevel-name-resolver-2.0.cc (GlobbingVisitor::visit):
	Add tuple struct type to the resolver's context.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Change enum namespace from value to type
74d6449 
The enum type shall be in type namespace, not value namespace.

gcc/rust/ChangeLog:

	* resolve/rust-toplevel-name-resolver-2.0.cc (GlobbingVisitor::visit):
	Change enum type namespace.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Struct are types, not values
2a84cf0 
We shall search in the right namespace. The correct namespace for struct
is type namespace.

gcc/rust/ChangeLog:

	* typecheck/rust-hir-type-check-item.cc (TypeCheckItem::visit): Change
	search location for struct types.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Add constant identifiers to the value namespace
74d47bc 
Constants could not be resolved without their identifier in the right
scope.

gcc/rust/ChangeLog:

	* resolve/rust-toplevel-name-resolver-2.0.cc (TopLevel::visit): Add
	constant identifiers to the resolver.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Remove extern block scoping
86b75b2 
Remove extern block scoping visit function, use the default visitor visit
function instead. We do not need scoping for extern block as their
element shall be visible from the extern block scope.

gcc/rust/ChangeLog:

	* resolve/rust-default-resolver.cc (DefaultResolver::visit): Remove
	visitor implementation and scoping.
	* resolve/rust-default-resolver.h: Remove function prototype.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Remove unsafe block empty visit function
b6646bc 
We need to visit subcomponents in unsafe elements, this means we can
leverage the default ast visitor's code instead of duplicating it.

gcc/rust/ChangeLog:

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

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Use new name resolver to compile constant items
cfdbadc 
Constant items were handled only by the old resolver, this lead to an
ICE when using the new resolver on some rust code containing a constant
item as the new and the old resolver cannot be used at the same time.

gcc/rust/ChangeLog:

	* backend/rust-compile-item.cc (CompileItem::visit): Check the resolver
	flag and use the new one when required.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Reinject Self parameter in new resolver
58655fb 
The old resolver injected a Self generic parameter in order to help the
trait solver. This is clearly sketchy at best and should be fixed in
the future.

gcc/rust/ChangeLog:

	* resolve/rust-toplevel-name-resolver-2.0.cc (TopLevel::visit): Add
	Self generic parameter injection and a warning.
	* resolve/rust-toplevel-name-resolver-2.0.h: Add function prototype.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Update assignment operator with cratenum
d6f2c80 
Crate number was not assigned with the other fields in the assignment
operator overload of a CannonicalPath.

gcc/rust/ChangeLog:

	* util/rust-canonical-path.h: Also assign crate number.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Prevent getting immutable context with classic nr
180be5b 
Immutable name resolution context is not initialized when the classic
name resolution is in use. It can therefore not be used, the getter would
error out.

gcc/rust/ChangeLog:

	* typecheck/rust-hir-type-check-path.cc (TypeCheckExpr::resolve_root_path):
	Only get immutable name resolution context when name resolution 2.0 is
	used.
	* typecheck/rust-hir-type-check-type.cc (TypeCheckType::resolve_root_path):
	Likewise.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Fix quoted string format
2bdd499 
This format dialog triggered a warning.

gcc/rust/ChangeLog:

	* resolve/rust-toplevel-name-resolver-2.0.cc (TopLevel::handle_use_dec):
	Replace the string format %<%s%> with the proper %qs format.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Add mappings for struct base and struct fields
9f03038 
Definition/usage mapping during name resolution was missing.

gcc/rust/ChangeLog:

	* resolve/rust-late-name-resolver-2.0.cc (Late::visit): Add mapping
	implementation.
	* resolve/rust-late-name-resolver-2.0.h: Add function visitor prototype
	override.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P
Fix use rebind name resolution.
7c18255 
Name resolution for rebind were missing.

gcc/rust/ChangeLog:

	* resolve/rust-toplevel-name-resolver-2.0.cc (TopLevel::handle_use_glob):
	Change function prototype to use a reference instead.
	(TopLevel::handle_use_dec): Likewise.
	(TopLevel::handle_rebind): Add name resolution on rebind use
	declarations.
	(flatten_rebind): Change prototype to accept a pair of path/alias.
	(flatten_list): Adapt call to flatten_rebind.
	(flatten): Adapt call to flatten_rebind.
	(flatten_glob): Remove unused part.
	(TopLevel::visit): Add rebind resolution.
	* resolve/rust-toplevel-name-resolver-2.0.h: Adapt function prototypes.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@CohenArthur @P-E-P
compile: resolve-path-ref: properly resolve nodeId with nr2.0
da5f5bf 
gcc/rust/ChangeLog:

	* backend/rust-compile-resolve-path.cc: Attempt to resolve names
	also using new name resolution context.
	* backend/rust-compile-resolve-path.h: Add new declaration.
@CohenArthur @P-E-P
nr2.0: Add new test cases.
f5dcf49 
gcc/testsuite/ChangeLog:

	* rust/compile/name_resolution13.rs: Add new module and remove compile
	step.
	* rust/compile/name_resolution14.rs: New test.
	* rust/compile/name_resolution15.rs: New test.
	* rust/compile/name_resolution16.rs: New test.
	* rust/compile/name_resolution17.rs: New test.
	* rust/compile/name_resolution18.rs: New test.
	* rust/compile/name_resolution19.rs: New test.
	* rust/compile/name_resolution20.rs: New test.
	* rust/compile/name_resolution21.rs: New test.
@P-E-P
Add globbing name resolution 2.0 test
33bed05 
Add a few test for globbing to highlight function call ambiguities.

gcc/testsuite/ChangeLog:

	* rust/compile/name_resolution23.rs: New test.
	* rust/compile/name_resolution24.rs: New test.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P P-E-P force-pushed the name-res-globbing-from-existing-base branch from 4b768eb to 01d83db Compare March 26, 2024 10:00
@P-E-P
Copy link
Member Author

P-E-P commented Mar 26, 2024

I don't think there are any test cases for glob imports, could we add some?

I've added two tests, name_resolution23.rs and name_resolution24.rs. The first one checks the globbing works and ambiguity errors are not triggered when the ambiguous symbol is not used. The second one ensure an error is emitted when an ambiguous symbol is called.

@P-E-P P-E-P force-pushed the name-res-globbing-from-existing-base branch from 01d83db to 33bed05 Compare March 26, 2024 15:14
@P-E-P
Change dfs function return type to support gcc 4.8
721c6fa 
GCC 4.8 does not handle pair with references correctly. We need to use a
properly typed struct instead.

gcc/rust/ChangeLog:

	* resolve/rust-forever-stack.h: Change dfs function prototype and
	declare dfs return type structure.
	* resolve/rust-forever-stack.hxx: Adapt dfs function to the new return
	type.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
@P-E-P P-E-P added this pull request to the merge queue Mar 26, 2024
Merged via the queue into Rust-GCC:master with commit 65f283d Mar 26, 2024
9 checks passed
@CohenArthur
Copy link
Member

amazing work on this @P-E-P, well done :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CohenArthur CohenArthur CohenArthur approved these changes

@philberty philberty Awaiting requested review from philberty

Assignees

@P-E-P P-E-P

Labels
enhancement
Projects
name resolution 2.0
Status: Done
Milestone
GCC 14.1 release
Development

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