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.

Sign up for GitHub

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

Jump to bottom

Generic pointers coerceable #1931

Merged
merged 10 commits into from
Feb 28, 2023
Merged

Generic pointers coerceable #1931

merged 10 commits into from
Feb 28, 2023

Conversation

philberty
Copy link
Member

@philberty philberty commented Feb 26, 2023 •
edited
Loading

In order to fix the following issues we need to begin the journey to get rid of
the can_eq interface as well as the autoderef_flag hack inside it. In non-generic
contexts in order to call const pointer lang item methods the can_eq interface
was not possible to infer that these types are valid. This patch set adds a new
unify_and interface so that we can optionally inject inference variables and only
commit them when everything is ok and on failure cleanup. So during method
resolution and picking we can now pick using our TryCoerce interface instead
of the bad can_eq one.

Fixes #1903 #1901 #878
Addresses #1895

@philberty philberty added the bug label Feb 26, 2023
@philberty philberty added this to the Final upstream patches milestone Feb 26, 2023
@philberty philberty self-assigned this Feb 26, 2023
@philberty philberty mentioned this pull request Feb 27, 2023
Closed
41 tasks
@philberty
gccrs: Fix ICE in ADTType::is_concrete
b6a8e19 
Signed-off-by: Philip Herron <[email protected]>

gcc/rust/ChangeLog:

	* typecheck/rust-tyty.cc (ADTType::is_concrete):  need to consider if is a num_variant
	* typecheck/rust-tyty.h: refactor to cc file
@philberty philberty changed the title Generic pointers are an unsafe coerceable ptr Generic pointers coerceable Feb 28, 2023
@philberty philberty requested a review from P-E-P February 28, 2023 19:35
@philberty philberty linked an issue Feb 28, 2023 that may be closed by this pull request
Pointers are coerceable #1901
Closed
@philberty
gccrs: refactor unify commit as a static function from unify code
a44cf95 
Signed-off-by: Philip Herron <[email protected]>

gcc/rust/ChangeLog:

	* typecheck/rust-unify.cc (UnifyRules::Resolve): refactor
	(UnifyRules::commit): refactor
	* typecheck/rust-unify.h: likewise
@philberty
gccrs: Generic pointers are coerceable
fa9c51a 
This is a complex type-system change where it begins out journey to get
rid of our can_eq interface. Rust allows:

  let x:*mut T
  let y = x as *mut u8;

Which requires us to consider find a way to infer what T should be so as
to keep unify happy. This means we need to introduce a new unify_and
interface where we can optionally inject inference variables as well as
only commit the inference variable joins when they are sucsessful.

So for this case we can then inject an implicit inference variables for T
that can unify against u8 to make this a valid type-resolution.

Fixes #1930

Signed-off-by: Philip Herron <[email protected]>

gcc/rust/ChangeLog:

	* backend/rust-compile-expr.cc (CompileExpr::resolve_method_address): update to new inteface
	* typecheck/rust-coercion.cc (TypeCoercionRules::coerce_unsafe_ptr): likewise
	(TypeCoercionRules::coerce_borrowed_pointer): likewise
	* typecheck/rust-hir-type-check.h: likewise
	* typecheck/rust-type-util.cc (unify_site_and): new interface to allow for infer and commit
	* typecheck/rust-type-util.h (unify_site_and): likewise
	* typecheck/rust-typecheck-context.cc (TypeCheckContext::clear_type): new interface
	* typecheck/rust-unify.cc (UnifyRules::UnifyRules): update
	(UnifyRules::Resolve): new optional flags for commit and infer
	(UnifyRules::go): likewise
	(UnifyRules::expect_adt): refactor to use new interface
	(UnifyRules::expect_reference): likewise
	(UnifyRules::expect_pointer): likewise
	(UnifyRules::expect_array): likewise
	(UnifyRules::expect_slice): likewise
	(UnifyRules::expect_fndef): likewise
	(UnifyRules::expect_fnptr): likewise
	(UnifyRules::expect_tuple): likewise
	(UnifyRules::expect_closure): likewise
	* typecheck/rust-unify.h: refactor interface

gcc/testsuite/ChangeLog:

	* rust/compile/issue-1930.rs: New test.
@philberty
gccrs: Allow infer vars on the lhs too
4ae86b6 
We should allow implicit inference on the expected side too not just the
receiver.

Signed-off-by: Philip Herron <[email protected]>

gcc/rust/ChangeLog:

	* typecheck/rust-unify.cc (UnifyRules::go): allow lhs infer vars
@philberty
gccrs: Make coercion sites autoderef cycle optional
48c65d2 
Signed-off-by: Philip Herron <[email protected]>

gcc/rust/ChangeLog:

	* typecheck/rust-casts.cc (TypeCastRules::check): update to new interface
	* typecheck/rust-coercion.cc (TypeCoercionRules::Coerce): likewise
	(TypeCoercionRules::TryCoerce): likewise
	(TypeCoercionRules::TypeCoercionRules): likewise
	* typecheck/rust-coercion.h: likewise
	* typecheck/rust-type-util.cc (coercion_site): likewise
@philberty
gccrs: Only emit errors during type-bounds checking when required
92d3169 
Signed-off-by: Philip Herron <[email protected]>

gcc/rust/ChangeLog:

	* typecheck/rust-unify.cc (UnifyRules::go): respect the emit_errors flag
@philberty
gccrs: autoderef unconstify so we can use in non const contexts
d5397d9 
Signed-off-by: Philip Herron <[email protected]>

gcc/rust/ChangeLog:

	* backend/rust-compile-base.h: unconsify
	* backend/rust-compile.cc (HIRCompileBase::coercion_site): likewise
	(HIRCompileBase::coercion_site1): likewise
	* typecheck/rust-autoderef.cc (Adjuster::try_deref_type): likewise
	(Adjuster::try_raw_deref_type): likewise
	(Adjuster::try_unsize_type): likewise
	(AutoderefCycle::cycle): likewise
	(AutoderefCycle::try_autoderefed): likewise
	* typecheck/rust-autoderef.h: likewise
	* typecheck/rust-coercion.cc (TypeCoercionRules::select): likewise
	* typecheck/rust-coercion.h: likewise
	* typecheck/rust-hir-dot-operator.cc (MethodResolver::Probe): likewise
	(MethodResolver::select): likewise
	* typecheck/rust-hir-dot-operator.h: likewise
@philberty
gccrs: bug-fix implicit inference checks
5c9a0e4 
When generating implicit inference variables we can miss cases where the
other side might be another inference variable too but it runs the risk of
generating unending inference cycles needing more info but if they other
side is a non general inference variables like <integer> or <float> this
is safe to do so and allows us to infer mroe cases.

Signed-off-by: Philip Herron <[email protected]>

gcc/rust/ChangeLog:

	* typecheck/rust-unify.cc (UnifyRules::go): fix inference check
@philberty
gccrs: Fix method resolution to use TryCoerce
27c4590 
Rust allows us to call generic pointer methods on pointers so in non
generic contexts the old code using the bad can_eq interface couldn't
handle this case. So taking advantage of our new unify_and interface to try
and infer when required we can start using our TryCoerce interface to reuse
existing code to assemble possible candidates more acurately using the
existing coercion rules.

Fixes #1901 #878

Signed-off-by: Philip Herron <[email protected]>

gcc/rust/ChangeLog:

	* typecheck/rust-coercion.cc (TypeCoercionRules::Coerce): Add new try_flag
	(TypeCoercionRules::TypeCoercionRules): set new try flag
	(TypeCoercionRules::do_coercion): default to a final unify_and in the else case
	(TypeCoercionRules::coerce_unsafe_ptr): cannot coerce to a ptr from ref during autoderef
	(TypeCoercionRules::coerce_borrowed_pointer): respect coerceable mutability
	* typecheck/rust-coercion.h: update header
	* typecheck/rust-hir-dot-operator.cc (MethodResolver::select): use new TryCoerce interface
	(MethodResolver::append_adjustments): ensure we maintain adjustment mappings
	* typecheck/rust-hir-dot-operator.h: add new method append_adjustments
	* typecheck/rust-hir-type-check-expr.cc (TypeCheckExpr::visit): extra logging

gcc/testsuite/ChangeLog:

	* rust/compile/issue-1901.rs: New test.
@philberty
gccrs: Remove cmp_autoderef_mode hack from old autoderef
ed45b94 
Signed-off-by: Philip Herron <[email protected]>

gcc/rust/ChangeLog:

	* typecheck/rust-tyty-cmp.h: remove
	* typecheck/rust-tyty.cc (set_cmp_autoderef_mode): likewise
	(reset_cmp_autoderef_mode): likewise
	* typecheck/rust-tyty.h (set_cmp_autoderef_mode): likewise
	(reset_cmp_autoderef_mode): likewise
@philberty philberty enabled auto-merge February 28, 2023 19:51
@philberty philberty added this pull request to the merge queue Feb 28, 2023
Merged via the queue into master with commit 41890d2 Feb 28, 2023
CohenArthur
CohenArthur reviewed Mar 1, 2023
View reviewed changes
gcc/rust/typecheck/rust-coercion.cc
if (autoderef_flag && receiver_is_non_ptr)
{
// it is unsafe to autoderef to raw pointers
return CoercionResult::get_error ();
Copy link
Member

Choose a reason for hiding this comment

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

Unsafe as in "this is allowed in unsafe contexts"? Or is it always forbidden?

gcc/rust/typecheck/rust-hir-dot-operator.cc
Comment on lines +115 to +126
else if (receiver_is_ref && impl_self_is_ref)
{
const TyTy::ReferenceType &sptr
= *static_cast<const TyTy::ReferenceType *> (impl_self);
const TyTy::ReferenceType &ptr
= *static_cast<const TyTy::ReferenceType *> (raw);

// we could do this via lang-item assemblies if we refactor this
bool mut_match = sptr.mutability () == ptr.mutability ();
if (!mut_match)
return true;
}
Copy link
Member

Choose a reason for hiding this comment

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

So having ambiguous implementations for *mut T and *const T is allowed as you pointed out in zulip, but is it also allowed for &mut T and &T? Or am I misunderstanding this bit of code?

@philberty philberty deleted the phil/dev branch March 1, 2023 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CohenArthur CohenArthur CohenArthur left review comments

@P-E-P P-E-P Awaiting requested review from P-E-P

Assignees

@philberty philberty

Labels
bug
Projects
libcore 1.49
Status: Done
Milestone
Final upstream patches
Development

Successfully merging this pull request may close these issues.

Generic Pointers are castable Pointers are coerceable
2 participants
@philberty @CohenArthur