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

Remove --cfg dox from rustdoc.rs #77347

Merged
merged 3 commits into from
Oct 3, 2020
Merged

Remove --cfg dox from rustdoc.rs #77347

merged 3 commits into from
Oct 3, 2020

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Sep 29, 2020

This was added in #53076 because
several dependencies were using cfg(dox) instead of cfg(rustdoc) (now cfg(doc)).
I ran rg 'cfg\(dox\)' on the source tree with no matches, so I think
this is now safe to remove.

r? @Mark-Simulacrum
cc @QuietMisdreavus :)

This was added in rust-lang#53076 because
several dependencies were using `cfg(dox)` instead of `cfg(rustdoc)`.
I ran `rg 'cfg\(dox\)'` on the source tree with no matches, so I think
this is now safe to remove.
@jyn514 jyn514 added C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Sep 29, 2020
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 29, 2020
@@ -24,14 +24,10 @@ fn main() {
let mut dylib_path = bootstrap::util::dylib_path();
dylib_path.insert(0, PathBuf::from(libdir.clone()));

//FIXME(misdreavus): once stdsimd uses cfg(doc) instead of cfg(dox), remove the `--cfg dox`
//arguments here
let mut cmd = Command::new(rustdoc);
cmd.args(&args)
.arg("--cfg")
.arg(format!("stage{}", stage))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, this also looks stale, we should really be passing --cfg bootstrap in stage 0 these days, cfg(stage0) shouldn't be used by anything.

Want to update that in this PR as well? (We should match rustc.rs).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like rustc.rs doesn't set it either, I did find it set in compile.rs though:

compile.rs
422-                        .arg("--cfg")
423:                        .arg("bootstrap")

Maybe nothing actually needs --cfg bootstrap for rustdoc? I'll try removing --cfg stage0 and see.

@jyn514
Copy link
Member Author

jyn514 commented Sep 29, 2020

Actually it looks like I was a little overoptimistic, library/stdarch is still using cfg(dox) :(

library/stdarch/crates/core_arch/src/mod.rs
6:#[cfg(any(target_arch = "arm", target_arch = "aarch64", dox))]
17:    #[cfg(any(target_arch = "x86", dox))]
28:    #[cfg(any(target_arch = "x86_64", dox))]
...

I'll make a PR upstream first.

@Mark-Simulacrum
Copy link
Member

I guess this is blocked on a stdarch update?

@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 30, 2020
@jyn514
Copy link
Member Author

jyn514 commented Oct 1, 2020

Yes, that's right. What's the process for that, do I just open a PR updating the submodule?

@jyn514
Copy link
Member Author

jyn514 commented Oct 1, 2020

I included the stdarch update in this PR.

@Mark-Simulacrum
Copy link
Member

r? @Amanieu

I'll let @Amanieu approve the submodule bump.

@jyn514
Copy link
Member Author

jyn514 commented Oct 1, 2020

I also needed this diff to get the doc-tests to pass:

diff --git a/crates/core_arch/src/core_arch_docs.md b/crates/core_arch/src/core_arch_docs.md
index 64860050..3d363ea7 100644
--- a/crates/core_arch/src/core_arch_docs.md
+++ b/crates/core_arch/src/core_arch_docs.md
@@ -210,12 +210,6 @@ using LLVM's auto-vectorization to produce optimized vectorized code for
 AVX2 and also for the default platform.
 
 ```rust
-# #![cfg_attr(not(doc),feature(stdsimd))]
-# #[allow(unused_imports)]
-# #[cfg(not(doc))]
-# #[macro_use(is_x86_feature_detected)]
-# extern crate std_detect;
-
 fn main() {
     let mut dst = [0];
     add_quickly(&[1], &[2], &mut dst);

@Amanieu does that look like a reasonable change, should I make a PR with it?

@Amanieu
Copy link
Member

Amanieu commented Oct 1, 2020

@jyn514 Sounds good.

The primary purpose is to get the fixes from
rust-lang/stdarch#920
and rust-lang/stdarch#922.

The other changes included are
rust-lang/stdarch#917 and
rust-lang/stdarch#919.
@jyn514
Copy link
Member Author

jyn514 commented Oct 1, 2020

This is ready for review.

@Amanieu
Copy link
Member

Amanieu commented Oct 1, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Oct 1, 2020

📌 Commit ca98778 has been approved by Amanieu

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 1, 2020
@jyn514
Copy link
Member Author

jyn514 commented Oct 1, 2020

@bors rollup=iffy

Updates submodules and I'm still not sure if removing --cfg stageN will break things.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Oct 1, 2020
Remove --cfg dox from rustdoc.rs

This was added in rust-lang#53076 because
several dependencies were using `cfg(dox)` instead of `cfg(rustdoc)` (now `cfg(doc)`).
I ran `rg 'cfg\(dox\)'` on the source tree with no matches, so I think
this is now safe to remove.

r? @Mark-Simulacrum
cc @QuietMisdreavus :)
@bors
Copy link
Contributor

bors commented Oct 2, 2020

⌛ Testing commit ca98778 with merge 564586eb2abb8f9e5cd07f82266a87457f4ea1fe...

@bors
Copy link
Contributor

bors commented Oct 2, 2020

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 2, 2020
@jyn514
Copy link
Member Author

jyn514 commented Oct 2, 2020

Looks spurious? cc @rust-lang/cargo

failures:

---- clean::clean_git stdout ----
running `D:\a\rust\rust\build\x86_64-pc-windows-msvc\stage2-tools\x86_64-pc-windows-msvc\release\cargo.exe build`
running `D:\a\rust\rust\build\x86_64-pc-windows-msvc\stage2-tools\x86_64-pc-windows-msvc\release\cargo.exe clean -p dep`
running `D:\a\rust\rust\build\x86_64-pc-windows-msvc\stage2-tools\x86_64-pc-windows-msvc\release\cargo.exe build`
thread 'clean::clean_git' panicked at '
Expected: execs
    but: exited with exit code: 101
--- stdout

--- stderr
   Compiling dep v0.5.0 (file:///D:/a/rust/rust/build/x86_64-pc-windows-msvc/stage2-tools/x86_64-pc-windows-msvc/cit/t478/dep#800dc269)
   Compiling foo v0.0.1 (D:\a\rust\rust\build\x86_64-pc-windows-msvc\stage2-tools\x86_64-pc-windows-msvc\cit\t478\foo)
error: linking with `link.exe` failed: exit code: 0xc0000374
  |

@bors retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 2, 2020
@ehuss
Copy link
Contributor

ehuss commented Oct 2, 2020

I ran the test in a loop for a few hours locally, and didn't run into any problems. 0xc0000374 is STATUS_HEAP_CORRUPTION. I don't have any ideas what could cause that, other than a flaky VM or cosmic ray ☢️. I've never seen this error, but I'll keep an eye out if it happens again.

@bors
Copy link
Contributor

bors commented Oct 3, 2020

⌛ Testing commit ca98778 with merge 6f56fbd...

@bors
Copy link
Contributor

bors commented Oct 3, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: Amanieu
Pushing 6f56fbd to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 3, 2020
@bors bors merged commit 6f56fbd into rust-lang:master Oct 3, 2020
@rustbot rustbot added this to the 1.49.0 milestone Oct 3, 2020
@jyn514 jyn514 deleted the dox branch October 3, 2020 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants