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

Make TyTy::TupleType::get_unit_type cache its return value #3232

Merged
merged 1 commit into from
Nov 1, 2024

Conversation

powerboat9
Copy link
Contributor

This removes a call to Resolver::get_unit_type_node_id, which relies on the old resolver calling Resolver::set_unit_type_node_id

This removes a usage of Resolver::get_unit_type_node_id in
rust-hir-type-check-expr.cc (the HIR::TupleExpr overload of
TypeCheckExpr::visit).

gcc/rust/ChangeLog:

	* typecheck/rust-tyty.cc
	(TupleType::get_unit_type): Remove parameter, cache return
	value.
	* typecheck/rust-tyty.h
	(TupleType::get_unit_type): Remove parameter.
	* resolve/rust-late-name-resolver-2.0.cc
	(Late::setup_builtin_types): Adjust calls to get_unit_type.
	* resolve/rust-name-resolver.cc
	(Resolver::generate_builtins): Likewise.
	* typecheck/rust-hir-type-check-expr.cc
	(TypeCheckExpr::visit): Likewise.
	* typecheck/rust-hir-type-check-implitem.cc
	(TypeCheckTopLevelExternItem::visit): Likewise.
	(TypeCheckImplItem::visit): Likewise.
	* typecheck/rust-hir-type-check-item.cc
	(TypeCheckItem::visit): Likewise.
	* typecheck/rust-hir-type-check-stmt.cc
	(TypeCheckStmt::visit): Likewise.
	* typecheck/rust-hir-type-check-type.cc
	(TypeCheckType::visit): Likewise.
	* typecheck/rust-hir-type-check.cc
	(TraitItemReference::get_type_from_fn): Likewise.

Signed-off-by: Owen Avery <[email protected]>
@P-E-P
Copy link
Member

P-E-P commented Oct 29, 2024

So here we've got the same HIR id for all unit types. It seems fine but I do not know all implications so I'll let @philberty review

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 yes this is actually the right thing to do.

Historically i wasnt sure how the type system was going to work at all from reading lots of rustc. But this now makes unit-type work like the other builtins like i32.

The idea of the reference is that we have an hir node that reflects this type at this node-id. But when it comes to referencing it we update the reference but the ty_ref references the original declaration. We use this to manage generic parameter types because we can then simply replace the ty-ref on the type to point to what the new parameter points to which is still super scary to me and still suprises me that it seems to be working at all but i do have sleepless nights over it...

@philberty philberty added this pull request to the merge queue Nov 1, 2024
Merged via the queue into Rust-GCC:master with commit 94957c0 Nov 1, 2024
12 checks passed
@powerboat9 powerboat9 deleted the fix-unit branch November 1, 2024 16:29
Copy link
Member

@CohenArthur CohenArthur left a comment

Choose a reason for hiding this comment

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

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants