Skip to content

Conversation

ldm0
Copy link
Contributor

@ldm0 ldm0 commented Apr 28, 2020

The overloaded_deref_ty is a function for derefencing a type which overloads the Deref trait. But actually this function never uses the parameter pushed in until this PR. -_-

@rust-highfive
Copy link
Contributor

r? @petrochenkov

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 28, 2020
@Dylan-DPC-zz
Copy link

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Apr 29, 2020

📌 Commit 8d2f301 has been approved by Dylan-DPC

@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 Apr 29, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 29, 2020
Fix wrong argument in autoderef process

The `overloaded_deref_ty` is a function for derefencing a type which overloads the `Deref` trait. But actually this function never uses the parameter pushed in until this PR. -_-
This was referenced Apr 29, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 29, 2020
Rollup of 6 pull requests

Successful merges:

 - rust-lang#71507 (Document unsafety in core::ptr)
 - rust-lang#71572 (test iterator chain type length blowup)
 - rust-lang#71617 (Suggest `into` instead of `try_into` if possible with int types)
 - rust-lang#71627 (Fix wrong argument in autoderef process)
 - rust-lang#71678 (Add an index page for nightly rustc docs.)
 - rust-lang#71680 (Fix doc link to Eq trait from PartialEq trait)

Failed merges:

 - rust-lang#71597 (Rename Unique::empty() -> Unique::dangling())

r? @ghost
@bors bors merged commit 75561a5 into rust-lang:master Apr 29, 2020
@RalfJung
Copy link
Member

RalfJung commented May 6, 2020

Does this change behavior in any way? Is there a test case we could add to ensure it does not regress?

@ldm0
Copy link
Contributor Author

ldm0 commented May 6, 2020

@RalfJung I'm pretty sure this won't cause a regress. This just correct the function to do the thing it meant to do. Is this related to some bug? I can help reviewing.

@RalfJung
Copy link
Member

RalfJung commented May 6, 2020

Sorry, what I meant is -- this fixes a bug, right? If so, there should be a test to make sure that in the future, this does not get changed back to the old, wrong behavior that we had before your PR.

@ldm0
Copy link
Contributor Author

ldm0 commented May 6, 2020

@RalfJung This doesn't fix a bug directly. This actually fixes future bugs. The situation is like this:

a = 1
b = 2
add(a, b) {
    return a + 2
}
print(add(1, 2))

Here we get correct answer 3. But in the future when we calls add(1, 3), it quietly causes a bug (and it will be extremely hard to digout in such a large codebase). This PR just changes a + 2 to a + b.

And this function is an internal function and only called in one place so for a long time this bug is left undiscovered. I think for regression avoiding, a unit test is needed, but unit tests seem to be not exist in current compiler. :-(

@ldm0 ldm0 deleted the autoderefarg branch May 23, 2020 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants