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.

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

Improve handling of implicit Self parameter in AST #3238

Merged
merged 1 commit into from
Nov 6, 2024

Conversation

powerboat9
Copy link
Contributor

This should partially address #2349, although I'm not sure why trait_import_*.rs tests are still failing

gcc/rust/ChangeLog:

	* ast/rust-item.h
	(Trait::self_param): Add.
	(Trait::Trait): Initialize self_param.
	(Trait::operator=): Copy self_param.
	(Trait::insert_implicit_self): Remove.
	(Trait::get_implicit_self): Add.
	* hir/rust-ast-lower-item.cc
	(ASTLoweringItem::visit): Make sure implicit self is still
	lowered to HIR.
	* resolve/rust-ast-resolve-item.cc
	(ResolveItem::visit): Adjust handling of implicit self.
	* resolve/rust-early-name-resolver.cc
	(EarlyNameResolver::visit): Add commit to Trait visitor
	mentioning that implicit self is not visited.
	* resolve/rust-toplevel-name-resolver-2.0.cc
	(TopLevel::visit): Remove call to Trait::insert_implicit_self.

gcc/testsuite/ChangeLog:

	* rust/compile/nr2/exclude: Remove entries.
	* rust/link/generic_function_0.rs: No longer expect failure.
	* rust/link/trait_import_0.rs: Likewise.
	* rust/link/trait_import_1.rs
	(trait Sized): Add.

Signed-off-by: Owen Avery <[email protected]>
@powerboat9
Copy link
Contributor Author

powerboat9 commented Nov 4, 2024

Fixed the trait_import_*.rs issue, should now fix #2349

@P-E-P P-E-P requested review from philberty and P-E-P November 5, 2024 10:11
Copy link
Member

@P-E-P P-E-P left a comment

Choose a reason for hiding this comment

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

This is how I would have done it, great changes. @philberty Do you mind taking a look ?

@philberty philberty self-assigned this Nov 5, 2024
@philberty philberty added the bug label Nov 5, 2024
Copy link
Member

@philberty philberty left a comment

Choose a reason for hiding this comment

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

LGTM its been a while since i looked at the import stuff. But yeah there is a section on the Rust documentation internals that Traits used the hack that Self is an implicit generic so to make it just reuse all the normal plumping it was simpler to just inject this during name resolution at the time.

@philberty philberty added this pull request to the merge queue Nov 6, 2024
Merged via the queue into Rust-GCC:master with commit 335f6c2 Nov 6, 2024
12 checks passed
@powerboat9 powerboat9 deleted the fix-self branch November 6, 2024 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants