Rustfmt duplicates attributes

[joshlf]

I have a proc macro which defines the #[specialize_ip_addr] macro. Here's an example of its usage:

/// Set the IP address associated with this device.
#[specialize_ip_addr]
pub fn set_ip_addr<D: EventDispatcher, A: IpAddr>(
    ctx: &mut Context<D>,
    device_id: u64,
    addr: A,
    subnet: Subnet<A>,
) {
    #[ipv4addr]
    get_device_state(ctx, device_id).ipv4_addr = Some((addr, subnet));
    #[ipv6addr]
    get_device_state(ctx, device_id).ipv6_addr = Some((addr, subnet));
}

Rustfmt behaves weirdly when it encounters this function - it successfully parses it, but it duplicates the #[ipv6addr] attribute when formatting, resulting in this:

/// Set the IP address associated with this device.
#[specialize_ip_addr]
pub fn set_ip_addr<D: EventDispatcher, A: IpAddr>(
    ctx: &mut Context<D>,
    device_id: u64,
    addr: A,
    subnet: Subnet<A>,
) {
    #[ipv4addr]
    get_device_state(ctx, device_id).ipv4_addr = Some((addr, subnet));
    #[ipv6addr]
    #[ipv6addr]
    get_device_state(ctx, device_id).ipv6_addr = Some((addr, subnet));
}

This is the only function on which it happens, and I have a number of different functions using this proc macro, so it's fairly dependent on details of the way the function is structured, although I'm not sure how. Here are the functions I have where rustfmt does the right thing:

receive_icmp_packet

/// Receive an ICMP message in an IP packet.
#[specialize_ip_addr]
pub fn receive_icmp_packet<D: EventDispatcher, A: IpAddr, B: BufferMut>(
    ctx: &mut Context<D>,
    src_ip: A,
    dst_ip: A,
    mut buffer: B,
) -> bool {
    trace!("receive_icmp_packet({}, {})", src_ip, dst_ip);

    #[ipv4addr]
    {
        // TODO(joshlf): Add ICMP extension trait which has an associated packet
        // type so that we can pull packet parsing into the common part rather
        // than having to do it once for IPv4 and once for IPv6.
        let packet =
            match buffer.parse_with::<_, Icmpv4Packet<_>>(IcmpParseArgs::new(src_ip, dst_ip)) {
                Ok(packet) => packet,
                Err(err) => return false,
            };

        match packet {
            Icmpv4Packet::EchoRequest(echo_request) => {
                let req = *echo_request.message();
                let code = echo_request.code();
                // drop packet so we can re-use the underlying buffer
                mem::drop(echo_request);

                increment_counter!(ctx, "receive_icmp_packet::echo_request");

                // we're responding to the sender, so these are flipped
                let (src_ip, dst_ip) = (dst_ip, src_ip);
                send_ip_packet(ctx, dst_ip, IpProto::Icmp, |src_ip| {
                    BufferSerializer::new_vec(buffer).encapsulate(
                        IcmpPacketBuilder::<Ipv4, &[u8], _>::new(src_ip, dst_ip, code, req.reply()),
                    )
                });
                true
            }
            Icmpv4Packet::EchoReply(echo_reply) => {
                increment_counter!(ctx, "receive_icmp_packet::echo_reply");
                trace!("receive_icmp_packet: Received an EchoReply message");
                true
            }
            _ => log_unimplemented!(
                false,
                "ip::icmp::receive_icmp_packet: Not implemented for this packet type"
            ),
        }
    }

    #[ipv6addr]
    {
        let packet =
            match buffer.parse_with::<_, Icmpv6Packet<_>>(IcmpParseArgs::new(src_ip, dst_ip)) {
                Ok(packet) => packet,
                Err(err) => return false,
            };

        match packet {
            Icmpv6Packet::EchoRequest(echo_request) => {
                let req = *echo_request.message();
                let code = echo_request.code();
                // drop packet so we can re-use the underlying buffer
                mem::drop(echo_request);

                increment_counter!(ctx, "receive_icmp_packet::echo_request");

                // we're responding to the sender, so these are flipped
                let (src_ip, dst_ip) = (dst_ip, src_ip);
                send_ip_packet(ctx, dst_ip, IpProto::Icmp, |src_ip| {
                    BufferSerializer::new_vec(buffer).encapsulate(
                        IcmpPacketBuilder::<Ipv6, &[u8], _>::new(src_ip, dst_ip, code, req.reply()),
                    )
                });
                true
            }
            Icmpv6Packet::EchoReply(echo_reply) => {
                increment_counter!(ctx, "receive_icmp_packet::echo_reply");
                trace!("receive_icmp_packet: Received an EchoReply message");
                true
            }
            _ => log_unimplemented!(
                false,
                "ip::icmp::receive_icmp_packet: Not implemented for this packet type"
            ),
        }
    }
}

deliver

// Should we deliver this packet locally?
// deliver returns true if:
// - dst_ip is equal to the address set on the device
// - dst_ip is equal to the broadcast address of the subnet set on the device
// - dst_ip is equal to the global broadcast address
#[specialize_ip_addr]
fn deliver<D: EventDispatcher, A: IpAddr>(
    ctx: &mut Context<D>,
    device: DeviceId,
    dst_ip: A,
) -> bool {
    // TODO(joshlf):
    // - This implements a strict host model (in which we only accept packets
    //   which are addressed to the device over which they were received). This
    //   is the easiest to implement for the time being, but we should actually
    //   put real thought into what our host model should be (NET-1011).
    #[ipv4addr]
    return crate::device::get_ip_addr::<D, _>(ctx, device)
        .map(|(addr, subnet)| dst_ip == addr || dst_ip == subnet.broadcast())
        .unwrap_or(dst_ip == Ipv4::BROADCAST_ADDRESS);
    #[ipv6addr]
    return log_unimplemented!(false, "ip::deliver: Ipv6 not implemeneted");
}

forward

// Should we forward this packet, and if so, to whom?
#[specialize_ip_addr]
fn forward<D: EventDispatcher, A: IpAddr>(
    ctx: &mut Context<D>,
    dst_ip: A,
) -> Option<Destination<A::Version>> {
    let ip_state = &ctx.state().ip;

    #[ipv4addr]
    let forward = ip_state.v4.forward;
    #[ipv6addr]
    let forward = ip_state.v6.forward;

    if forward {
        lookup_route(ip_state, dst_ip)
    } else {
        None
    }
}

lookup_route

// Look up the route to a host.
#[specialize_ip_addr]
fn lookup_route<A: IpAddr>(state: &IpLayerState, dst_ip: A) -> Option<Destination<A::Version>> {
    #[ipv4addr]
    return state.v4.table.lookup(dst_ip);
    #[ipv6addr]
    return state.v6.table.lookup(dst_ip);
}

add_device_route

/// Add a route to the forwarding table.
#[specialize_ip_addr]
pub fn add_device_route<D: EventDispatcher, A: IpAddr>(
    ctx: &mut Context<D>,
    subnet: Subnet<A>,
    device: DeviceId,
) {
    let state = &mut ctx.state().ip;

    #[ipv4addr]
    state.v4.table.add_device_route(subnet, device);
    #[ipv6addr]
    state.v6.table.add_device_route(subnet, device);
}

get_inner_state

#[specialize_ip_addr]
fn get_inner_state<D: EventDispatcher, A: IpAddr>(
    state: &mut StackState<D>,
) -> &mut UdpStateInner<D, A> {
    #[ipv4addr]
    return &mut state.transport.udp.ipv4;
    #[ipv6addr]
    return &mut state.transport.udp.ipv6;
}

cc @cramertj

[topecongiro]

Thanks for the report!

This seems to be a duplicate of #3313. The fixed version should be available in next 24 hours via rustup component add rustfmt --toolchain nightly.

cc rust-lang/rust#58571.

[joshlf]

Awesome, thanks!