Skip to content

Conversation

@Jarcho
Copy link
Contributor

@Jarcho Jarcho commented Nov 4, 2025

When lowering for loops, the spans for the into_iter call and the Some pattern used the span of the provided pattern and head expression. If either of those came from a different SyntaxContext this would result in some very strange contexts. e.g.:

macro_rules! m { ($e:expr) => { { $e } } }
for _ in m!(expr) {}

This would result in the into_iter call have a context chain of desugar => m!() => root which is completely nonsensical; m!() does not have a for loop. The into_iter call also ends up located at { $e } rather than inside the for _ in _ part.

This fixes that by walking the spans up to the for loop's context first. This will not handle adjusting the location of macro variable expansions (e.g. for _ in $e), but this does adjust the context to match the for loops.


This ended up causing rust-lang/rust-clippy#16008. Clippy should be using a debug_assert rather than unreachable, but it still results in a bug either way.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 4, 2025
@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Nov 4, 2025
@rustbot
Copy link
Collaborator

rustbot commented Nov 4, 2025

r? @davidtwco

rustbot has assigned @davidtwco.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Comment on lines +1778 to +1779
let head_span =
head.span.find_ancestor_in_same_ctxt(e.span).unwrap_or(head.span).with_ctxt(for_ctxt);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The currently failing tests/ui/suggest-dereferences/invalid-suggest-deref-issue127590 can be fixed by making a second mark rather than reusing for_ctxt. I'm not exactly clear why this matters since the only change this makes is using two identical, but separate SyntaxContexts.

Ideally only a single SyntaxContext would be created since there's only one for loop.

Copy link
Contributor Author

@Jarcho Jarcho Nov 5, 2025

Choose a reason for hiding this comment

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

Narrowed this down to being different expansion call sites. If we replace the for loop with a call to into_iter then it always produces three diagnostics.

Errors
error[E0277]: `&std::slice::Iter<'_, i32>` is not an iterator
  --> src/main.rs:2:59
   |
 2 |     IntoIterator::into_iter(std::iter::zip([1i32].iter(), &[1i32].iter()));
   |                             --------------                ^^^^^^^^^^^^^^ `&std::slice::Iter<'_, i32>` is not an iterator
   |                             |
   |                             required by a bound introduced by this call
   |
   = help: the trait `Iterator` is not implemented for `&std::slice::Iter<'_, i32>`
   = note: required for `&std::slice::Iter<'_, i32>` to implement `IntoIterator`
note: required by a bound in `std::iter::zip`
  --> /playground/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/iter/adapters/zip.rs:70:8
   |
67 | pub fn zip<A, B>(a: A, b: B) -> Zip<A::IntoIter, B::IntoIter>
   |        --- required by a bound in this function
...
70 |     B: IntoIterator,
   |        ^^^^^^^^^^^^ required by this bound in `zip`
help: consider removing the leading `&`-reference
   |
 2 -     IntoIterator::into_iter(std::iter::zip([1i32].iter(), &[1i32].iter()));
 2 +     IntoIterator::into_iter(std::iter::zip([1i32].iter(), [1i32].iter()));
   |
help: consider changing this borrow's mutability
   |
 2 |     IntoIterator::into_iter(std::iter::zip([1i32].iter(), &mut [1i32].iter()));
   |                                                            +++

error[E0277]: `&std::slice::Iter<'_, i32>` is not an iterator
 --> src/main.rs:2:29
  |
2 |     IntoIterator::into_iter(std::iter::zip([1i32].iter(), &[1i32].iter()));
  |     ----------------------- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `&std::slice::Iter<'_, i32>` is not an iterator
  |     |
  |     required by a bound introduced by this call
  |
  = help: the trait `Iterator` is not implemented for `&std::slice::Iter<'_, i32>`
  = help: the trait `Iterator` is implemented for `std::slice::Iter<'_, T>`
  = note: `Iterator` is implemented for `&mut std::slice::Iter<'_, i32>`, but not for `&std::slice::Iter<'_, i32>`
  = note: required for `Zip<std::slice::Iter<'_, i32>, &std::slice::Iter<'_, i32>>` to implement `Iterator`
  = note: required for `Zip<std::slice::Iter<'_, i32>, &std::slice::Iter<'_, i32>>` to implement `IntoIterator`

error[E0277]: `&std::slice::Iter<'_, i32>` is not an iterator
 --> src/main.rs:2:5
  |
2 |     IntoIterator::into_iter(std::iter::zip([1i32].iter(), &[1i32].iter()));
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `&std::slice::Iter<'_, i32>` is not an iterator
  |
  = help: the trait `Iterator` is not implemented for `&std::slice::Iter<'_, i32>`
  = help: the trait `Iterator` is implemented for `std::slice::Iter<'_, T>`
  = note: `Iterator` is implemented for `&mut std::slice::Iter<'_, i32>`, but not for `&std::slice::Iter<'_, i32>`
  = note: required for `Zip<std::slice::Iter<'_, i32>, &std::slice::Iter<'_, i32>>` to implement `Iterator`
  = note: required for `Zip<std::slice::Iter<'_, i32>, &std::slice::Iter<'_, i32>>` to implement `IntoIterator`

The third error is filtered on for loops since when deduplicating obligation failures if the cause span is in a desugaring then the call site span is used instead. Originally the call site span for the into_iter call is the head expression which causes it to be suppressed since this matches the second error. With this PR the call site span is the for loop itself which doesn't match the previous two errors.

If this is actually a problem I can mark the head expression separately. I don't see this as too big of a deal since the second error will be present in either case and it's just as useless as the third one.

@rust-log-analyzer
Copy link
Collaborator

The job aarch64-gnu-llvm-20-1 failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)
failures:

---- [ui] tests/ui/traits/suggest-dereferences/invalid-suggest-deref-issue-127590.rs stdout ----

error: 2 diagnostics reported in JSON output but not expected in test file
tests/ui/traits/suggest-dereferences/invalid-suggest-deref-issue-127590.rs:6:24: ERROR: `&std::slice::Iter<'_, {integer}>` is not an iterator [E0277]
tests/ui/traits/suggest-dereferences/invalid-suggest-deref-issue-127590.rs:13:24: ERROR: `&std::slice::Iter<'_, {integer}>` is not an iterator [E0277]

thread '[ui] tests/ui/traits/suggest-dereferences/invalid-suggest-deref-issue-127590.rs' panicked at src/tools/compiletest/src/runtest.rs:922:13:
errors differ from expected
status: exit status: 1
command: env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/traits/suggest-dereferences/invalid-suggest-deref-issue-127590.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=/checkout/vendor" "--sysroot" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2" "--target=aarch64-unknown-linux-gnu" "--check-cfg" "cfg(test,FALSE)" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/aarch64-unknown-linux-gnu/test/ui/traits/suggest-dereferences/invalid-suggest-deref-issue-127590" "-A" "unused" "-W" "unused_attributes" "-A" "internal_features" "-A" "unused_parens" "-A" "unused_braces" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/aarch64-unknown-linux-gnu/native/rust-test-helpers"

stack backtrace:
   5: __rustc::rust_begin_unwind
             at /rustc/3b4dd9bf1410f8da6329baa36ce5e37673cbbd1f/library/std/src/panicking.rs:698:5
   6: core::panicking::panic_fmt

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

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants