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

Use lang items as paths #3290

Merged
merged 7 commits into from
Dec 9, 2024
Merged

Conversation

CohenArthur
Copy link
Member

@CohenArthur CohenArthur commented Dec 4, 2024

This PR makes it possible to use lang items as paths, for example when generating code for derives or for-loops. Instead of generating a complex path to something like core::marker::Copy, we can just use a LangItemPath to LangItem::COPY. This way, the code generation does not need to care about name resolution, about std's module hierarchy changing, or about duplicate definitions with the same module structure (e.g. if a user defines a copy trait in their own source file with their own core and marker module).

This is not going to build until #3289 is merged as it depends on those mappings - but it should still be reviewable

@CohenArthur CohenArthur added this to the Lang items path fixes milestone Dec 4, 2024
gcc/rust/ast/rust-path.cc Outdated Show resolved Hide resolved
gcc/rust/ast/rust-path.h Outdated Show resolved Hide resolved
gcc/rust/ast/rust-path.h Outdated Show resolved Hide resolved
gcc/rust/expand/rust-derive-copy.cc Outdated Show resolved Hide resolved
gcc/rust/hir/rust-ast-lower-type.cc Outdated Show resolved Hide resolved
gcc/rust/hir/rust-ast-lower-type.cc Outdated Show resolved Hide resolved
gcc/rust/resolve/rust-ast-resolve-item.cc Outdated Show resolved Hide resolved
gcc/rust/resolve/rust-ast-resolve-item.cc Show resolved Hide resolved
gcc/rust/resolve/rust-ast-resolve-type.h Outdated Show resolved Hide resolved
gcc/rust/resolve/rust-ast-resolve-type.h Show resolved Hide resolved
trait_path (std::unique_ptr<TypePath> (new TypePath (trait_path))),
impl_items (std::move (impl_items))
{}

Copy link
Contributor

Choose a reason for hiding this comment

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

This could probably use a delegating constructor

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not against that, I think that's a good idea. What did you have in mind exactly?

Copy link
Contributor

Choose a reason for hiding this comment

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

This constructor could become a delegating constructor, with this constructor as the target constructor.

https://en.cppreference.com/w/cpp/language/constructor#Delegating_constructor

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we can probably turn this into a good first PR as it's a nice cleanup but not functionally any different

This commit adds a new kind of Path, changes the base Path class and turns TypePath
into a child of the base Path class.

gcc/rust/ChangeLog:

	* ast/rust-path.h (class LangItemPath): New.
	(class TypePath): Adapt to accomodate LangItemPath.
	* ast/rust-ast.cc (TraitImpl::as_string): Use new checks for lang items.
	(QualifiedPathType::as_string): Likewise.
	(FormatArgs::set_outer_attrs): Likewise.
	* ast/rust-item.h (class TraitImpl): Likewise.
gcc/rust/ChangeLog:

	* expand/rust-derive-copy.cc: Use new LangItemPath for derive(Copy).
gcc/rust/ChangeLog:

	* ast/rust-item.h: Add new method to specifically get a type-path.
	* ast/rust-path.cc (LangItemPath::as_string): Implement properly.
	* hir/rust-ast-lower-type.cc (ASTLowerTypePath::translate): Adapt
	visitor to use the new LangItemPath.
	* hir/rust-ast-lower-type.h: Likewise.
	* resolve/rust-ast-resolve-item.cc (ResolveItem::visit): Likewise.
	* resolve/rust-ast-resolve-type.h: Likewise.
gcc/rust/ChangeLog:

	* resolve/rust-ast-resolve-item.cc (ResolveItem::visit): Adapt resolver
	to lang item paths.
	* resolve/rust-ast-resolve-type.h: Likewise.
gcc/rust/ChangeLog:

	* hir/rust-ast-lower-type.cc (ASTLowerTypePath::translate): Adapt to
	handle lang item paths.
	(ASTLowerTypePath::visit): Likewise.
	(ASTLowerTypePath::translate_type_path): New.
	(ASTLowerTypePath::translate_lang_item_type_path): New.
	* hir/rust-ast-lower-type.h: Adapt to handle lang item paths.
	* resolve/rust-ast-resolve-type.h: Likewise.
gcc/rust/ChangeLog:

	* resolve/rust-late-name-resolver-2.0.cc (Late::visit): New.
	* resolve/rust-late-name-resolver-2.0.h: New.
gcc/rust/ChangeLog:

	* ast/rust-path.h: Adapt children of Path to fix some NodeId issues.
@CohenArthur CohenArthur enabled auto-merge December 9, 2024 14:22
@CohenArthur CohenArthur added this pull request to the merge queue Dec 9, 2024
Merged via the queue into Rust-GCC:master with commit ed1b4d7 Dec 9, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants