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

Refactor lang item paths #3366

Merged
merged 11 commits into from
Jan 16, 2025

Conversation

CohenArthur
Copy link
Member

This PR contains the base for #3343 and refactors the way lang item paths are handled, either as path in expressions or as type paths. The changes are required for #3343 to work properly.

gcc/rust/ChangeLog:

	* ast/rust-ast.cc (BlockExpr::normalize_tail_expr): Remove overzealous
	std::move
gcc/rust/ChangeLog:

	* util/rust-attribute-values.h: Declare new attribute value.
	* util/rust-attributes.cc: Use it.
gcc/rust/ChangeLog:

	* ast/rust-collect-lang-items.cc (get_lang_item_attr): Show unknown attribute upon error.
@CohenArthur CohenArthur requested a review from P-E-P January 15, 2025 12:30
@CohenArthur CohenArthur force-pushed the refactor-lang-item-paths branch 2 times, most recently from 326f0c6 to 0b105fc Compare January 15, 2025 15:03
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.

Looks mostly great but I'd like some points to be resolved before merging.

gcc/rust/ast/rust-item.h Outdated Show resolved Hide resolved
gcc/rust/ast/rust-path.h 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

NodeId crate_scope_id = resolver->peek_crate_module_scope ();
if (segment->is_crate_path_seg ())
if (segment->is_lang_item ())
Copy link
Member

Choose a reason for hiding this comment

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

This part has a lot of indent levels, we should probably find a way to make it easier to read.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I agree, but also that part of the resolver is quite hard to understand and modify... I think I'd rather leave it like this and focus on nr2.0 which will eventually replace this anyway

gcc/rust/resolve/rust-ast-resolve-type.h Outdated Show resolved Hide resolved
gcc/rust/resolve/rust-late-name-resolver-2.0.cc Outdated Show resolved Hide resolved
@@ -118,6 +118,12 @@ LangItem::ToString (LangItem::Kind type)
return str.value ();
}

std::string
LangItem::PrettyString (LangItem::Kind type)
Copy link
Member

Choose a reason for hiding this comment

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

Weirdly (all) those functions in this file does not seem to follow our usual naming conventions or is it me ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I kept the existing convention going but they all are named a bit weird. In some cases I know @philberty has done that, where static class functions should be CamelCase to differentiate them. but it's not widespread through the codebase yet

gcc/rust/ast/rust-path.h Show resolved Hide resolved
gcc/rust/ast/rust-path.h Show resolved Hide resolved
gcc/rust/ast/rust-path.h Outdated Show resolved Hide resolved
gcc/rust/expand/rust-derive-copy.cc Show resolved Hide resolved
gcc/rust/hir/rust-ast-lower-type.cc Outdated Show resolved Hide resolved
gcc/rust/resolve/rust-ast-resolve-type.cc Outdated Show resolved Hide resolved
gcc/rust/resolve/rust-ast-resolve-type.h Outdated Show resolved Hide resolved
gcc/rust/resolve/rust-late-name-resolver-2.0.cc Outdated Show resolved Hide resolved
gcc/rust/util/rust-hir-map.cc Outdated Show resolved Hide resolved
gcc/rust/util/rust-hir-map.cc Outdated Show resolved Hide resolved
@CohenArthur CohenArthur force-pushed the refactor-lang-item-paths branch 2 times, most recently from 4bbfbea to ec2403c Compare January 16, 2025 13:32
Lang item typepaths were not handled properly, and required a complete overhaul.
All old classes that concerned lang item paths are now modified to use a simpler
version of `AST::LangItemPath`, which has been removed. TypePath segments can now
be lang items, as this is requied for having generic lang item paths such as
PhantomData<T>.

gcc/rust/ChangeLog:

	* ast/rust-path.h: Rework how lang item paths are represented.
	* ast/rust-path.cc: Likewise.
	* ast/rust-item.h: Likewise.
	* ast/rust-ast.cc: Likewise.
	* ast/rust-ast-collector.cc: Adapt to new lang item path system.
	* ast/rust-ast-collector.h: Likewise.
	* ast/rust-ast-visitor.cc (DefaultASTVisitor::visit): Likewise.
	* ast/rust-ast-visitor.h: Likewise.
	* expand/rust-derive-copy.cc: Likewise.
	* expand/rust-derive.h: Likewise.
	* hir/rust-ast-lower-base.cc (ASTLoweringBase::visit): Likewise.
	* hir/rust-ast-lower-base.h: Likewise.
	* hir/rust-ast-lower-type.cc (ASTLowerTypePath::translate): Likewise.
	(ASTLowerTypePath::visit): Likewise.
	* hir/rust-ast-lower-type.h: Likewise.
	* resolve/rust-ast-resolve-base.cc (ResolverBase::visit): Likewise.
	* resolve/rust-ast-resolve-base.h: Likewise.
	* resolve/rust-ast-resolve-item.cc (ResolveItem::visit): Likewise.
	* resolve/rust-ast-resolve-type.h: Likewise.
	* resolve/rust-ast-resolve-type.cc (ResolveRelativeTypePath::go): Likewise.
	* resolve/rust-late-name-resolver-2.0.cc (Late::visit): Likewise.
	* resolve/rust-late-name-resolver-2.0.h: Likewise.
	* hir/tree/rust-hir-path.cc (TypePathSegment::TypePathSegment): Likewise.
	(TypePathSegmentGeneric::TypePathSegmentGeneric): Likewise.
	* hir/tree/rust-hir-path.h: Likewise.
	* typecheck/rust-hir-type-check-type.cc (TypeCheckType::resolve_root_path): Likewise.
	* ast/rust-ast-builder.cc: Likewise.
	* ast/rust-ast-builder.h: Likewise.
gcc/rust/ChangeLog:

	* typecheck/rust-hir-type-check-type.cc (TypeCheckType::resolve_root_path): Adapt
	code to handle lang item type paths.
gcc/rust/ChangeLog:

	* hir/rust-ast-lower-type.cc (ASTLowerTypePath::visit): Adapt code to lang item
	type path segments.
gcc/rust/ChangeLog:

	* ast/rust-collect-lang-items.cc (CollectLangItems::visit): New.
	* ast/rust-collect-lang-items.h: New.
Which formats a lang item as it appears in source code.

gcc/rust/ChangeLog:

	* util/rust-lang-item.cc (LangItem::PrettyString): New.
	* util/rust-lang-item.h: New.
This method errors out if the lang item has not been declared yet.

gcc/rust/ChangeLog:

	* util/rust-hir-map.cc (Mappings::get_lang_item_node): New.
	* util/rust-hir-map.h: New function.
gcc/rust/ChangeLog:

	* ast/rust-ast-collector.cc (TokenCollector::visit): Fix collector to better
	handle lang item type path segments.
gcc/rust/ChangeLog:

	* ast/rust-ast-collector.cc (TokenCollector::visit): Visit tuple pattern items as
	separated by commas.
@CohenArthur CohenArthur enabled auto-merge January 16, 2025 13:35
@CohenArthur CohenArthur added this pull request to the merge queue Jan 16, 2025
Merged via the queue into Rust-GCC:master with commit aacecba Jan 16, 2025
12 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