Skip to content
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

[rustdoc] Remove duplicated options in doctests compilation flags saved in argument file #129266

Conversation

GuillaumeGomez
Copy link
Member

When I took a look about what was stored in this file recently, I realized that a lot of options were duplicated. Using a set will prevent that. I used a BTreeSet to keep the original flags order just in case.

r? @notriddle

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Aug 19, 2024
@notriddle
Copy link
Contributor

BTreeSet makes the order deterministic, but it doesn't preserve anything. It just implicitly sorts its contents. That's important to keep in mind, because some CLI arguments override each other, and in that case, the order is significant.

I'd rather try to figure out why the duplicates are being added in the first place, if it's a problem. I'd also like to know if there's enough of a problem to justify the risk associated with this change.

@GuillaumeGomez
Copy link
Member Author

Oh. That's quite a bit misunderstanding on my end. I'll rewrite this PR with a FxHashSet on the side instead then. I'll also check where the duplication might come from.

@notriddle
Copy link
Contributor

notriddle commented Aug 19, 2024

FxIndexSet is the one that preserves insertion order.

But that's still not right, because of this:

$ cat > test.rs
pub fn MyFunc() {}
^D
$ rustc --crate-type=lib test.rs
warning: function `MyFunc` should have a snake case name
 --> test.rs:1:8
  |
1 | pub fn MyFunc() {}
  |        ^^^^^^ help: convert the identifier to snake case: `my_func`
  |
  = note: `#[warn(non_snake_case)]` on by default

warning: 1 warning emitted

$ rustc --crate-type=lib -Anonstandard_style test.rs
$ rustc --crate-type=lib -Anonstandard_style -Wnonstandard_style test.rs
warning: function `MyFunc` should have a snake case name
 --> test.rs:1:8
  |
1 | pub fn MyFunc() {}
  |        ^^^^^^ help: convert the identifier to snake case: `my_func`
  |
  = note: `-W non-snake-case` implied by `-W nonstandard-style`
  = help: to override `-W nonstandard-style` add `#[allow(non_snake_case)]`

warning: 1 warning emitted

$ rustc --crate-type=lib -Anonstandard_style -Wnonstandard_style -Anonstandard_style test.rs

@GuillaumeGomez
Copy link
Member Author

I wonder if it isn't only linked to bootstrap... For std:

--crate-type=bin
--cfg=feature="addr2line"
--cfg=feature="backtrace"
--cfg=feature="miniz_oxide"
--cfg=feature="object"
--cfg=feature="panic_unwind"
--cfg=feature="std_detect_dlsym_getauxval"
--cfg=feature="std_detect_file_io"
--cfg=backtrace_in_libstd
--cfg=reliable_f128
--check-cfg=cfg(docsrs)
--check-cfg=cfg(feature, values("addr2line", "backtrace", "compiler-builtins-c", "compiler-builtins-mangled-names", "compiler-builtins-mem", "compiler-builtins-no-asm", "compiler-builtins-no-f16-f128", "llvm-libunwind", "miniz_oxide", "object", "optimize_for_size", "panic-unwind", "panic_immediate_abort", "panic_unwind", "profiler", "profiler_builtins", "std_detect_dlsym_getauxval", "std_detect_env_override", "std_detect_file_io", "system-llvm-libunwind", "windows_raw_dylib"))
--check-cfg=cfg(bootstrap)
--check-cfg=cfg(target_arch, values("xtensa"))
--check-cfg=cfg(feature, values(any()))
--check-cfg=cfg(netbsd10)
--check-cfg=cfg(restricted_std)
--check-cfg=cfg(backtrace_in_libstd)
--check-cfg=cfg(reliable_f16)
--check-cfg=cfg(reliable_f128)
--check-cfg=cfg(reliable_f16_math)
--check-cfg=cfg(reliable_f128_math)
--check-cfg=cfg(feature,values(any()))
--check-cfg=cfg(bootstrap)
--check-cfg=cfg(bootstrap)
-Ldependency=/home/imperio/rust/rust/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps
-Ldependency=/home/imperio/rust/rust/build/x86_64-unknown-linux-gnu/stage1-std/release/deps
--extern=priv:addr2line=/home/imperio/rust/rust/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/libaddr2line-2f6bc7465759172a.rlib
--extern=alloc=/home/imperio/rust/rust/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/liballoc-9d29cfbd93b3ce8c.rlib
--extern=priv:cfg_if=/home/imperio/rust/rust/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/libcfg_if-08d69ad8f422b865.rlib
--extern=priv:compiler_builtins=/home/imperio/rust/rust/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/libcompiler_builtins-a226f6c8e7a05ede.rlib
--extern=core=/home/imperio/rust/rust/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/libcore-24518d8502db248c.rlib
--extern=priv:hashbrown=/home/imperio/rust/rust/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/libhashbrown-2dd09a2b85ae2d1e.rlib
--extern=libc=/home/imperio/rust/rust/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/liblibc-dd431d6e6bb88a94.rlib
--extern=priv:miniz_oxide=/home/imperio/rust/rust/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/libminiz_oxide-c02a713d05171c73.rlib
--extern=priv:object=/home/imperio/rust/rust/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/libobject-a4b005e8b800d4b1.rlib
--extern=priv:panic_abort=/home/imperio/rust/rust/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/libpanic_abort-e1d044588a0a331d.rlib
--extern=priv:panic_unwind=/home/imperio/rust/rust/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/libpanic_unwind-0b5b4186da3f9eb9.rlib
--extern=priv:rand=/home/imperio/rust/rust/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/librand-ae4ad0f659915047.rlib
--extern=priv:rand_xorshift=/home/imperio/rust/rust/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/librand_xorshift-12d1815c31938ceb.rlib
--extern=priv:rustc_demangle=/home/imperio/rust/rust/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/librustc_demangle-8a163e58437f063d.rlib
--extern=priv:std=/home/imperio/rust/rust/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/libstd-b6b346b5eaa52d7d.so
--extern=priv:std=/home/imperio/rust/rust/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/libstd-b6b346b5eaa52d7d.rlib
--extern=priv:std_detect=/home/imperio/rust/rust/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/libstd_detect-1eb25ba504254871.rlib
--extern=priv:unwind=/home/imperio/rust/rust/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps/libunwind-d9a440f1418a6aeb.rlib
-Ccodegen-units=1
-Cembed-bitcode=no
-Csymbol-mangling-version=legacy
-Zunstable-options
-Zunstable-options
-Zcrate-attr=doc(html_root_url="https://doc.rust-lang.org/nightly/")
-Zcrate-attr=warn(rust_2018_idioms)
-Zunstable-options
-Zforce-unstable-if-unmarked
-Zunstable-options

In comparison for sysinfo:

--crate-type=bin
--cfg=feature="component"
--cfg=feature="default"
--cfg=feature="disk"
--cfg=feature="multithread"
--cfg=feature="network"
--cfg=feature="system"
--cfg=feature="user"
--check-cfg=cfg(docsrs)
--check-cfg=cfg(feature, values("apple-app-store", "apple-sandbox", "c-interface", "component", "debug", "default", "disk", "linux-netdevs", "linux-tmpfs", "multithread", "network", "serde", "system", "unknown-ci", "user", "windows"))
-Ldependency=/home/imperio/rust/sysinfo/target/debug/deps
-Ldependency=/home/imperio/rust/sysinfo/target/debug/deps
--extern=bstr=/home/imperio/rust/sysinfo/target/debug/deps/libbstr-3e074d8f1d701b6b.rlib
--extern=libc=/home/imperio/rust/sysinfo/target/debug/deps/liblibc-7693c5932ee0f044.rlib
--extern=memchr=/home/imperio/rust/sysinfo/target/debug/deps/libmemchr-302139f7e3395110.rlib
--extern=rayon=/home/imperio/rust/sysinfo/target/debug/deps/librayon-479f60daa59ce8d0.rlib
--extern=serde_json=/home/imperio/rust/sysinfo/target/debug/deps/libserde_json-28499da49e5db98d.rlib
--extern=sysinfo=/home/imperio/rust/sysinfo/target/debug/deps/libsysinfo-8df96b17b8e357aa.rlib
--extern=tempfile=/home/imperio/rust/sysinfo/target/debug/deps/libtempfile-96c7bd2272a09215.rlib
-Ccodegen-units=1
-Cembed-bitcode=no

@GuillaumeGomez
Copy link
Member Author

Closing in favor of #129278.

@GuillaumeGomez GuillaumeGomez deleted the rm-doctest-flags-duplication branch August 19, 2024 20:31
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-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants