Skip to content

Conversation

@matklad
Copy link
Contributor

@matklad matklad commented Aug 17, 2019

This doesn't actually infer indexing types, but at least it walks sub-expressions!

Initially, I wanted to make Index just a new kind of BinOp (b/c indexing is kind of a binary op), so I've refactoring binop handing a bit.

However, in the end I've decided to add a separate expr kind for Index, because foo[0], &foo[1] and &mut foo[1] all seem to need slightly different handing, which is not binop-like

r? @flodiebold

Unlike ast::BinOp, it has significantly more structure to it, so it's
easier to, say, handle all assignment-like operations in the same way.
/// The `..` operator for right-open ranges
RangeRightOpen,
/// The `..=` operator for right-closed ranges
RangeRightClosed,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We use separate RangeExpr for this

@flodiebold
Copy link
Member

LGTM, I like the reorganization of the binary ops!

bors r+

bors bot added a commit that referenced this pull request Aug 17, 2019
1694: Implement initial type-inference support for Index r=flodiebold a=matklad

This doesn't actually infer indexing types, but at least it walks sub-expressions!

Initially, I wanted to make `Index` just a new kind of `BinOp` (b/c indexing is kind of a binary op), so I've refactoring binop handing a bit.

However, in the end I've decided to add a separate expr kind for Index, because `foo[0]`, `&foo[1]` and `&mut foo[1]` all seem to need slightly different handing, which is not binop-like

r? @flodiebold 

Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
@bors
Copy link
Contributor

bors bot commented Aug 17, 2019

Build succeeded

@bors bors bot merged commit 189d879 into master Aug 17, 2019
@bors bors bot deleted the ops branch August 17, 2019 16:07
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.

3 participants