Fix rustdoc ICE on macros defined within functions

[Manishearth]

fixes #47639

[rust-highfive]

r? @petrochenkov

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

[Manishearth]

r? @QuietMisdreavus

[kennytm]

[00:05:40] tidy error: /checkout/src/test/rustdoc/issue-47639.rs:11: trailing whitespace

[Manishearth]

fixed

…

-Manish Goregaokar

On Fri, Feb 2, 2018 at 1:31 PM, kennytm ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/test/rustdoc/issue-47639.rs <#47959 (comment)>: > @@ -0,0 +1,16 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or +// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license +// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// This should not ICE [00:05:40] tidy error: /checkout/src/test/rustdoc/issue-47639.rs:11: trailing whitespace — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#47959 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABivSAdPD_KY9FC_ZBJaYRJvbqgd_uiRks5tQsDNgaJpZM4R2vx4> .

[QuietMisdreavus]

Seeking a second opinion. Does someone on @rust-lang/compiler have a better grasp of the ramifications of turning off the "buffered-but-not-fired lints are bugs" functionality when running as rustdoc? @jseyfried? Something about "just patching out the line that panics" seems off, but then again the "everybody loops" pass is also a bit strange, so maybe it balances out.

[Manishearth]

The assumption made in that bug! is that all linted nodes must be visited. This is an invariant that holds true in the absence of the everybody-loops pass (and if it were to break it would be problematic), however everybody-loops throws things for a toss by removing nodes wholesale. Proc macros can also remove nodes, but not after they get linted (admittedly a shaky way of holding up the invariant, but hey, that's why we have the bug!)

A more robust solution would be to note down the macros removed by everybody-loops but imo that's a bit like overkill.

[Mark-Simulacrum]

Could we instead gate this check on whether everybody loops pass was run? Not sure whether that's better necessarily, but it seems at least like a minor improvement.

[Manishearth]

Where should I stash that fact? On the session? On the context? One trick that could be done is to empty that buffer immediately after running the pass, or turn off the unused macro built-in for rustdoc.

…

On Feb 2, 2018 11:59 PM, "Mark Simulacrum" ***@***.***> wrote: Could we instead gate this check on whether everybody loops pass was run? Not sure whether that's *better* necessarily, but it seems at least like a minor improvement. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#47959 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABivSO07T0VL1qvEYSTiaWRBeRQ2LUkjks5tQ1GugaJpZM4R2vx4> .

[Mark-Simulacrum]

Hm. Yeah, let's not change anything then unless the compiler team decides on something. Is there a chance you could prepare a PR to backport this (minimal) fix to beta? It seems like the smallest change we can make that should fix this regression and I'd like to get it on beta ASAP.

[Manishearth]

done: #47969

won't be around for a while, feel free to force push to that branch or whatever to get it landed

[eddyb]

actually_rustdoc is pre-existing?!

[eddyb]

Oh this is not CLI-facing, just an internal flag used for literally causing the problem here.

[Manishearth]

Anyway, r?

We've landed the beta one so this isn't urgent anymore but nightly is still broken.

[Mark-Simulacrum]

I think we should land this and perhaps iterate later if necessary. If anyone objects, feel free to r-

@bors r+

[bors]

📌 Commit ee737c7 has been approved by Mark-Simulacrum