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.

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

[WIP] Lint duplicate trait and lifetime bounds #57909

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
5 participants
@alexreg
Copy link
Contributor

alexreg commented Jan 26, 2019

For example, T: Foo + Foo, 'a: 'b + 'b, or in-band T: Foo together with where T: Foo, etc.

I ran into a brick wall with htis one, as it's a lot trickier than it first seems, though I should have a fair bit of boilerplate in place here. Leaving this for someone else to pick up hopefully (ideally before it bitrots into oblivion!).

CC @nikomatsakis

@rust-highfive

This comment was marked as resolved.

Copy link
Collaborator

rust-highfive commented Jan 26, 2019

r? @oli-obk

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

@alexreg

This comment has been minimized.

Copy link
Contributor Author

alexreg commented Jan 26, 2019

r? @alexreg (not because there's anything to review, but just so it doesn't appear in someone else's queue)

@rust-highfive rust-highfive assigned alexreg and unassigned oli-obk Jan 26, 2019

@alexreg alexreg force-pushed the alexreg:lint-duplicate-bounds branch from b7c700f to 47a0dfe Jan 26, 2019

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Jan 26, 2019

The job x86_64-gnu-llvm-6.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_time:end:294d575b:start=1548469779857298814,finish=1548469780845276571,duration=987977757
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
Setting environment variables from .travis.yml
$ export IMAGE=x86_64-gnu-llvm-6.0
---
    97% |███████████████████████████████▍| 532kB 57.4MB/s eta 0:00:01
    99% |████████████████████���███████████| 542kB 55.8MB/s eta 0:00:01
    100% |████████████████████████████████| 552kB 26.3MB/s 
Collecting botocore==1.12.86 (from awscli)
  Downloading https://files.pythonhosted.org/packages/d7/af/fd9c0f1f0fdc03d3367a56f35093f8b1020ba1a97ead9fa580156895944b/botocore-1.12.86-py2.py3-none-any.whl (5.2MB)
    0% |▏                               | 20kB 28.4MB/s eta 0:00:01
    0% |▏                               | 30kB 33.8MB/s eta 0:00:01
    0% |▎                               | 40kB 38.0MB/s eta 0:00:01
    0% |▎                               | 51kB 42.1MB/s eta 0:00:01
---
[00:06:24]    Compiling arena v0.0.0 (/checkout/src/libarena)
[00:06:24]    Compiling syntax_pos v0.0.0 (/checkout/src/libsyntax_pos)
[00:06:28]    Compiling rustc_errors v0.0.0 (/checkout/src/librustc_errors)
[00:07:47]    Compiling syntax_ext v0.0.0 (/checkout/src/libsyntax_ext)
[00:07:48] error[E0428]: the name `DEPRECATED_IN_FUTURE` is defined multiple times
[00:07:48]    --> src/librustc/lint/mod.rs:112:9
[00:07:48]     |
[00:07:48] 112 |           $vis static $NAME: &$crate::lint::Lint = &$crate::lint::Lint {
[00:07:48]     |           |
[00:07:48]     |           |
[00:07:48]     |           `DEPRECATED_IN_FUTURE` redefined here
[00:07:48]     |           previous definition of the value `DEPRECATED_IN_FUTURE` here
[00:07:48]    ::: src/librustc/lint/builtin.rs:349:1
[00:07:48]     |
[00:07:48]     |
[00:07:48] 349 | / declare_lint! {
[00:07:48] 350 | |     pub DEPRECATED_IN_FUTURE,
[00:07:48] 351 | |     Allow,
[00:07:48] 352 | |     "detects use of items that will be deprecated in a future version",
[00:07:48] 353 | |     report_in_external_macro: true
[00:07:48]     | |_- in this macro invocation
[00:07:48]     |
[00:07:48]     |
[00:07:48]     = note: `DEPRECATED_IN_FUTURE` must be defined only once in the value namespace of this module
[00:07:53] error: unused import: `errors::DiagnosticBuilder`
[00:07:53]  --> src/librustc/traits/util.rs:1:5
[00:07:53]   |
[00:07:53] 1 | use errors::DiagnosticBuilder;

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

// Lint duplicate bounds.
let bounds = predicates.iter().filter_map(|(pred, sp)| {
match pred {
Predicate::Trait(ref pred) =>

This comment has been minimized.

@Centril

Centril Jan 26, 2019

Contributor

Use default match bindings?

let mut err = tcx.struct_span_lint_node(
lint::builtin::DUPLICATE_BOUNDS,
node_id,
bounds[range.clone()].iter().map(|(_, _, sp)| *sp).collect::<Vec<_>>(),

This comment has been minimized.

@Centril

Centril Jan 26, 2019

Contributor
Suggested change Beta
bounds[range.clone()].iter().map(|(_, _, sp)| *sp).collect::<Vec<_>>(),
bounds[range.clone()].iter().map(|(.., sp)| *sp).collect::<Vec<_>>(),

This comment has been minimized.

@alexreg

alexreg Jan 26, 2019

Author Contributor

I think it's clearer this way.

bound_name));
debug!("zzz: 1: {:?} / {:?}", bounds[range.start].0, bounds[range.start].1);
err.span_label(bounds[range.start].2, "first use of bound");
for i in (range.start + 1)..range.end {

This comment has been minimized.

@Centril

Centril Jan 26, 2019

Contributor

Why not get a subslice and iterate over that + pattern match out the parts of the tuple?

This comment has been minimized.

@alexreg

alexreg Jan 26, 2019

Author Contributor

Because it's important to have the indices, as you can see from the following code.

This comment has been minimized.

@Centril

Centril Jan 26, 2019

Contributor

You can use .enumerate() to get the indices :)

This comment has been minimized.

@alexreg

alexreg Jan 27, 2019

Author Contributor

Yeah, but the thing is I'm keeping track of the start index of a "run"... this sort of made most sense to me. shrug


let mut seq_start = 0;
for i in 1..bounds.len() {
if (&bounds[i].0, &bounds[i].1) != (&bounds[i - 1].0, &bounds[i - 1].1) {

This comment has been minimized.

@Centril

Centril Jan 26, 2019

Contributor

You can use tuple_windows from itertools if available to express this more nicely.

This comment has been minimized.

@alexreg

alexreg Jan 26, 2019

Author Contributor

If only... introducing dependencies on itertools in the compiler is a big no no.

This comment has been minimized.

@Centril

Centril Jan 26, 2019

Contributor

Bummer... I would have expected it to already be used somewhere... :/

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Feb 7, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment