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

Better diagnostic for binary operation on BoxedValues #59467

Merged
merged 2 commits into from
Mar 29, 2019
Merged

Better diagnostic for binary operation on BoxedValues #59467

merged 2 commits into from
Mar 29, 2019

Conversation

hgallagher1993
Copy link
Contributor

Fixes #59458

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @estebank (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 27, 2019
estebank
estebank reviewed Mar 27, 2019
View reviewed changes
Copy link
Contributor

@estebank estebank left a comment

Choose a reason for hiding this comment

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

Couple of minor nitpicks, but otherwise r=me

src/test/ui/derives/derives-span-PartialEq-tuple-struct.stderr Outdated
@@ -3,6 +3,9 @@ error[E0369]: binary operation `==` cannot be applied to type `Error`
|
LL | Error
| ^^^^^
| |
| Error
| Error
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a check for the case when both left and right overlap to avoid this output?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is fixed

src/test/ui/span/issue-39018.stderr Outdated
| | |
| | &str
| &str
| `+` can't be used to concatenate two `&str` strings
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you change the span for the explanation to use the span pointing at the operator?

Copy link
Contributor Author

@hgallagher1993 hgallagher1993 Mar 28, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I think this is fixed? Not sure if issue-47380.stderr looks right though

@estebank
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 29, 2019

📌 Commit 4644c3a has been approved by estebank

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 29, 2019
Centril added a commit to Centril/rust that referenced this pull request Mar 29, 2019
@Centril
Rollup merge of rust-lang#59467 - hgallagher1993:local_branch, r=este…
fda2066 
…bank

Better diagnostic for binary operation on BoxedValues

Fixes rust-lang#59458
@Centril Centril mentioned this pull request Mar 29, 2019
Merged
bors added a commit that referenced this pull request Mar 29, 2019
@bors
Auto merge of #59522 - Centril:rollup, r=Centril
d920dfc 
Rollup of 9 pull requests

Successful merges:

 - #59366 (Update books)
 - #59436 (Update jemalloc-sys to version 0.3.0)
 - #59454 (Update rustfmt to 1.2.0)
 - #59462 (Fix error in Rust 2018 + no_core environment)
 - #59467 (Better diagnostic for binary operation on BoxedValues)
 - #59473 (Do not emit incorrect borrow suggestion involving macros and fix overlapping multiline spans)
 - #59480 (Update stdsimd)
 - #59486 (Visit `ImplItem` in `dead_code` lint)
 - #59510 (Rename `type_parameters` to `generics` and so on)

Failed merges:

 - #59516 (Update cargo)

r? @ghost
bors added a commit that referenced this pull request Mar 29, 2019
@bors
Auto merge of #59522 - Centril:rollup, r=Centril
e782d79 
Rollup of 9 pull requests

Successful merges:

 - #59366 (Update books)
 - #59436 (Update jemalloc-sys to version 0.3.0)
 - #59454 (Update rustfmt to 1.2.0)
 - #59462 (Fix error in Rust 2018 + no_core environment)
 - #59467 (Better diagnostic for binary operation on BoxedValues)
 - #59473 (Do not emit incorrect borrow suggestion involving macros and fix overlapping multiline spans)
 - #59480 (Update stdsimd)
 - #59486 (Visit `ImplItem` in `dead_code` lint)
 - #59510 (Rename `type_parameters` to `generics` and so on)

Failed merges:

 - #59516 (Update cargo)

r? @ghost
@bors bors merged commit 4644c3a into rust-lang:master Mar 29, 2019
@hgallagher1993 hgallagher1993 deleted the local_branch branch March 30, 2019 05:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@estebank estebank estebank left review comments

Assignees

@estebank estebank

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hgallagher1993 @rust-highfive @estebank @bors