Skip to content

Warn unused type aliases#37631

Closed
sanxiyn wants to merge 3 commits into
rust-lang:masterfrom
sanxiyn:unused-type-alias-2
Closed

Warn unused type aliases#37631
sanxiyn wants to merge 3 commits into
rust-lang:masterfrom
sanxiyn:unused-type-alias-2

Conversation

@sanxiyn

@sanxiyn sanxiyn commented Nov 7, 2016

Copy link
Copy Markdown
Contributor

The interesting part (type aliases used by UFCS) is already tested by issue-23808.rs. But it only tests used type aliases are not warned, so I added a simple test to test unused type aliases are warned.

Fix #37455.

@rust-highfive

Copy link
Copy Markdown
Contributor

r? @Aatch

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

@petrochenkov

Copy link
Copy Markdown
Contributor

r? @eddyb
(Overlap with lazy-start)

@eddyb

eddyb commented Nov 7, 2016

Copy link
Copy Markdown
Contributor

What's the motivation between the changes to how path resolutions are stored?
I have a somewhat more complex design I've been working on, which satisfies the needs of incremental and on-demand type-checking.
It'd be already up but I'm still trying to untangle the privacy pass from syntactical types.

@sanxiyn

sanxiyn commented Nov 8, 2016

Copy link
Copy Markdown
Contributor Author

The motivation is to read partial resolution after it is overwritten by full resolution in def_map, so that UFCS path can keep type alias alive.

It can be achieved by simply keeping a copy of def_map as returned by resolve, but this (def_map only stores full resolution) seemed cleaner to me.

@eddyb

eddyb commented Nov 8, 2016

Copy link
Copy Markdown
Contributor

@sanxiyn Ah this is completely unnecessary on my branch, which has a separate HIR node for the original resolution (i.e. the T in T::A). Do you mind waiting for that?

@sanxiyn

sanxiyn commented Nov 8, 2016

Copy link
Copy Markdown
Contributor Author

Sure. It would be much better if I can get the original resolution directly from HIR node without maintaining the side table. Thanks.

@eddyb

eddyb commented Nov 9, 2016

Copy link
Copy Markdown
Contributor

@sanxiyn #37676 is up, you can probably rebase on top of it if you want (although I should fix Travis first).

@nikomatsakis nikomatsakis assigned eddyb and unassigned Aatch Nov 10, 2016
@brson

brson commented Nov 11, 2016

Copy link
Copy Markdown
Contributor

Nice fix.

bors added a commit to rust-lang/cargo that referenced this pull request Nov 15, 2016
Remove unused type aliases

Found by rust-lang/rust#37631 and necessary to land because of cargotest.
@bors

bors commented Nov 17, 2016

Copy link
Copy Markdown
Collaborator

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

@sanxiyn sanxiyn force-pushed the unused-type-alias-2 branch from 41e0b5a to 6c3af17 Compare November 18, 2016 09:37
@sanxiyn sanxiyn force-pushed the unused-type-alias-2 branch from 6c3af17 to 2f858e7 Compare November 22, 2016 07:29
@bors

bors commented Nov 28, 2016

Copy link
Copy Markdown
Collaborator

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

@eddyb

eddyb commented Nov 28, 2016

Copy link
Copy Markdown
Contributor

Sorry for the delay but this should be straight-forward now.

@petrochenkov

Copy link
Copy Markdown
Contributor

Should this be closed?

@sanxiyn

sanxiyn commented Dec 17, 2016

Copy link
Copy Markdown
Contributor Author

I will close this when the reimplementation lands.

bors added a commit that referenced this pull request Dec 18, 2016
 Warn unused type aliases, reimplemented

Reimplementation of #37631. Fix #37455.
@sanxiyn sanxiyn closed this Dec 19, 2016
@sanxiyn sanxiyn deleted the unused-type-alias-2 branch December 19, 2016 02:32
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.

7 participants