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

Add description for LUB Coercion #808

Merged
merged 2 commits into from Oct 5, 2020
Merged

Conversation

@ldm0
Copy link
Contributor

@ldm0 ldm0 commented May 12, 2020

As requested in rust-lang/rust#71599. Also references the rust-lang/rust#48109 (comment)

Copy link
Collaborator

@Havvy Havvy left a comment

First off, thanks for writing this! That said, it looks very rushed and doesn't actually describe what these coercions actually are. I don't actually know the algorithm for the coercion, so just including that would be tremendously helpful, but I've listed extensive notes on my thoughts on the contents of this PR. How much you want to turn them into work for yourself is up to you, except again, we do need the actual algorithm.

src/type-coercions.md Outdated Show resolved Hide resolved
src/type-coercions.md Outdated Show resolved Hide resolved
src/type-coercions.md Outdated Show resolved Hide resolved
src/type-coercions.md Outdated Show resolved Hide resolved
src/type-coercions.md Outdated Show resolved Hide resolved
src/type-coercions.md Outdated Show resolved Hide resolved
src/type-coercions.md Show resolved Hide resolved
src/type-coercions.md Outdated Show resolved Hide resolved
@ldm0
Copy link
Contributor Author

@ldm0 ldm0 commented May 12, 2020

Notice I added explanation for LubCoerce with expected type in the end. That's because it's really how rustc currently works. You can check that by test these:

This compiles:

let esc: &[u8] = match true {
    true => b"ab", // &[u8; 2]
    false => b"c",  // &[u8; 1]
};

This failes to compile:

let esc = match true {
    true => b"ab",
    false => b"c",
};

Why?

  1. Lub(&[u8], &[u8; 2]) == &[u8]
  2. Lub(&[u8], &[u8; 1]) == &[u8]
  3. Lub(&[u8; 2], &[u8; 1]) fails
@ldm0 ldm0 requested a review from Havvy May 13, 2020
src/type-coercions.md Outdated Show resolved Hide resolved
src/type-coercions.md Outdated Show resolved Hide resolved
src/type-coercions.md Outdated Show resolved Hide resolved
src/type-coercions.md Outdated Show resolved Hide resolved
src/type-coercions.md Outdated Show resolved Hide resolved
src/type-coercions.md Outdated Show resolved Hide resolved
src/type-coercions.md Outdated Show resolved Hide resolved
src/type-coercions.md Outdated Show resolved Hide resolved
@ldm0 ldm0 force-pushed the ldm0:lubcoercion branch from 699130e to afcb386 May 13, 2020
@ldm0 ldm0 requested a review from Havvy May 13, 2020
@Havvy
Copy link
Collaborator

@Havvy Havvy commented May 13, 2020

I've sent my review as a new PR @ ldm0#1

@Havvy Havvy requested a review from ehuss May 13, 2020
Copy link
Collaborator

@Havvy Havvy left a comment

Looking at @ehuss for final approval.

(Also, I think the algorithm might be better expressed in text than code after looking at it, but that can be done later)

@Havvy
Havvy approved these changes May 13, 2020
Copy link
Collaborator

@ehuss ehuss left a comment

I also generally prefer to not introduce ad-hoc pseudo-code, since it is undefined and open to interpretation. However, I think something is better than nothing, so it's probably fine for now. I also suspect a truly formal definition would be a much greater effort.

I'm not qualified to comment on the actual rules, since I know nothing about type coercion. I would be more comfortable with a lang team member to approve.

src/type-coercions.md Outdated Show resolved Hide resolved
src/type-coercions.md Outdated Show resolved Hide resolved
@ldm0
Copy link
Contributor Author

@ldm0 ldm0 commented May 14, 2020

Waiting on a lang team member to approve. :D

To check the correctness of the pseudo code, reviewers can check CoerceMany::coerce_inner() and FnCtxt::try_find_coercion_lub(). To check how compiler processes the expected type, reviewers can check FnCtxt::check_match().

@ldm0

This comment has been minimized.

src/type-coercions.md Outdated Show resolved Hide resolved
@ldm0 ldm0 force-pushed the ldm0:lubcoercion branch from e084b8d to edda76f May 18, 2020
@nikomatsakis nikomatsakis self-assigned this May 27, 2020
Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Gave this a first read. Sorry for the long delay. I'll try to offer some more concrete suggestions in a bit. I am wondering if we can do something less "pseduo-code" and a bit higher-level -- that said, the algorithm is kind of "operational" though.

src/type-coercions.md Outdated Show resolved Hide resolved
```

In this example, both `foo` and `bar` has the type
`LubCoerce(typeof(a), typeof(a), typeof(c))` and `baz` has the type

This comment has been minimized.

@nikomatsakis

nikomatsakis Jun 2, 2020
Contributor

Hmm this notation is kind of ad-hoc -- I expected to see an actual type here. I guess I might say something like:

The type of foo and bar is the result of applying the LUB coercion to the types of a, b, and c.


LUB coercion is performed by the following algorithm:

```txt

This comment has been minimized.

@nikomatsakis

nikomatsakis Jun 2, 2020
Contributor

I wonder btw if we should incorporate mdbook-mermaid into the reference ... then we could use a flow-chart here -- not sure how much better that would be ;)

This comment has been minimized.

@ldm0

ldm0 Jun 2, 2020
Author Contributor

cc @ehuss

LUB coercion is performed by the following algorithm:

```txt
Lub(a: Type, b: Type):

This comment has been minimized.

@nikomatsakis

nikomatsakis Jun 2, 2020
Contributor

Hmm, the actual code seems to have a bit more of a "bias", i.e., it has a notion of "old and new", and ordering I think maybe can matter. I wonder if we can write this section with a bit less specificity -- let me check how the coercion section is written.

This comment has been minimized.

@ldm0

ldm0 Jun 2, 2020
Author Contributor

@nikomatsakis Thanks for reviewing. Does the "old and new" means the prev_ty and new_ty in the code? I think it's represented in this part:

LubCoerce(vars...):
  result = vars.get(0)
  for var in vars[1..]:
    result = Lub(result, var)
  return result

Where result represents the prev_ty and var represents the new_ty. If you mean other things with "old and new". Could you elaborate more?

But for the ordering, I re-thinked about it and found it do matters in some cases(some edge cases). I will remove the property 1 later.

This comment has been minimized.

@nikomatsakis

nikomatsakis Jun 10, 2020
Contributor

Yes, old and new does correspond to prev and new.

@ldm0 ldm0 force-pushed the ldm0:lubcoercion branch from edda76f to 4370fdc Jun 2, 2020
@ldm0 ldm0 requested a review from nikomatsakis Jun 10, 2020
@nikomatsakis
Copy link
Contributor

@nikomatsakis nikomatsakis commented Jun 10, 2020

So I guess an alternative would be to describe the LUB coercion more informally and less precisely. Something like the following (@ldm0 you would obviously want to amend this to include the coercion from FnDef/closures to fn pointers that started all of this). I'm curious @ehuss or others what you think of these two options.

I think I mildly prefer an informal description like this one, because I think that it is probably sufficient to give readers an intuition for what the compiler is trying to do, and I suspect that the more precise description is ultimately not sufficiently precise to be "authoratative" and hence we're better off leaving that job to the code itself.


In some contexts, the compiler must coerce together multiple types to try and find the most general type. This is called a "Least Upper Bound" coercion. Some examples where the LUB coercion algorithm is used are:

  • to find the common type for a series of match arms; or
  • to find the common type for the return type of a closure with multiple return statements.

In each such case, there are a set of types T0..Tn to be mutually coerced to some target type T_t, which is unknown to start. Computing the LUB coercion is done iteratively. The target type T_t begins as the type T0. For each new type Ti, we consider whether

  • If Ti can be coerced to the current target type T_t, then no change is made.
  • Otherwise, we check whether T_t can be coerced to Ti; if so, the T_t is changed to Ti. (This check is also conditioned on whether all of the source expressions considered thus far have implicit coercions.)
  • If not, we try to compute a mutual supertype of T_t and Ti, which will become the new target type.

Examples

XXX give some interesting examples

Caveat

This description is obviously informal. Making it more precise is expected to proceed as part of a general effort to specify the Rust type checker more precisely.

@Havvy
Copy link
Collaborator

@Havvy Havvy commented Jun 11, 2020

The list of where LUB coercions happen should at least be an exhaustive list, not just "examples where it is performed".

@Havvy
Copy link
Collaborator

@Havvy Havvy commented Aug 19, 2020

Where are we with this?

@ldm0 ldm0 force-pushed the ldm0:lubcoercion branch from 4370fdc to 2f4da5c Sep 3, 2020
@ldm0 ldm0 requested review from Havvy and ehuss Sep 3, 2020
@ldm0
Copy link
Contributor Author

@ldm0 ldm0 commented Sep 3, 2020

Sorry for the long delay. Rewrite done. Exhaustive list added

src/type-coercions.md Outdated Show resolved Hide resolved
@ldm0 ldm0 force-pushed the ldm0:lubcoercion branch from 1100aec to d5a5e32 Sep 16, 2020
@ldm0 ldm0 requested a review from ehuss Sep 16, 2020
@nikomatsakis
Copy link
Contributor

@nikomatsakis nikomatsakis commented Oct 5, 2020

Thanks!

@nikomatsakis nikomatsakis merged commit 0f6b234 into rust-lang:master Oct 5, 2020
1 check passed
1 check passed
Test
Details
@ldm0 ldm0 deleted the ldm0:lubcoercion branch Oct 6, 2020
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Oct 14, 2020
Update books

## reference

5 commits in 56a13c082ee90736c08d6abdcd90462517b703d3..1b78182e71709169dc0f1c3acdc4541b6860e1c4
2020-09-14 23:20:16 -0700 to 2020-10-11 13:53:47 -0700
- Specify that SSE4.1 includes SSSE3 instead of SSE3 (rust-lang/reference#892)
- Fix mutable expressions that can be dereferenced (rust-lang/reference#890)
- Fix grammar in memory model (rust-lang/reference#889)
- Add style checks. (rust-lang/reference#886)
- Add description for LUB Coercion (rust-lang/reference#808)

## book

1 commits in cb28dee95e5e50b793e6ba9291c5d1568d3ad72e..451a1e30f2dd137aa04e142414eafb8d05f87f84
2020-09-09 10:06:00 -0500 to 2020-10-05 09:11:18 -0500
- clarify description of when ? can be used (rust-lang/book#2471)

## rust-by-example

1 commits in 7d3ff1c12db08a847a57a054be4a7951ce532d2d..152475937a8d8a1f508d8eeb57db79139bc803d9
2020-09-28 15:54:25 -0300 to 2020-10-09 09:29:50 -0300
- Add 1.45.0 cast documentation (rust-lang/rust-by-example#1384)

## embedded-book

2 commits in dd310616308e01f6cf227f46347b744aa56b77d9..79ab7776929c66db83203397958fa7037d5d9a30
2020-09-26 08:54:08 +0000 to 2020-10-12 08:00:05 +0000
- llvm-objdump: Use two hyphens in flags to objdump  (rust-embedded/book#270)
- Start/hardware: clarify which file needs tweaking  (rust-embedded/book#266)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.