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

Cosmetic improvements to doc comments #58341

Merged
merged 5 commits into from
Feb 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
10 changes: 5 additions & 5 deletions src/bootstrap/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,17 +60,17 @@ pub trait Step: 'static + Clone + Debug + PartialEq + Eq + Hash {
/// Run this rule for all hosts without cross compiling.
const ONLY_HOSTS: bool = false;

/// Primary function to execute this rule. Can call `builder.ensure(...)`
/// Primary function to execute this rule. Can call `builder.ensure()`
/// with other steps to run those.
fn run(self, builder: &Builder) -> Self::Output;

/// When bootstrap is passed a set of paths, this controls whether this rule
/// will execute. However, it does not get called in a "default" context
/// when we are not passed any paths; in that case, make_run is called
/// when we are not passed any paths; in that case, `make_run` is called
/// directly.
fn should_run(run: ShouldRun) -> ShouldRun;

/// Build up a "root" rule, either as a default rule or from a path passed
/// Builds up a "root" rule, either as a default rule or from a path passed
/// to us.
///
/// When path is `None`, we are executing in a context where no paths were
Expand Down Expand Up @@ -648,7 +648,7 @@ impl<'a> Builder<'a> {
add_lib_path(vec![self.rustc_libdir(compiler)], cmd);
}

/// Get a path to the compiler specified.
/// Gets a path to the compiler specified.
pub fn rustc(&self, compiler: Compiler) -> PathBuf {
if compiler.is_snapshot(self) {
self.initial_rustc.clone()
Expand All @@ -659,7 +659,7 @@ impl<'a> Builder<'a> {
}
}

/// Get the paths to all of the compiler's codegen backends.
/// Gets the paths to all of the compiler's codegen backends.
fn codegen_backends(&self, compiler: Compiler) -> impl Iterator<Item = PathBuf> {
fs::read_dir(self.sysroot_codegen_backends(compiler))
.into_iter()
Expand Down
4 changes: 2 additions & 2 deletions src/bootstrap/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,10 +227,10 @@ lazy_static! {
pub static ref INTERNER: Interner = Interner::default();
}

/// This is essentially a HashMap which allows storing any type in its input and
/// This is essentially a `HashMap` which allows storing any type in its input and
/// any type in its output. It is a write-once cache; values are never evicted,
/// which means that references to the value can safely be returned from the
/// get() method.
/// `get()` method.
#[derive(Debug)]
pub struct Cache(
RefCell<HashMap<
Expand Down
2 changes: 1 addition & 1 deletion src/bootstrap/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ impl Step for Rustc {
});
}

/// Build the compiler.
/// Builds the compiler.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this an improvement rather than a random stylistic change?
What will prevent people from using their preferred form again?

=> controversial

Copy link
Contributor Author

@alexreg alexreg Feb 10, 2019

Choose a reason for hiding this comment

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

Not controversial at all. This is the standard/conventional style, to use the present tense, and improves consistency too. What prevents this happening again is people like you and me making sure this is used when reviewing PRs.

Copy link
Member

Choose a reason for hiding this comment

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

"Build the compiler" is also present tense, so that's not even the point here. The difference is about using 2nd or 3rd person singular.

But indeed the majority of doc comments seem to be written in 3rd person.

Copy link
Contributor

Choose a reason for hiding this comment

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

@RalfJung
[ot] I never realized imperative is actually a usual present tense with implicit "you" in English, it's an entirely separate form without a tense in Russian.

Copy link
Member

@RalfJung RalfJung Feb 11, 2019

Choose a reason for hiding this comment

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

Yeah same in German... English just has the most boring verbs.^^

I guess it's actually meant to be imperative here, not 2nd person singular. Whatever.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@RalfJung @petrochenkov Well yes, but it's the imperative mood... the issue is so many verb forms are degenerate in English. In any case, the convention seems to be 3rd person singular present indicative. ;-) I.e., "[This function] builds the compiler." This makes sense to me.

///
/// This will build the compiler for a particular stage of the build using
/// the `compiler` targeting the `target` architecture. The artifacts
Expand Down
2 changes: 1 addition & 1 deletion src/bootstrap/clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
//! Responsible for cleaning out a build directory of all old and stale
//! artifacts to prepare for a fresh build. Currently doesn't remove the
//! `build/cache` directory (download cache) or the `build/$target/llvm`
//! directory unless the --all flag is present.
//! directory unless the `--all` flag is present.

use std::fs;
use std::io::{self, ErrorKind};
Expand Down
8 changes: 4 additions & 4 deletions src/bootstrap/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ impl Step for Std {
});
}

/// Build the standard library.
/// Builds the standard library.
///
/// This will build the standard library for a particular stage of the build
/// using the `compiler` targeting the `target` architecture. The artifacts
Expand Down Expand Up @@ -269,7 +269,7 @@ impl Step for StartupObjects {
});
}

/// Build and prepare startup objects like rsbegin.o and rsend.o
/// Builds and prepare startup objects like rsbegin.o and rsend.o
///
/// These are primarily used on Windows right now for linking executables/dlls.
/// They don't require any library support as they're just plain old object
Expand Down Expand Up @@ -334,7 +334,7 @@ impl Step for Test {
});
}

/// Build libtest.
/// Builds libtest.
///
/// This will build libtest and supporting libraries for a particular stage of
/// the build using the `compiler` targeting the `target` architecture. The
Expand Down Expand Up @@ -455,7 +455,7 @@ impl Step for Rustc {
});
}

/// Build the compiler.
/// Builds the compiler.
///
/// This will build the compiler for a particular stage of the build using
/// the `compiler` targeting the `target` architecture. The artifacts
Expand Down
2 changes: 1 addition & 1 deletion src/bootstrap/dist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ impl Step for Mingw {
run.builder.ensure(Mingw { host: run.target });
}

/// Build the `rust-mingw` installer component.
/// Builds the `rust-mingw` installer component.
///
/// This contains all the bits and pieces to run the MinGW Windows targets
/// without any extra installed software (e.g., we bundle gcc, libraries, etc).
Expand Down
8 changes: 4 additions & 4 deletions src/bootstrap/doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ impl Step for TheBook {
});
}

/// Build the book and associated stuff.
/// Builds the book and associated stuff.
///
/// We need to build:
///
Expand Down Expand Up @@ -611,7 +611,7 @@ impl Step for WhitelistedRustc {
});
}

/// Generate whitelisted compiler crate documentation.
/// Generates whitelisted compiler crate documentation.
///
/// This will generate all documentation for crates that are whitelisted
/// to be included in the standard documentation. This documentation is
Expand Down Expand Up @@ -683,7 +683,7 @@ impl Step for Rustc {
});
}

/// Generate compiler documentation.
/// Generates compiler documentation.
///
/// This will generate all documentation for compiler and dependencies.
/// Compiler documentation is distributed separately, so we make sure
Expand Down Expand Up @@ -784,7 +784,7 @@ impl Step for Rustdoc {
});
}

/// Generate compiler documentation.
/// Generates compiler documentation.
///
/// This will generate all documentation for compiler and dependencies.
/// Compiler documentation is distributed separately, so we make sure
Expand Down
18 changes: 9 additions & 9 deletions src/bootstrap/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@
//! ## Copying stage0 {std,test,rustc}
//!
//! This copies the build output from Cargo into
//! `build/$HOST/stage0-sysroot/lib/rustlib/$ARCH/lib`. FIXME: This step's
//! `build/$HOST/stage0-sysroot/lib/rustlib/$ARCH/lib`. FIXME: this step's
//! documentation should be expanded -- the information already here may be
//! incorrect.
//!
Expand Down Expand Up @@ -504,7 +504,7 @@ impl Build {
cleared
}

/// Get the space-separated set of activated features for the standard
/// Gets the space-separated set of activated features for the standard
/// library.
fn std_features(&self) -> String {
let mut features = "panic-unwind".to_string();
Expand All @@ -521,7 +521,7 @@ impl Build {
features
}

/// Get the space-separated set of activated features for the compiler.
/// Gets the space-separated set of activated features for the compiler.
fn rustc_features(&self) -> String {
let mut features = String::new();
if self.config.jemalloc {
Expand Down Expand Up @@ -609,7 +609,7 @@ impl Build {
self.out.join(&*target).join("crate-docs")
}

/// Returns true if no custom `llvm-config` is set for the specified target.
/// Returns `true` if no custom `llvm-config` is set for the specified target.
///
/// If no custom `llvm-config` was specified then Rust's llvm will be used.
fn is_rust_llvm(&self, target: Interned<String>) -> bool {
Expand Down Expand Up @@ -857,13 +857,13 @@ impl Build {
.map(|p| &**p)
}

/// Returns true if this is a no-std `target`, if defined
/// Returns `true` if this is a no-std `target`, if defined
fn no_std(&self, target: Interned<String>) -> Option<bool> {
self.config.target_config.get(&target)
.map(|t| t.no_std)
}

/// Returns whether the target will be tested using the `remote-test-client`
/// Returns `true` if the target will be tested using the `remote-test-client`
/// and `remote-test-server` binaries.
fn remote_tested(&self, target: Interned<String>) -> bool {
self.qemu_rootfs(target).is_some() || target.contains("android") ||
Expand Down Expand Up @@ -1059,7 +1059,7 @@ impl Build {
self.rust_info.version(self, channel::CFG_RELEASE_NUM)
}

/// Return the full commit hash
/// Returns the full commit hash.
fn rust_sha(&self) -> Option<&str> {
self.rust_info.sha()
}
Expand All @@ -1079,7 +1079,7 @@ impl Build {
panic!("failed to find version in {}'s Cargo.toml", package)
}

/// Returns whether unstable features should be enabled for the compiler
/// Returns `true` if unstable features should be enabled for the compiler
/// we're building.
fn unstable_features(&self) -> bool {
match &self.config.channel[..] {
Expand Down Expand Up @@ -1327,7 +1327,7 @@ impl<'a> Compiler {
self
}

/// Returns whether this is a snapshot compiler for `build`'s configuration
/// Returns `true` if this is a snapshot compiler for `build`'s configuration
pub fn is_snapshot(&self, build: &Build) -> bool {
self.stage == 0 && self.host == build.build
}
Expand Down
16 changes: 8 additions & 8 deletions src/bootstrap/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ const ADB_TEST_DIR: &str = "/data/tmp/work";
/// The two modes of the test runner; tests or benchmarks.
#[derive(Debug, PartialEq, Eq, Hash, Copy, Clone, PartialOrd, Ord)]
pub enum TestKind {
/// Run `cargo test`
/// Run `cargo test`.
Test,
/// Run `cargo bench`
/// Run `cargo bench`.
Bench,
}

Expand Down Expand Up @@ -1288,7 +1288,7 @@ impl Step for DocTest {
run.never()
}

/// Run `rustdoc --test` for all documentation in `src/doc`.
/// Runs `rustdoc --test` for all documentation in `src/doc`.
///
/// This will run all tests in our markdown documentation (e.g., the book)
/// located in `src/doc`. The `rustdoc` that's run is the one that sits next to
Expand Down Expand Up @@ -1408,7 +1408,7 @@ impl Step for ErrorIndex {
});
}

/// Run the error index generator tool to execute the tests located in the error
/// Runs the error index generator tool to execute the tests located in the error
/// index.
///
/// The `error_index_generator` tool lives in `src/tools` and is used to
Expand Down Expand Up @@ -1614,7 +1614,7 @@ impl Step for Crate {
}
}

/// Run all unit tests plus documentation tests for a given crate defined
/// Runs all unit tests plus documentation tests for a given crate defined
/// by a `Cargo.toml` (single manifest)
///
/// This is what runs tests for crates like the standard library, compiler, etc.
Expand Down Expand Up @@ -1833,7 +1833,7 @@ fn envify(s: &str) -> String {
/// the standard library and such to the emulator ahead of time. This step
/// represents this and is a dependency of all test suites.
///
/// Most of the time this is a noop. For some steps such as shipping data to
/// Most of the time this is a no-op. For some steps such as shipping data to
/// QEMU we have to build our own tools so we've got conditional dependencies
/// on those programs as well. Note that the remote test client is built for
/// the build target (us) and the server is built for the target.
Expand Down Expand Up @@ -1904,7 +1904,7 @@ impl Step for Distcheck {
run.builder.ensure(Distcheck);
}

/// Run "distcheck", a 'make check' from a tarball
/// Runs "distcheck", a 'make check' from a tarball
fn run(self, builder: &Builder) {
builder.info("Distcheck");
let dir = builder.out.join("tmp").join("distcheck");
Expand Down Expand Up @@ -1965,7 +1965,7 @@ impl Step for Bootstrap {
const DEFAULT: bool = true;
const ONLY_HOSTS: bool = true;

/// Test the build system itself
/// Tests the build system itself.
fn run(self, builder: &Builder) {
let mut cmd = Command::new(&builder.initial_cargo);
cmd.arg("test")
Expand Down
4 changes: 2 additions & 2 deletions src/bootstrap/tool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ impl Step for ToolBuild {
run.never()
}

/// Build a tool in `src/tools`
/// Builds a tool in `src/tools`
///
/// This will build the specified tool with the specified `host` compiler in
/// `stage` into the normal cargo output directory.
Expand Down Expand Up @@ -621,7 +621,7 @@ tool_extended!((self, builder),
);

impl<'a> Builder<'a> {
/// Get a `Command` which is ready to run `tool` in `stage` built for
/// Gets a `Command` which is ready to run `tool` in `stage` built for
/// `host`.
pub fn tool_cmd(&self, tool: Tool) -> Command {
let mut cmd = Command::new(self.tool_exe(tool));
Expand Down
2 changes: 1 addition & 1 deletion src/bootstrap/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ pub fn exe(name: &str, target: &str) -> String {
}
}

/// Returns whether the file name given looks like a dynamic library.
/// Returns `true` if the file name given looks like a dynamic library.
pub fn is_dylib(name: &str) -> bool {
name.ends_with(".dylib") || name.ends_with(".so") || name.ends_with(".dll")
}
Expand Down
6 changes: 3 additions & 3 deletions src/build_helper/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ pub fn mtime(path: &Path) -> SystemTime {
.unwrap_or(UNIX_EPOCH)
}

/// Returns whether `dst` is up to date given that the file or files in `src`
/// Returns `true` if `dst` is up to date given that the file or files in `src`
/// are used to generate it.
///
/// Uses last-modified time checks to verify this.
Expand All @@ -190,12 +190,12 @@ pub struct NativeLibBoilerplate {
}

impl NativeLibBoilerplate {
/// On OSX we don't want to ship the exact filename that compiler-rt builds.
/// On macOS we don't want to ship the exact filename that compiler-rt builds.
/// This conflicts with the system and ours is likely a wildly different
/// version, so they can't be substituted.
///
/// As a result, we rename it here but we need to also use
/// `install_name_tool` on OSX to rename the commands listed inside of it to
/// `install_name_tool` on macOS to rename the commands listed inside of it to
/// ensure it's linked against correctly.
pub fn fixup_sanitizer_lib_name(&self, sanitizer_name: &str) {
if env::var("TARGET").unwrap() != "x86_64-apple-darwin" {
Expand Down
4 changes: 2 additions & 2 deletions src/liballoc/borrow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,11 +137,11 @@ impl<T> ToOwned for T
/// ```
/// use std::borrow::{Cow, ToOwned};
///
/// struct Items<'a, X: 'a> where [X]: ToOwned<Owned=Vec<X>> {
/// struct Items<'a, X: 'a> where [X]: ToOwned<Owned = Vec<X>> {
/// values: Cow<'a, [X]>,
/// }
///
/// impl<'a, X: Clone + 'a> Items<'a, X> where [X]: ToOwned<Owned=Vec<X>> {
/// impl<'a, X: Clone + 'a> Items<'a, X> where [X]: ToOwned<Owned = Vec<X>> {
/// fn new(v: Cow<'a, [X]>) -> Self {
/// Items { values: v }
/// }
Expand Down
2 changes: 1 addition & 1 deletion src/liballoc/collections/binary_heap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -863,7 +863,7 @@ struct Hole<'a, T: 'a> {
}

impl<'a, T> Hole<'a, T> {
/// Create a new Hole at index `pos`.
/// Create a new `Hole` at index `pos`.
///
/// Unsafe because pos must be within the data slice.
#[inline]
Expand Down
2 changes: 1 addition & 1 deletion src/liballoc/collections/btree/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2368,7 +2368,7 @@ impl<'a, K: Ord, V> OccupiedEntry<'a, K, V> {

/// Gets a mutable reference to the value in the entry.
///
/// If you need a reference to the `OccupiedEntry` which may outlive the
/// If you need a reference to the `OccupiedEntry` that may outlive the
/// destruction of the `Entry` value, see [`into_mut`].
///
/// [`into_mut`]: #method.into_mut
Expand Down
Loading