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

Add compiler docs testing to CI. #46278

Merged
merged 6 commits into from
Jan 1, 2018
Merged

Conversation

MaloJaffre
Copy link
Contributor

@MaloJaffre MaloJaffre commented Nov 26, 2017

Fixes #47025.
I don't know if x86_64-gnu is the right builder for this, but there seems to be time left on Travis.

Remaining problems blocking this PR:

See individual commits for more details.

@rust-highfive
Copy link
Collaborator

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@kennytm kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Nov 26, 2017
@MaloJaffre
Copy link
Contributor Author

MaloJaffre commented Nov 26, 2017

Currently failing (probably #46271): https://travis-ci.org/rust-lang/rust/jobs/307535449#L6794-L6818.
At least that shows that the compiler docs are properly tested with this PR.

@Michael-F-Bryan
Copy link

@MaloJaffre, is this part of an overall fix for #29893?

@MaloJaffre
Copy link
Contributor Author

MaloJaffre commented Nov 28, 2017

@Michael-F-Bryan No, because I haven't read this issue before opening this PR, but it is a step forward, because, in the meantime, we can catch regressions of compiler docs building.

@kennytm kennytm added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 1, 2017
@kennytm
Copy link
Member

kennytm commented Dec 1, 2017

The fix for issue #46271 is PR #46405. Marking as S-blocked until that PR is merged.

@MaloJaffre
Copy link
Contributor Author

MaloJaffre commented Dec 3, 2017

Restarting CI to see if it passes, now that #46405 has been merged.

@MaloJaffre MaloJaffre closed this Dec 3, 2017
@MaloJaffre MaloJaffre reopened this Dec 3, 2017
@kennytm kennytm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Dec 3, 2017
@MaloJaffre
Copy link
Contributor Author

MaloJaffre commented Dec 3, 2017

Now it's a lot of broken links in syntax, syntax_pos and rustc that failed the build!

EDIT: it seems that these are intended to be links to std docs, but they reference their own crates instead.

@carols10cents
Copy link
Member

Friendly ping to keep this on your radar, @MaloJaffre! How's it going?

@MaloJaffre
Copy link
Contributor Author

@carols10cents I'm currently trying to understand why the links are broken, but it's not very successful...

@steveklabnik
Copy link
Member

It depends per link; some of these point to a README.md, for example, which isn't actually rendered. Those links should be removed. Some of them might be broken due to re-exporting things, that is, if a link is put into something that's re-exported, it often breaks the link in one place since you can't write two different links for the same thing. You have to take it on a case-by-case basis.

@MaloJaffre
Copy link
Contributor Author

MaloJaffre commented Dec 13, 2017

After further investigation, this PR is blocked by: (EDIT: see PR description)
- rust-random/rand#207 and release of rand 0.3.19
- issue #32129 (syntax::symbol::InternedString derefs to &str)

Meanwhile, I will fix the other broken links that are really in compiler docs.

@kennytm kennytm added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Dec 13, 2017
@steveklabnik
Copy link
Member

The solution to the second one, for now, is just to remove the link.

@MaloJaffre
Copy link
Contributor Author

MaloJaffre commented Dec 15, 2017

@steveklabnik Did you mean to add doc(hidden) to the Deref impl for InternedString and others? I'm not sure it's a good idea to remove the links in &str docs which are broken when items implement Deref<Target=str>.

@MaloJaffre MaloJaffre changed the title [WIP] Add compiler docs testing to CI. Add compiler docs testing to CI. Dec 31, 2017
@kennytm kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Dec 31, 2017
@@ -158,7 +158,7 @@ impl DepGraph {
/// what state they have access to. In particular, we want to
/// prevent implicit 'leaks' of tracked state into the task (which
/// could then be read without generating correct edges in the
/// dep-graph -- see the [README] for more details on the
/// dep-graph -- see the module-level README for more details on the
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps link the README to https://github.com/rust-lang/rust/blob/master/src/librustc/dep_graph/README.md instead of removing the link?

(Alternatively, make use of #![doc(include="README.md")] in mod.rs to display the README directly in the rustdoc. This could be another PR once we get the compiler docs into the CI.)

/// fn baz<'a, 'b>() -> impl Trait<'a, 'b> { ... }
/// ```
/// fn baz<'a, 'b>() -> impl Trait<'a, 'b> { ... }
/// ```
Copy link
Member

Choose a reason for hiding this comment

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

This block should be ```text?

/// ```
/// let a = f().g( 'b: { let x = d(); let y = d(); x.h(y) } ) ;
/// ```
///
/// ```text
Copy link
Member

Choose a reason for hiding this comment

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

Better merge this block with the one above and make it ```text.

@@ -588,7 +588,7 @@ pub type CrateConfig = HirVec<P<MetaItem>>;
/// The top-level data structure that stores the entire contents of
/// the crate currently being compiled.
///
/// For more details, see [the module-level README](README.md).
/// For more details, see the module-level README.
Copy link
Member

Choose a reason for hiding this comment

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

@@ -8,7 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

//! MIR datatypes and passes. See [the README](README.md) for details.
//! MIR datatypes and passes. See the module-level README for details.
Copy link
Member

Choose a reason for hiding this comment

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

/// various **compiler queries** that have been performed. See [the
/// README](README.md) for more deatils.
/// various **compiler queries** that have been performed. See the
/// the module-level README for more details.
Copy link
Member

Choose a reason for hiding this comment

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

@@ -149,7 +149,7 @@
//! The binary of a crate will not only contain machine code for the items
//! defined in the source code of that crate. It will also contain monomorphic
//! instantiations of any extern generic functions and of functions marked with
//! #[inline].
//! \#[inline].
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps `…` it instead of \ it.

@@ -94,7 +94,7 @@
//! inlined, so it can distribute function instantiations accordingly. Since
//! there is no way of knowing for sure which functions LLVM will decide to
//! inline in the end, we apply a heuristic here: Only functions marked with
//! #[inline] are considered for inlining by the partitioner. The current
//! \#[inline] are considered for inlining by the partitioner. The current
Copy link
Member

Choose a reason for hiding this comment

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

Ditto

pub declared_stable_lang_features: Vec<(Symbol, Span)>,
/// #![feature] attrs for non-language (library) features
/// \#![feature] attrs for non-language (library) features
Copy link
Member

Choose a reason for hiding this comment

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

Ditto

/// will be unified.
///
/// [upvar inference]: src/librustc_typeck/check/upvar.rs
/// inferred. Once upvar inference (in src/librustc_typeck/check/upvar.rs)
Copy link
Member

Choose a reason for hiding this comment

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

Please backquote the path.

@MaloJaffre
Copy link
Contributor Author

MaloJaffre commented Dec 31, 2017

@kennytm Thanks for your review!
I've fixed everything according to your remarks, except for the list formatting (I wasn't sure exactly how to format this part).

EDIT: After checking formatting of other lists in API docs, I applied your suggested formatting.

@bors
Copy link
Contributor

bors commented Jan 1, 2018

☔ The latest upstream changes (presumably #47052) made this pull request unmergeable. Please resolve the merge conflicts.

It tested rust-lang#44953.
`log` macros in newer versions are no longer recursive, so these duplicated
error messages (about unstable feature uses) previously occurring at each
level of recursion are no longer possible, even with the fix by rust-lang#45540.
Furthermore this test breaks when multiple versions of `log` are in the
sysroot (`log 0.3.9` depends on`log 0.4.1`)
Update `rand` crate to `0.3.19`.
Update `log` crate to `0.3.9` and `0.4.1`.
Update `parking_lot_core` crate to `0.2.9`.
Upgrade all flate2 dependencies to `1.0.1`.
- Update `rust-installer` submodule.
@kennytm
Copy link
Member

kennytm commented Jan 1, 2018

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Jan 1, 2018

📌 Commit 2449230 has been approved by kennytm

bors added a commit that referenced this pull request Jan 1, 2018
Add compiler docs testing to CI.

Fixes #47025.
I don't know if `x86_64-gnu` is the right builder for this, but there seems to be time left on [Travis](https://travis-ci.org/rust-lang/rust/jobs/307488864).

Remaining problems blocking this PR:
- [x] broken links caused by rustdoc issues:
  - [x] `pub use self::Enum::...`: #46766 and #46767 (fixed by #47050, thanks @ollie27!)
  - [x] `impl Deref for DerefToStdType`: #32129 (ignored in linkchecker)
  - [x] `#[feature(decl_macro)]` and `use std::vec`: #47038 (ignored in linkchecker)
  - [x]  `rustc_data_structures::sync::{Lrc, RwLock}` aliases `std` types: #32130 (ignored in linkchecker)
- [x] markdown differences, in rust repository and in external crates, now failing the build with #46880 merged (all fixed)
- [x] multiple crate updates needed: `rand`, `log`, `parking_lot_core`, `flate2`
  - [x] submodule updates needed to deduplicate dependencies: `rust-installer`, ~`cargo`~ (done by #47052)
  - [x] #44953 test broken by `log` update (removed, this can be controversial)
- [x] Waiting `x86_64-gnu` build results ([done](https://travis-ci.org/rust-lang/rust/builds/323451069))

See individual commits for more details.
@bors
Copy link
Contributor

bors commented Jan 1, 2018

⌛ Testing commit 2449230 with merge 5deba22...

@bors
Copy link
Contributor

bors commented Jan 1, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: kennytm
Pushing 5deba22 to master...

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test internal docs on CI
10 participants