-
Notifications
You must be signed in to change notification settings - Fork 898
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enabling struct_field_align_threshold=20
broke struct
#4928
Comments
Could you please provide your original snippet that can be used for reproduction, as well as the version of rustfmt you are using? We have issue templates to help structure the input but looks like we accidentally dropped those in some branch shuffling |
In terms of version, just before posting these issues I'd switched from my distro's rustc to using rustup, so I had the latest stable and latest nightly. I was using the The project I was playing with applying rustfmt to is https://github.com/jnqnfe/pulse-binding-rust The specific snippet in it's original form is: /// Playback and record buffer metrics.
#[repr(C)]
#[derive(Debug, Default, Copy, Clone, PartialEq, Eq)]
pub struct BufferAttr {
/* NOTE: This struct must be directly usable by the C API, thus same attributes/layout/etc */
/// Maximum length of the buffer in bytes.
///
/// Setting this to [`std::u32::MAX`] will initialize this to the maximum value supported by the
/// server, which is recommended. In strict low-latency playback scenarios you might want to set
/// this to a lower value, likely together with the [`stream::FlagSet::ADJUST_LATENCY`] flag. If
/// you do so, you ensure that the latency doesn’t grow beyond what is acceptable for the use
/// case, at the cost of getting more underruns if the latency is lower than what the server can
/// reliably handle.
///
/// [`stream::FlagSet::ADJUST_LATENCY`]: crate::stream::FlagSet::ADJUST_LATENCY
pub maxlength: u32,
/// Target length of the buffer (playback only). The server tries to assure that at least
/// `tlength` bytes are always available in the per-stream server-side playback buffer. The
/// server will only send requests for more data as long as the buffer has less than this number
/// of bytes of data.
///
/// It is recommended to set this to [`std::u32::MAX`], which will initialize this to a value
/// that is deemed sensible by the server. However, this value will default to something like
/// 2s; for applications that have specific latency requirements this value should be set to
/// the maximum latency that the application can deal with.
///
/// When [`stream::FlagSet::ADJUST_LATENCY`] is not set this value will influence only the
/// per-stream playback buffer size. When [`stream::FlagSet::ADJUST_LATENCY`] is set, the
/// overall latency of the sink plus the playback buffer size is configured to this value. Set
/// [`stream::FlagSet::ADJUST_LATENCY`] if you are interested in adjusting the overall latency.
/// Don’t set it if you are interested in configuring the server-side per-stream playback buffer
/// size.
///
/// [`stream::FlagSet::ADJUST_LATENCY`]: crate::stream::FlagSet::ADJUST_LATENCY
pub tlength: u32,
/// Pre-buffering (playback only). The server does not start with playback before at least
/// `prebuf` bytes are available in the buffer. It is recommended to set this to
/// [`std::u32::MAX`], which will initialize this to the same value as `tlength`, whatever that
/// may be.
///
/// Initialize to `0` to enable manual start/stop control of the stream. This means that
/// playback will not stop on underrun and playback will not start automatically, instead
/// [`Stream::cork()`] needs to be called explicitly. If you set this value to `0` you should
/// also set [`stream::FlagSet::START_CORKED`]. Should underrun occur, the read index of the
/// output buffer overtakes the write index, and hence the fill level of the buffer is negative.
///
/// Start of playback can be forced using [`Stream::trigger()`] even though the prebuffer size
/// hasn’t been reached. If a buffer underrun occurs, this prebuffering will be again enabled.
///
/// [`Stream::cork()`]: crate::stream::Stream::cork
/// [`Stream::trigger()`]: crate::stream::Stream::trigger
/// [`stream::FlagSet::START_CORKED`]: crate::stream::FlagSet::START_CORKED
pub prebuf: u32,
/// Minimum request (playback only). The server does not request less than `minreq` bytes from
/// the client, instead it waits until the buffer is free enough to request more bytes at once.
///
/// It is recommended to set this to [`std::u32::MAX`], which will initialize this to a value
/// that is deemed sensible by the server. This should be set to a value that gives PulseAudio
/// enough time to move the data from the per-stream playback buffer into the hardware playback
/// buffer.
pub minreq: u32,
/// Fragment size (recording only). The server sends data in blocks of `fragsize` bytes size.
///
/// Large values diminish interactivity with other operations on the connection context but
/// decrease control overhead. It is recommended to set this to `std::u32::MAX`, which will
/// initialize this to a value that is deemed sensible by the server. However, this value will
/// default to something like 2s; For applications that have specific latency requirements this
/// value should be set to the maximum latency that the application can deal with.
///
/// If [`stream::FlagSet::ADJUST_LATENCY`] is set the overall source latency will be adjusted
/// according to this value. If it is not set the source latency is left unmodified.
///
/// [`stream::FlagSet::ADJUST_LATENCY`]: crate::stream::FlagSet::ADJUST_LATENCY
pub fragsize: u32,
} which can be found in https://github.com/jnqnfe/pulse-binding-rust/blob/master/pulse-binding/src/def.rs |
Please run |
I understand and I appreciate that it would have been most helpful to have specified which nightly version I was actually using in the original reports. However I've updated several times since then so giving you the current version string is problematic unless I go through every report to confirm that the problems all still occur with with this version. Sigh. So, starting with this report, the version string is rustfmt 1.4.37-stable (a178d03 2021-07-26) and yes it still occurs against my master branch of the linked project (if you add the quoted config). |
No worries! You mentioned running If so could you actually share The stable version you shared is precisely the type of thing I was looking for, but just want to make sure we've got the same toolchain you're actually using in the project. For future reference, there's no need to vet against every version or anything like that, we just need a version where it can be reproduced to have that marker on the issue. |
@calebcartwright can this be closed? I believe this was fixed by #5159. |
Indeed. Do you have access to close? |
Yeah, I do. I just wanted to double check that it's okay that I go ahead and close it. |
closing as this has been resolved by #5159. |
With the following config, having just enabled the
struct_field_align_threshold = 20
line, and runningcargo nightly fmt
, rustfmt removed the terminating commas from some structs like the following, breaking them.Trying to compile or re-run rustfmt afterwards obviously causes compile errors.
The text was updated successfully, but these errors were encountered: