Set `ctx.inside_macro` based on `FmtVisitor.parent_context` in `FmtVisitor::get_context`

[ytmimi]

Fixes #5526

When creating new contexts with FmtVisitor::get_context we set the value of ctx.inside_macro based on the parent_context if it exists. This helps us remember that we're inside of a macro when creating new RewriteContext from recursively created FmtVisitors.

A similar issue to #5526 was reported in #3139, and remembering that we're inside of a macro in this way helps us go down a code path that resolved #3139 and was implemented in PR #3142.

[calebcartwright]

Could you explain why an existing test had to be modified? Haven't looked at it (nor the input/source file) yet, but that's often an indicator of an issue

[ytmimi]

Could you explain why an existing test had to be modified? Haven't looked at it (nor the input/source file) yet, but that's often an indicator of an issue

Here's a cleaned up version of the source snippet for context. Note that we're inside a macro and handle.written() does not have a trailing comma.

    euc_jp_decoder_functions!({
        let trail_minus_offset = byte.wrapping_sub(0xA1);
        // Fast-track Hiragana (60% according to Lunde)
        // and Katakana (10% according to Lunde).
        if jis0208_lead_minus_offset == 0x03 && trail_minus_offset < 0x53 {
            // Hiragana
            handle.write_upper_bmp(0x3041 + trail_minus_offset as u16)
        } else if jis0208_lead_minus_offset == 0x04 && trail_minus_offset < 0x56 {
            // Katakana
            handle.write_upper_bmp(0x30A1 + trail_minus_offset as u16)
        } else if trail_minus_offset > (0xFE - 0xA1) {
            if byte < 0x80 {
                return (
                    DecoderResult::Malformed(1, 0),
                    unread_handle_trail.unread(),
                    handle.written()
                );
            }
            return (
                DecoderResult::Malformed(2, 0),
                unread_handle_trail.consumed(),
                handle.written()
            );
        } else {
            unreachable!();
        }
    });

The target test would then add a trailing comma at the end of each handle.written() call. But since we're in the context of a macro I don't think we should be adding tokens. After the change I made to pass along the parent_context's value for inside_macro we no longer add a trailing comma, so I adjusted the test.

[calebcartwright]

Thanks for the additional detail. Could you take a look at the results from the diff check run https://github.com/rust-lang/rustfmt/runs/8171301629?check_suite_focus=true#step:4:324 and summarize when you get a chance?

Even if this is a change that should be made, is possible we could need to version gate to avoid introducing breaking formatting changes (though if a sufficiently strong argument could be made that this is a bug fix then I'd consider doing so without a gate)

[ytmimi]

There were three repos that reported diffs: rust-lang/rust, Rocket, and hyper.

The rust-lang/rust diff was just reiterating the change I made to the system test in this PR. The Rocket error was similar. Here's a modified code snippet from the Rocket diff:

fn test() {
    assert_eq!(multi, Multi {
        checks: vec![true, false, false],
        names: vec!["Sam".into(), "Smith".into(), "Bob".into()],
        news: vec!["Here".into(), "also here".into()],
        dogs: {
            let mut map = HashMap::new();
            map.insert("fido".into(), Dog { barks: true, trained: true });
            map.insert("George".into(), Dog { barks: false, trained: true });
            map.insert("bob boo".into(), Dog { barks: false, trained: false });
            map
        },
        more_dogs: {
            let mut map = HashMap::new();
            map.insert("My Dog".into(), Dog { barks: true, trained: true });
            map
        }
    });
}

the current master wants to add a trailing comma when formatting Dog { barks: false, trained: true } over multiple lines, however the current PR does not add the trailing comma.

The hyper diff seems more involved and I'll need to do more investigating. For now, here are the code snippets that get poorly formatted by this PR:

#[bench]
fn throughput_fixedsize_many_chunks(b: &mut test::Bencher) {
    bench_server!(b, ("content-length", "1000000"), move || {
        static S: &[&[u8]] = &[&[b'x'; 1_000] as &[u8]; 1_000] as _;
        BodyExt::boxed(StreamBody::new(
            stream::iter(S.iter()).map(|&s| Ok::<_, String>(s)),
        ))
    })
}

#[bench]
fn throughput_chunked_many_chunks(b: &mut test::Bencher) {
    bench_server!(b, ("transfer-encoding", "chunked"), || {
        static S: &[&[u8]] = &[&[b'x'; 1_000] as &[u8]; 1_000] as _;
        BodyExt::boxed(StreamBody::new(
            stream::iter(S.iter()).map(|&s| Ok::<_, String>(s)),
        ))
    })
}

And here's how those snippets are formatted:

#[bench]
fn throughput_fixedsize_many_chunks(b: &mut test::Bencher) {
    bench_server!(b, ("content-length", "1000000"), move || {
        static S: &[&[u8]] = &[&[b'x'; 1_000] as &[u8]; 1_000] as _;
        BodyExt::boxed(StreamBody::new(stream::iter(S.iter()).map(|&s| Ok::<
            _,
            String,
        >(
            s
        )),))
    })
}

#[bench]
fn throughput_chunked_many_chunks(b: &mut test::Bencher) {
    bench_server!(b, ("transfer-encoding", "chunked"), || {
        static S: &[&[u8]] = &[&[b'x'; 1_000] as &[u8]; 1_000] as _;
        BodyExt::boxed(StreamBody::new(stream::iter(S.iter()).map(|&s| Ok::<
            _,
            String,
        >(
            s
        )),))
    })
}