Skip to content

Make overlapping_inherent_impls lint a hard error - #40728

Merged
bors merged 1 commit into
rust-lang:masterfrom
topecongiro:stabilize
Apr 1, 2017
Merged

Make overlapping_inherent_impls lint a hard error#40728
bors merged 1 commit into
rust-lang:masterfrom
topecongiro:stabilize

Conversation

@topecongiro

@topecongirotopecongiro commented Mar 22, 2017

Copy link
Copy Markdown
Contributor

Closes#36889.

@rust-highfive

rust-highfive commented Mar 22, 2017

Copy link
Copy Markdown
Contributor

r? @arielb1

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

@brsonbrson added the relnotes Marks issues that should be documented in the release notes of the next release. label Mar 23, 2017
estebank
estebank reviewed Mar 25, 2017
View reviewed changes
Comment threadsrc/librustc_typeck/coherence/inherent.rs Outdated

@estebankestebankMar 25, 2017•
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The code looks good, but could you add a span_label to this diagnostic pointing at item1's and another pointing at item2's span?

That way the code output would look this way:

error: duplicate definitions with name `id`
--> <anon>:4:5
|
4 | fn id(){}//~ ERROR duplicate definitions
| ^^^^^^^^^^ duplicate definitions for `id`
...
8 | fn id(){}
| ---------- other definition for `id`

@topecongirotopecongiroMar 27, 2017

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@estebank Thank you for your comment. I will rebase accordingly 😸

@bors

bors commented Mar 29, 2017

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #40867) made this pull request unmergeable. Please resolve the merge conflicts.

@arielb1

arielb1 commented Mar 30, 2017

Copy link
Copy Markdown
Contributor

@bors r+

@bors

bors commented Mar 30, 2017

Copy link
Copy Markdown
Collaborator

📌 Commit 11ce5b7 has been approved by arielb1

@bors

bors commented Mar 30, 2017

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 11ce5b7 with merge ba21ac0...

@bors

bors commented Mar 30, 2017

Copy link
Copy Markdown
Collaborator

💔 Test failed - status-travis

@alexcrichton

alexcrichton commented Mar 30, 2017

Copy link
Copy Markdown
Member

@bors: retry

@bors

bors commented Mar 31, 2017

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 11ce5b7 with merge c9c7c45...

@bors

bors commented Mar 31, 2017

Copy link
Copy Markdown
Collaborator

💔 Test failed - status-appveyor

@alexcrichton

alexcrichton commented Mar 31, 2017 via email

Copy link
Copy Markdown
Member

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Mar 31, 2017
@frewsxcv
Rollup merge ofrust-lang#40728- topecongiro:stabilize, r=arielb1
bd77add
Make overlapping_inherent_impls lint a hard error
Closesrust-lang#36889.
@frewsxcvfrewsxcv mentioned this pull request Mar 31, 2017
Closed
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Mar 31, 2017
@frewsxcv
Rollup merge ofrust-lang#40728- topecongiro:stabilize, r=arielb1
c6e2512
Make overlapping_inherent_impls lint a hard error
Closesrust-lang#36889.
@frewsxcvfrewsxcv mentioned this pull request Mar 31, 2017
Merged
bors added a commit that referenced this pull request Mar 31, 2017
@bors
Auto merge of#40972- frewsxcv:rollup, r=frewsxcv
2f16528
Rollup of 6 pull requests
- Successful merges: #40703, #40728, #40763, #40871, #40935, #40947
- Failed merges:
@bors
bors merged commit 11ce5b7 into rust-lang:masterApr 1, 2017
@estebank

estebank commented Apr 3, 2017

Copy link
Copy Markdown
Contributor

@topecongiro, is it me or the rebased version that was merged didn't have the "hard error" code? If that is the case, could you please send a PR with it?

@topecongiro

topecongiro commented Apr 4, 2017

Copy link
Copy Markdown
ContributorAuthor

@estebank Looks like #40683 splitted src/librustc_typeck/coherence/inherent.rs into two files and the hard error code disappeared when I rebased without paying attention. Sorry for confusion. I will create a PR shortly.

@topecongirotopecongiro mentioned this pull request Apr 4, 2017
Merged
arielb1 pushed a commit to arielb1/rust that referenced this pull request Apr 5, 2017
Ariel Ben-Yehuda
Rollup merge ofrust-lang#41052- topecongiro:overlapping_inherent_im…
a69fcfa
…pls, r=estebank
Make 'overlapping_inherent_impls' lint a hard error
This is ought to be implemented in PR rust-lang#40728. Unfortunately, when I rebased the PR to resolve merge conflict, the "hard error" code disappeared. This PR complements the initial PR.
Now the following rust code gives the following error:
```rust
struct Foo;
impl Foo {
fn id() {}
}
impl Foo {
fn id() {}
}
fn main() {}
```
```
error[E0592]: duplicate definitions with name `id`
--> /home/topecongiro/test.rs:4:5
|
4 | fn id() {}
| ^^^^^^^^^^ duplicate definitions for `id`
...
8 | fn id() {}
| ---------- other definition for `id`
error: aborting due to previous error
```
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@estebankestebankestebank left review comments

Assignees

@arielb1arielb1

Labels

relnotesMarks issues that should be documented in the release notes of the next release.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@topecongiro@rust-highfive@bors@arielb1@alexcrichton@estebank@brson