Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Avoid type-checking addition and indexing twice. #40863

Merged
merged 1 commit into from
Apr 6, 2017

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Mar 27, 2017

Fixes #40610 by moving the common check_expr_coercable_to_type call before the error reporting logic for binops and removing the one from check_str_addition.
Fixes #40861 by removing an unnecessary check_expr_coercable_to_type call.

@eddyb eddyb added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Mar 27, 2017
@rust-highfive
Copy link
Contributor

r? @pnkfelix

(rust_highfive has picked a reviewer for you, use r? to override)

@arielb1
Copy link
Contributor

arielb1 commented Mar 27, 2017

@bors r+

@bors
Copy link
Collaborator

bors commented Mar 27, 2017

📌 Commit 112f36a has been approved by arielb1

@eddyb
Copy link
Member Author

eddyb commented Mar 27, 2017

@bors r- Travis failed (I'll investigate later)

@nikomatsakis nikomatsakis added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Apr 4, 2017
@nikomatsakis
Copy link
Contributor

Accepting for beta, once @eddyb fixes travis failures.

Small patch, regression.

cc @rust-lang/compiler

bors added a commit that referenced this pull request Apr 5, 2017
[beta] Backport accepted nominations

This is a backport of

* #40813
* #40849

This also includes #41069

Finally, this includes a bump to beta .3.

This is all current nominations except #40863, which is not passing tests yet.
@eddyb eddyb force-pushed the coerce-only-once branch from 112f36a to edc7f9a Compare April 6, 2017 18:42
@eddyb
Copy link
Member Author

eddyb commented Apr 6, 2017

I suspect there's another bug elsewhere but at least I hope I can make this work.

@@ -257,9 +262,6 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
}
};

// see `NB` above
self.check_expr_coercable_to_type(rhs_expr, rhs_ty_var);

(rhs_ty_var, return_ty)
Copy link
Member Author

Choose a reason for hiding this comment

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

Tests weren't passing locally (actually, libcollections wasn't building at all after a rebase) when rhs_ty_var was replaced with rhs_ty.

@eddyb
Copy link
Member Author

eddyb commented Apr 6, 2017

@bors r=arielb1

@bors
Copy link
Collaborator

bors commented Apr 6, 2017

📌 Commit edc7f9a has been approved by arielb1

@eddyb
Copy link
Member Author

eddyb commented Apr 6, 2017

cc @alexcrichton This should be ready for backport now, apologies for the delay.

@bors
Copy link
Collaborator

bors commented Apr 6, 2017

⌛ Testing commit edc7f9a with merge 50c1864...

bors added a commit that referenced this pull request Apr 6, 2017
Avoid type-checking addition and indexing twice.

Fixes #40610 by moving the common `check_expr_coercable_to_type` call before the error reporting logic for binops and removing the one from `check_str_addition`.
Fixes #40861 by removing an unnecessary `check_expr_coercable_to_type` call.
@bors
Copy link
Collaborator

bors commented Apr 6, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: arielb1
Pushing 50c1864 to master...

@bors bors merged commit edc7f9a into rust-lang:master Apr 6, 2017
@eddyb eddyb deleted the coerce-only-once branch April 7, 2017 06:22
@alexcrichton alexcrichton removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Apr 20, 2017
bors added a commit that referenced this pull request Apr 21, 2017
[beta] Final backports to beta

Backport of:

* #40863
* #41085
* #41354
* #41378
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants