events.action: opened
events.author: estebank
events.masked_author: username_0
events.text: Fix #63987.
events.title: Add check for overlapping ranges to unreachable patterns lint
events.type: issue
events.action: created
events.author: rust-highfive
events.masked_author: username_1
events.text: r? @username_9
  
  (rust_highfive has picked a reviewer for you, use r? to override)
events.type: comment
events.action: created
events.author: estebank
events.masked_author: username_0
events.text: Should this be separated to new lint instead of overloading `unreachable_patterns`?
  
  I'm explicitly not handling the "multiple holes caught by `_`" case because it has the potential of being extremely annoying. It likely belongs in clippy complaining about `_` being bad style.
  
  CC @username_2 @oli-obk @username_5
events.type: comment
events.action: created
events.author: varkor
events.masked_author: username_2
events.text: I'll look at the code more closely later, but I think this should be pulled out into its own lint, as I think there are legitimate reasons to want overlapping ranges (though overlapping by a single element seems less intentional).
events.type: comment
events.action: created
events.author: rust-highfive
events.masked_author: username_1
events.text: The job `x86_64-gnu-llvm-6.0` of your PR [failed](https://dev.azure.com/rust-lang/rust/_build/results?buildId=7001&view=logs&jobId=d0d954b5-f111-5dc4-4d76-03b6c9d0cf7e) ([raw log](https://dev.azure.com/rust-lang/e71b0ddf-dd27-435a-873c-e30f86eea377/_apis/build/builds/7001/logs/49)). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.
  
  <details><summary><i>Click to expand the log.</i></summary>
  
  ```plain
  2019-08-29T23:24:43.6838383Z ##[command]git remote add origin https://github.com/rust-lang/rust
  2019-08-29T23:24:43.7040191Z ##[command]git config gc.auto 0
  2019-08-29T23:24:43.7124890Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
  2019-08-29T23:24:43.7174202Z ##[command]git config --get-all http.proxy
  2019-08-29T23:24:43.7315436Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/64007/merge:refs/remotes/pull/64007/merge
  ---
  2019-08-30T00:00:53.2149461Z    Compiling syntax_pos v0.0.0 (/checkout/src/libsyntax_pos)
  2019-08-30T00:00:56.8983572Z error: multiple patterns covering the same range
  2019-08-30T00:00:56.8984603Z    --> src/librustc_target/abi/mod.rs:494:13
  2019-08-30T00:00:56.8985035Z     |
  2019-08-30T00:00:56.8985977Z 493 |             -0x0000_0000_0000_0080..=0x0000_0000_0000_007f => I8,
  2019-08-30T00:00:56.8986705Z     |             ---------------------------------------------- this range overlaps on `-128i128..=127i128`
  2019-08-30T00:00:56.8987629Z 494 |             -0x0000_0000_0000_8000..=0x0000_0000_0000_7fff => I16,
  2019-08-30T00:00:56.8988345Z     |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ overlapping patterns
  2019-08-30T00:00:56.8989197Z     = note: `-D unreachable-patterns` implied by `-D warnings`
  2019-08-30T00:00:56.8993751Z 
  2019-08-30T00:00:56.9038160Z error: multiple patterns covering the same range
  2019-08-30T00:00:56.9038720Z    --> src/librustc_target/abi/mod.rs:495:13
  2019-08-30T00:00:56.9038720Z    --> src/librustc_target/abi/mod.rs:495:13
  2019-08-30T00:00:56.9039143Z     |
  2019-08-30T00:00:56.9039771Z 493 |             -0x0000_0000_0000_0080..=0x0000_0000_0000_007f => I8,
  2019-08-30T00:00:56.9040370Z     |             ---------------------------------------------- this range overlaps on `-128i128..=127i128`
  2019-08-30T00:00:56.9041465Z 494 |             -0x0000_0000_0000_8000..=0x0000_0000_0000_7fff => I16,
  2019-08-30T00:00:56.9042043Z     |             ---------------------------------------------- this range overlaps on `-32768i128..=32767i128`
  2019-08-30T00:00:56.9042972Z 495 |             -0x0000_0000_8000_0000..=0x0000_0000_7fff_ffff => I32,
  2019-08-30T00:00:56.9043460Z     |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ overlapping patterns
  2019-08-30T00:00:56.9118088Z error: multiple patterns covering the same range
  2019-08-30T00:00:56.9118386Z    --> src/librustc_target/abi/mod.rs:496:13
  2019-08-30T00:00:56.9118603Z     |
  2019-08-30T00:00:56.9118603Z     |
  2019-08-30T00:00:56.9118937Z 493 |             -0x0000_0000_0000_0080..=0x0000_0000_0000_007f => I8,
  2019-08-30T00:00:56.9119300Z     |             ---------------------------------------------- this range overlaps on `-128i128..=127i128`
  2019-08-30T00:00:56.9119641Z 494 |             -0x0000_0000_0000_8000..=0x0000_0000_0000_7fff => I16,
  2019-08-30T00:00:56.9120177Z     |             ---------------------------------------------- this range overlaps on `-32768i128..=32767i128`
  2019-08-30T00:00:56.9120647Z 495 |             -0x0000_0000_8000_0000..=0x0000_0000_7fff_ffff => I32,
  2019-08-30T00:00:56.9121083Z     |             ---------------------------------------------- this range overlaps on `-2147483648i128..=2147483647i128`
  2019-08-30T00:00:56.9121385Z 496 |             -0x8000_0000_0000_0000..=0x7fff_ffff_ffff_ffff => I64,
  2019-08-30T00:00:56.9121834Z     |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ overlapping patterns
  2019-08-30T00:00:56.9167451Z error: multiple patterns covering the same range
  2019-08-30T00:00:56.9167785Z    --> src/librustc_target/abi/mod.rs:505:13
  2019-08-30T00:00:56.9168028Z     |
  2019-08-30T00:00:56.9168028Z     |
  2019-08-30T00:00:56.9168323Z 504 |             0..=0x0000_0000_0000_00ff => I8,
  2019-08-30T00:00:56.9169093Z     |             ------------------------- this range overlaps on `0u128..=255u128`
  2019-08-30T00:00:56.9169393Z 505 |             0..=0x0000_0000_0000_ffff => I16,
  2019-08-30T00:00:56.9169904Z     |             ^^^^^^^^^^^^^^^^^^^^^^^^^ overlapping patterns
  2019-08-30T00:00:56.9227068Z error: multiple patterns covering the same range
  2019-08-30T00:00:56.9227399Z    --> src/librustc_target/abi/mod.rs:506:13
  2019-08-30T00:00:56.9227670Z     |
  2019-08-30T00:00:56.9227670Z     |
  2019-08-30T00:00:56.9227990Z 504 |             0..=0x0000_0000_0000_00ff => I8,
  2019-08-30T00:00:56.9228377Z     |             ------------------------- this range overlaps on `0u128..=255u128`
  2019-08-30T00:00:56.9228672Z 505 |             0..=0x0000_0000_0000_ffff => I16,
  2019-08-30T00:00:56.9229243Z     |             ------------------------- this range overlaps on `0u128..=65535u128`
  2019-08-30T00:00:56.9229827Z 506 |             0..=0x0000_0000_ffff_ffff => I32,
  2019-08-30T00:00:56.9230170Z     |             ^^^^^^^^^^^^^^^^^^^^^^^^^ overlapping patterns
  2019-08-30T00:00:56.9261946Z error: multiple patterns covering the same range
  2019-08-30T00:00:56.9262273Z    --> src/librustc_target/abi/mod.rs:507:13
  2019-08-30T00:00:56.9262814Z     |
  2019-08-30T00:00:56.9262814Z     |
  2019-08-30T00:00:56.9263142Z 504 |             0..=0x0000_0000_0000_00ff => I8,
  2019-08-30T00:00:56.9263509Z     |             ------------------------- this range overlaps on `0u128..=255u128`
  2019-08-30T00:00:56.9263856Z 505 |             0..=0x0000_0000_0000_ffff => I16,
  2019-08-30T00:00:56.9264226Z     |             ------------------------- this range overlaps on `0u128..=65535u128`
  2019-08-30T00:00:56.9264709Z 506 |             0..=0x0000_0000_ffff_ffff => I32,
  2019-08-30T00:00:56.9265162Z     |             ------------------------- this range overlaps on `0u128..=4294967295u128`
  2019-08-30T00:00:56.9265467Z 507 |             0..=0xffff_ffff_ffff_ffff => I64,
  2019-08-30T00:00:56.9265827Z     |             ^^^^^^^^^^^^^^^^^^^^^^^^^ overlapping patterns
  2019-08-30T00:00:58.4044166Z error: aborting due to 6 previous errors
  2019-08-30T00:00:58.4049163Z 
  2019-08-30T00:00:58.4526332Z error: Could not compile `rustc_target`.
  2019-08-30T00:00:58.4545539Z warning: build failed, waiting for other jobs to finish...
  ---
  2019-08-30T00:01:00.2008025Z == clock drift check ==
  2019-08-30T00:01:00.2025703Z   local time: Fri Aug 30 00:01:00 UTC 2019
  2019-08-30T00:01:00.2905893Z   network time: Fri, 30 Aug 2019 00:01:00 GMT
  2019-08-30T00:01:00.2910698Z == end clock drift check ==
  2019-08-30T00:01:01.6587570Z ##[error]Bash exited with code '1'.
  2019-08-30T00:01:01.6630166Z ##[section]Starting: Checkout
  2019-08-30T00:01:01.6631826Z ==============================================================================
  2019-08-30T00:01:01.6631877Z Task         : Get sources
  2019-08-30T00:01:01.6631927Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
  ```
  
  </details><p></p>
  
  [I'm a bot](https://github.com/rust-ops/rust-log-analyzer)! 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](https://github.com/rust-ops/rust-log-analyzer/issues?q=is%3Aopen+is%3Aissue+label%3Afeature-request))
events.type: comment
events.action: created
events.author: joshtriplett
events.masked_author: username_3
events.text: Conclusions from the lang team meeting:
  
  - We think there are legitimate use cases for some kinds of overlaps. For instance, matching a couple of specific letters and then matching a whole range that covers those letters as a fallback.
  - Thus, we think this would be prone to false positives.
  - We think some heuristics might help with that. For instance, overlapping ranges that don't *completely* overlap (0..10 and 5..15) seem more reasonable to warn about, rather than 5..10 and 0..20.
events.type: comment
events.action: created
events.author: scottmcm
events.masked_author: username_4
events.text: I feel strongly that this shouldn't be `unreachable_patterns` because the arm is reachable.
  
  My interpretation here is that this needs to be heuristic-heavy, so it being in clippy sounds fine.  But if there's some obvious simple heuristic that's essentially always correct I have no objection to having it in the compiler.  (Maybe something like "the fix is just to change a literal to a different literal".)
events.type: comment
events.action: created
events.author: estebank
events.masked_author: username_0
events.text: I could restrict this new lint to single element overlap (`0..=10`/`10..=20`) and disjointed partial overlaps (`0..10`/`5..15`) and ignore the common desired case (`100..102`/`0..=255`). That would cover the cases I was worried about to begin with. Is this reasonable? I don't mind dropping this and relying on clippy instead :)
events.type: comment
events.action: created
events.author: Centril
events.masked_author: username_5
events.text: I think going for the single element overlap (just for ranges) is something I'd be OK with in rustc.
events.type: comment
events.action: created
events.author: bors
events.masked_author: username_6
events.text: :umbrella: The latest upstream changes (presumably #64271) made this pull request unmergeable. Please resolve the merge conflicts.
events.type: comment
events.action: created
events.author: hdhoang
events.masked_author: username_7
events.text: ping from triage @username_0, any update after the reviewers' comment?
events.type: comment
events.action: created
events.author: JohnCSimon
events.masked_author: username_8
events.text: Pinging again from triage. 
  @username_0  Can you please address the merge conflicts. 
  Thanks!
events.type: comment
events.action: created
events.author: JohnCSimon
events.masked_author: username_8
events.text: Hello from triage:
  @username_0 -
  This PR has sat idle for awhile, so I'm closing it as inactive, 
  Please reopen this when you have time to resolve the merge conflicts. 
  
  Thank you!
events.type: comment
events.action: created
events.author: estebank
events.masked_author: username_0
events.text: Rebased and changed logic to only be emitted in the following cases:
  
  ```
  1 ---------
  2     ----------
  
  1     ---------
  2  ----------
  
  1  ----
  2     ----
  
  1     ----
  2 -----
  ```
events.type: comment
events.action: created
events.author: Centril
events.masked_author: username_5
events.text: But that's not a single element overlap?
events.type: comment
events.action: created
events.author: estebank
events.masked_author: username_0
events.text: But I can further restrict it of course.
events.type: comment
events.action: created
events.author: Centril
events.masked_author: username_5
events.text: Yes, let's start conservative for now with the consensus that is expressed in the PR discussion and we can consider expanding the scope later on.
events.type: comment
events.action: created
events.author: estebank
events.masked_author: username_0
events.text: @username_5 [done](https://github.com/rust-lang/rust/pull/64007/commits/593cdcccf28361df155c37916dcfcbe1bf19d9a5) and [done](https://github.com/rust-lang/rust/issues/65477).
events.type: comment
events.action: created
events.author: matthewjasper
events.masked_author: username_9
events.text: @username_6 r+
events.type: comment
events.action: created
events.author: bors
events.masked_author: username_6
events.text: :pushpin: Commit 593cdcccf28361df155c37916dcfcbe1bf19d9a5 has been approved by `username_9`
  
  <!-- @username_6 r=username_9 593cdcccf28361df155c37916dcfcbe1bf19d9a5 -->
  <!-- homu: {"type":"Approved","sha":"593cdcccf28361df155c37916dcfcbe1bf19d9a5","approver":"username_9"} -->
events.type: comment