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

Integer and float variable kinds #470

Merged
merged 8 commits into from
May 26, 2020
Merged

Integer and float variable kinds #470

merged 8 commits into from
May 26, 2020

Conversation

AzureMarker
Copy link
Member

Closes #327

This adds support for integer and float variable kinds. Example usage:

test! {
    program {
        trait Foo {}
        struct Bar {}

        impl Foo for usize {}
        impl Foo for Bar {}
    }

    goal {
        exists<int N> {
            N: Foo
        }
    } yields {
        "Unique; substitution [?0 := Uint(Usize)]"
    }
}

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

The main problem I see is that int/float are not 'distinct' from ordinary types. I think we need some tests like

exists<T, int N> {
    T = N, N = usize
}

this should pass, and something like

exists<T, int N> {
    T = N, T = char
}

this should have no solution.

chalk-ir/src/debug.rs Show resolved Hide resolved
chalk-ir/src/debug.rs Show resolved Hide resolved
chalk-solve/src/infer/invert.rs Outdated Show resolved Hide resolved
chalk-solve/src/infer/unify.rs Outdated Show resolved Hide resolved
tests/test/numerics.rs Show resolved Hide resolved
@nikomatsakis
Copy link
Contributor

I guess I didn't expect to add an enum to Ty but rather to have 5 variants:

  • Ty
  • IntTy
  • FloatTy
  • Lifetime
  • Const(Ty)

but I'm not sure which setup is nicer. This one may be more elegant.

@AzureMarker
Copy link
Member Author

I guess I didn't expect to add an enum to Ty but rather to have 5 variants

Adding the enum to VariableKind::Ty is effectively the same thing (now you have Ty(General), Ty(Integer), and Ty(Float)), but it's easier to group the "Ty-like" variants together (Ty(_)).

@AzureMarker AzureMarker requested a review from nikomatsakis May 25, 2020 17:10
(
&TyData::InferenceVar(var1, kind1),
&TyData::InferenceVar(var2, kind2),
) if kind1 == kind2 => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Maybe this would be nicer if we combined the three cases and moved the if into the arm?

I'm imagining

(
                &TyData::InferenceVar(var1, kind1),
                &TyData::InferenceVar(var2, kind2),
            ) => {
    if kind1 == kind2 {
        ...
    } else if kind1.is_general() || kind2.is_general() {
        ...
    } else {
        Err(...)
    }
}

goal {
exists<T, float N> {
T = N, N = f32
}
Copy link
Contributor

Choose a reason for hiding this comment

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

let's add

exists<int N, float F> { N = F }

and check it gives an error

This introduces a new notation to distinguish integer inference
variables from general inference variables: ?0i. The "i" represents that
it is an integer variable.

Another new notation is for integer variables in goals. Similar to
consts, integer variables are notated as `int N`, for example:
```
exists<int N> { ... }
```
Adds new notation for float inference variables, just like what was
added for integers.
- `?0f` is a float inference variable
- `float N` is the notation for float variables in goals, for example:
  ```
  exists<float N> { ... }
  ```
The recursive solver would fail on these new tests without this change.
@nikomatsakis nikomatsakis merged commit f4977ab into rust-lang:master May 26, 2020
@AzureMarker AzureMarker deleted the feature/integer-float-kinds branch May 26, 2020 23:04
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.

Handling integer/float variables
2 participants