From 6e1003927758d977121d12d166b91a194f473ae5 Mon Sep 17 00:00:00 2001 From: Philip Herron Date: Mon, 26 May 2025 19:30:45 +0100 Subject: [PATCH 1/2] gccrs: Fix non canonical type bug with tuples When working on rpit we needed to change to use a monomorphized clone of the result of function calls. This ended up causing a verify gimple issue with tuples because: fn test(a: A, b: B) -> (A, B) When passing for example: let a = test:: (123, 456) -> (A=i32, B=i32) The resulting gimple types became: const struct (A=i32, B=i32) vs struct (i32, i32) We removed the VIEW_CONVERT_EXPR support to auto fix this stuff a good while ago because it hides these kinds of issues because the type hasher included the A=i32, B=i32 vs the i32, i32 name so updating this to use get_name instead keeps the naming the same as well as the fields meaning these types are 100% equivilant and therefore no conversion is required. This only occurs because tuples are not named types we should really add more rust specific info on our gimple TYPES. gcc/rust/ChangeLog: * backend/rust-compile-type.cc (TyTyResolveCompile::visit): use get_name * typecheck/rust-tyty.cc (TupleType::get_name): likewise Signed-off-by: Philip Herron --- gcc/rust/backend/rust-compile-type.cc | 2 +- gcc/rust/typecheck/rust-tyty.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/gcc/rust/backend/rust-compile-type.cc b/gcc/rust/backend/rust-compile-type.cc index 5be2b9e317e4..903d0ce85416 100644 --- a/gcc/rust/backend/rust-compile-type.cc +++ b/gcc/rust/backend/rust-compile-type.cc @@ -454,7 +454,7 @@ TyTyResolveCompile::visit (const TyTy::TupleType &type) } tree struct_type_record = Backend::struct_type (fields); - translated = Backend::named_type (type.as_string (), struct_type_record, + translated = Backend::named_type (type.get_name (), struct_type_record, type.get_ident ().locus); } diff --git a/gcc/rust/typecheck/rust-tyty.cc b/gcc/rust/typecheck/rust-tyty.cc index 4da51ffdbfd1..09a9b97354be 100644 --- a/gcc/rust/typecheck/rust-tyty.cc +++ b/gcc/rust/typecheck/rust-tyty.cc @@ -1992,7 +1992,7 @@ TupleType::get_name () const std::string fields_buffer; for (const TyVar &field : get_fields ()) { - fields_buffer += field.get_tyty ()->as_string (); + fields_buffer += field.get_tyty ()->get_name (); bool has_next = (i + 1) < get_fields ().size (); fields_buffer += has_next ? ", " : ""; i++; From 0c061c63482f409b923e485592e6e83b67b5f90d Mon Sep 17 00:00:00 2001 From: Philip Herron Date: Mon, 19 May 2025 18:02:13 +0100 Subject: [PATCH 2/2] gccrs: Initial support for Return Position Impl Trait This is the initial patch for RPIT, we can build on this to handle the more complex cases but there are enough distinct changes going on here that it should just get merged now. RPIT is really a sneaky generic so for example: fn foo() -> impl Bar { Baz } This is represented as: fn () -> OpaqueType Bar. But when we handle the coercion site for Baz on impl Bar when we type resolve the function we know that the underlying type is Baz. Note this function is _not_ generic so its using this special OpaqueType and keeping track of the underlying type in its ty_ref reference hir-id which will resolve to Baz. This also means if we have a case where maybe this was in an if statement: fn foo(a: i32) -> impl Bar { if a > 10 { Baz } else { Qux } } The rules of impl Bar is that Baz is handled but Baz and Qux are different underlying types so this is not allowed. The reason is impl traits are not generic and although from a programmer perspective the callers dont know what the underlying type is, the compiler _knows_ what it is. So really when you call a function and get its return position impl trait the compiler knows what to do and does all whats nessecary to handle calling functions using that type etc. gcc/rust/ChangeLog: * backend/rust-compile-type.cc (TyTyResolveCompile::visit): we need to resolve the underlying type * typecheck/rust-substitution-mapper.cc (SubstMapperInternal::visit): just clone * typecheck/rust-tyty-call.cc (TypeCheckCallExpr::visit): ensure we monomphize to get the underlying * typecheck/rust-tyty.cc (BaseType::destructure): handle opaque types (OpaqueType::resolve): this is much simpler now (OpaqueType::handle_substitions): no longer needed * typecheck/rust-tyty.h: update header * typecheck/rust-unify.cc (UnifyRules::expect_opaque): unify rules for opaque gcc/testsuite/ChangeLog: * rust/compile/bad-rpit1.rs: New test. * rust/execute/torture/impl_rpit1.rs: New test. * rust/execute/torture/impl_rpit2.rs: New test. * rust/execute/torture/impl_rpit3.rs: New test. Signed-off-by: Philip Herron --- gcc/rust/backend/rust-compile-type.cc | 4 +- .../typecheck/rust-substitution-mapper.cc | 2 +- gcc/rust/typecheck/rust-tyty-call.cc | 2 +- gcc/rust/typecheck/rust-tyty.cc | 72 ++------------- gcc/rust/typecheck/rust-tyty.h | 2 - gcc/rust/typecheck/rust-unify.cc | 90 +++++++++---------- gcc/testsuite/rust/compile/bad-rpit1.rs | 26 ++++++ .../rust/execute/torture/impl_rpit1.rs | 28 ++++++ .../rust/execute/torture/impl_rpit2.rs | 36 ++++++++ .../rust/execute/torture/impl_rpit3.rs | 25 ++++++ 10 files changed, 169 insertions(+), 118 deletions(-) create mode 100644 gcc/testsuite/rust/compile/bad-rpit1.rs create mode 100644 gcc/testsuite/rust/execute/torture/impl_rpit1.rs create mode 100644 gcc/testsuite/rust/execute/torture/impl_rpit2.rs create mode 100644 gcc/testsuite/rust/execute/torture/impl_rpit3.rs diff --git a/gcc/rust/backend/rust-compile-type.cc b/gcc/rust/backend/rust-compile-type.cc index 903d0ce85416..5ca1d82b3d40 100644 --- a/gcc/rust/backend/rust-compile-type.cc +++ b/gcc/rust/backend/rust-compile-type.cc @@ -755,7 +755,9 @@ TyTyResolveCompile::visit (const TyTy::DynamicObjectType &type) void TyTyResolveCompile::visit (const TyTy::OpaqueType &type) { - translated = error_mark_node; + rust_assert (type.can_resolve ()); + auto underlying = type.resolve (); + translated = TyTyResolveCompile::compile (ctx, underlying, trait_object_mode); } tree diff --git a/gcc/rust/typecheck/rust-substitution-mapper.cc b/gcc/rust/typecheck/rust-substitution-mapper.cc index 878c4d54a0a8..23116ff80401 100644 --- a/gcc/rust/typecheck/rust-substitution-mapper.cc +++ b/gcc/rust/typecheck/rust-substitution-mapper.cc @@ -374,7 +374,7 @@ SubstMapperInternal::visit (TyTy::DynamicObjectType &type) void SubstMapperInternal::visit (TyTy::OpaqueType &type) { - resolved = type.handle_substitions (mappings); + resolved = type.clone (); } // SubstMapperFromExisting diff --git a/gcc/rust/typecheck/rust-tyty-call.cc b/gcc/rust/typecheck/rust-tyty-call.cc index 75cf58faf89e..0d187f37ee38 100644 --- a/gcc/rust/typecheck/rust-tyty-call.cc +++ b/gcc/rust/typecheck/rust-tyty-call.cc @@ -246,7 +246,7 @@ TypeCheckCallExpr::visit (FnType &type) } type.monomorphize (); - resolved = type.get_return_type ()->clone (); + resolved = type.get_return_type ()->monomorphized_clone (); } void diff --git a/gcc/rust/typecheck/rust-tyty.cc b/gcc/rust/typecheck/rust-tyty.cc index 09a9b97354be..c35a6c129489 100644 --- a/gcc/rust/typecheck/rust-tyty.cc +++ b/gcc/rust/typecheck/rust-tyty.cc @@ -546,17 +546,14 @@ BaseType::destructure () const { x = p->get (); } - // else if (auto p = x->try_as ()) - // { - // auto pr = p->resolve (); - - // rust_debug ("XXXXXX") - - // if (pr == x) - // return pr; + else if (auto p = x->try_as ()) + { + auto pr = p->resolve (); + if (pr == x) + return pr; - // x = pr; - // } + x = pr; + } else { return x; @@ -3624,28 +3621,7 @@ BaseType * OpaqueType::resolve () const { TyVar var (get_ty_ref ()); - BaseType *r = var.get_tyty (); - - while (r->get_kind () == TypeKind::OPAQUE) - { - OpaqueType *rr = static_cast (r); - if (!rr->can_resolve ()) - break; - - TyVar v (rr->get_ty_ref ()); - BaseType *n = v.get_tyty (); - - // fix infinite loop - if (r == n) - break; - - r = n; - } - - if (r->get_kind () == TypeKind::OPAQUE && (r->get_ref () == r->get_ty_ref ())) - return TyVar (r->get_ty_ref ()).get_tyty (); - - return r; + return var.get_tyty (); } bool @@ -3655,41 +3631,9 @@ OpaqueType::is_equal (const BaseType &other) const if (can_resolve () != other2.can_resolve ()) return false; - if (can_resolve ()) - return resolve ()->can_eq (other2.resolve (), false); - return bounds_compatible (other, UNDEF_LOCATION, false); } -OpaqueType * -OpaqueType::handle_substitions (SubstitutionArgumentMappings &subst_mappings) -{ - // SubstitutionArg arg = SubstitutionArg::error (); - // bool ok = subst_mappings.get_argument_for_symbol (this, &arg); - // if (!ok || arg.is_error ()) - // return this; - - // OpaqueType *p = static_cast (clone ()); - // subst_mappings.on_param_subst (*p, arg); - - // // there are two cases one where we substitute directly to a new PARAM and - // // otherwise - // if (arg.get_tyty ()->get_kind () == TyTy::TypeKind::PARAM) - // { - // p->set_ty_ref (arg.get_tyty ()->get_ref ()); - // return p; - // } - - // // this is the new subst that this needs to pass - // p->set_ref (mappings.get_next_hir_id ()); - // p->set_ty_ref (arg.get_tyty ()->get_ref ()); - - // return p; - - rust_unreachable (); - return nullptr; -} - // StrType StrType::StrType (HirId ref, std::set refs) diff --git a/gcc/rust/typecheck/rust-tyty.h b/gcc/rust/typecheck/rust-tyty.h index b14e9f25221d..e35585c6a130 100644 --- a/gcc/rust/typecheck/rust-tyty.h +++ b/gcc/rust/typecheck/rust-tyty.h @@ -441,8 +441,6 @@ class OpaqueType : public BaseType std::string get_name () const override final; bool is_equal (const BaseType &other) const override; - - OpaqueType *handle_substitions (SubstitutionArgumentMappings &mappings); }; class StructFieldType diff --git a/gcc/rust/typecheck/rust-unify.cc b/gcc/rust/typecheck/rust-unify.cc index 4cfc197c430a..219db9a6df9e 100644 --- a/gcc/rust/typecheck/rust-unify.cc +++ b/gcc/rust/typecheck/rust-unify.cc @@ -1788,59 +1788,51 @@ UnifyRules::expect_closure (TyTy::ClosureType *ltype, TyTy::BaseType *rtype) TyTy::BaseType * UnifyRules::expect_opaque (TyTy::OpaqueType *ltype, TyTy::BaseType *rtype) { - switch (rtype->get_kind ()) + if (rtype->is ()) { - case TyTy::INFER: { - TyTy::InferType *r = static_cast (rtype); - bool is_valid - = r->get_infer_kind () == TyTy::InferType::InferTypeKind::GENERAL; - if (is_valid) - return ltype->clone (); - } - break; - - case TyTy::OPAQUE: { - auto &type = *static_cast (rtype); - if (ltype->num_specified_bounds () != type.num_specified_bounds ()) - { - return new TyTy::ErrorType (0); - } + TyTy::OpaqueType *ro = rtype->as (); + if (!ltype->is_equal (*ro)) + return new TyTy::ErrorType (0); - if (!ltype->bounds_compatible (type, locus, true)) - { + if (ltype->can_resolve () && ro->can_resolve ()) + { + auto lr = ltype->resolve (); + auto rr = ro->resolve (); + + auto res = UnifyRules::Resolve (TyTy::TyWithLocation (lr), + TyTy::TyWithLocation (rr), locus, + commit_flag, false /* emit_error */, + infer_flag, commits, infers); + if (res->get_kind () == TyTy::TypeKind::ERROR) return new TyTy::ErrorType (0); - } - - return ltype->clone (); - } - break; - - case TyTy::CLOSURE: - case TyTy::SLICE: - case TyTy::PARAM: - case TyTy::POINTER: - case TyTy::STR: - case TyTy::ADT: - case TyTy::REF: - case TyTy::ARRAY: - case TyTy::FNDEF: - case TyTy::FNPTR: - case TyTy::TUPLE: - case TyTy::BOOL: - case TyTy::CHAR: - case TyTy::INT: - case TyTy::UINT: - case TyTy::FLOAT: - case TyTy::USIZE: - case TyTy::ISIZE: - case TyTy::NEVER: - case TyTy::PLACEHOLDER: - case TyTy::PROJECTION: - case TyTy::DYNAMIC: - case TyTy::ERROR: - return new TyTy::ErrorType (0); + } + else if (ltype->can_resolve ()) + { + auto lr = ltype->resolve (); + ro->set_ty_ref (lr->get_ref ()); + } + else if (ro->can_resolve ()) + { + auto rr = ro->resolve (); + ltype->set_ty_ref (rr->get_ref ()); + } } - return new TyTy::ErrorType (0); + else if (ltype->can_resolve ()) + { + auto underly = ltype->resolve (); + auto res = UnifyRules::Resolve (TyTy::TyWithLocation (underly), + TyTy::TyWithLocation (rtype), locus, + commit_flag, false /* emit_error */, + infer_flag, commits, infers); + if (res->get_kind () == TyTy::TypeKind::ERROR) + return new TyTy::ErrorType (0); + } + else + { + ltype->set_ty_ref (rtype->get_ref ()); + } + + return ltype; } } // namespace Resolver diff --git a/gcc/testsuite/rust/compile/bad-rpit1.rs b/gcc/testsuite/rust/compile/bad-rpit1.rs new file mode 100644 index 000000000000..d8c21b11c64a --- /dev/null +++ b/gcc/testsuite/rust/compile/bad-rpit1.rs @@ -0,0 +1,26 @@ +#[lang = "sized"] +trait Sized {} + +trait Foo { + fn id(&self) -> i32; +} + +struct A; +struct B; + +impl Foo for A { + fn id(&self) -> i32 { + 1 + } +} + +impl Foo for B { + fn id(&self) -> i32 { + 2 + } +} + +fn make_foo(cond: bool) -> impl Foo { + if cond { A } else { B } + // { dg-error "mismatched types, expected .A. but got .B. .E0308." "" { target *-*-* } .-1 } +} diff --git a/gcc/testsuite/rust/execute/torture/impl_rpit1.rs b/gcc/testsuite/rust/execute/torture/impl_rpit1.rs new file mode 100644 index 000000000000..8ce5f21ef87e --- /dev/null +++ b/gcc/testsuite/rust/execute/torture/impl_rpit1.rs @@ -0,0 +1,28 @@ +#[lang = "sized"] +trait Sized {} + +trait Foo { + fn id(&self) -> i32; +} + +struct Thing(i32); + +impl Foo for Thing { + fn id(&self) -> i32 { + self.0 + } +} + +fn make_thing(a: i32) -> impl Foo { + Thing(a) +} + +fn use_foo(f: impl Foo) -> i32 { + f.id() +} + +fn main() -> i32 { + let value = make_thing(42); + let val = use_foo(value); + val - 42 +} diff --git a/gcc/testsuite/rust/execute/torture/impl_rpit2.rs b/gcc/testsuite/rust/execute/torture/impl_rpit2.rs new file mode 100644 index 000000000000..f7cbbb668649 --- /dev/null +++ b/gcc/testsuite/rust/execute/torture/impl_rpit2.rs @@ -0,0 +1,36 @@ +#[lang = "sized"] +trait Sized {} + +trait Foo { + fn id(&self) -> i32; +} + +struct Thing(i32); + +impl Thing { + fn double(&self) -> i32 { + // { dg-warning "associated function is never used: .double." "" { target *-*-* } .-1 } + self.0 * 2 + } +} + +impl Foo for Thing { + fn id(&self) -> i32 { + self.0 + } +} + +fn make_thing(a: i32) -> impl Foo { + Thing(a) +} + +fn use_foo(f: impl Foo) -> i32 { + f.id() +} + +fn main() -> i32 { + let value = make_thing(21); + let id = use_foo(value); + + id - 21 +} diff --git a/gcc/testsuite/rust/execute/torture/impl_rpit3.rs b/gcc/testsuite/rust/execute/torture/impl_rpit3.rs new file mode 100644 index 000000000000..dd68eb29ef96 --- /dev/null +++ b/gcc/testsuite/rust/execute/torture/impl_rpit3.rs @@ -0,0 +1,25 @@ +#[lang = "sized"] +trait Sized {} + +trait Foo { + fn id(&self) -> i32; +} + +struct Thing(i32); + +impl Foo for Thing { + fn id(&self) -> i32 { + self.0 + } +} + +fn make_thing() -> impl Foo { + Thing(99) +} + +fn main() -> i32 { + let v = make_thing(); + let r = &v; + let val = r.id(); + val - 99 +}