tools: comments

Author: Alexander Regueiro

src/tools/compiletest/src/common.rs

@@ -145,101 +145,101 @@ pub struct Config {
     pub force_valgrind: bool,
 
     /// The path to the Clang executable to run Clang-based tests with. If
-    /// `None` then these tests will be ignored.
+    /// `None`, then these tests will be ignored.
     pub run_clang_based_tests_with: Option<String>,
 
-    /// The directory containing the tests to run
+    /// The directory containing the tests to run.
     pub src_base: PathBuf,
 
-    /// The directory where programs should be built
+    /// The directory where programs should be built.
     pub build_base: PathBuf,
 
-    /// The name of the stage being built (stage1, etc)
+    /// The name of the stage being built (stage1, etc.)
     pub stage_id: String,
 
-    /// The test mode, compile-fail, run-fail, run-pass
+    /// The test mode, compile-fail, run-fail, run-pass.
     pub mode: Mode,
 
-    /// Run ignored tests
+    /// Run ignored tests.
     pub run_ignored: bool,
 
-    /// Only run tests that match this filter
+    /// Only run tests that match this filter.
     pub filter: Option<String>,
 
-    /// Exactly match the filter, rather than a substring
+    /// Exactly match the filter, rather than a substring.
     pub filter_exact: bool,
 
-    /// Write out a parseable log of tests that were run
+    /// Write out a parseable log of tests that were run.
     pub logfile: Option<PathBuf>,
 
     /// A command line to prefix program execution with,
-    /// for running under valgrind
+    /// for running under valgrind.
     pub runtool: Option<String>,
 
     /// Flags to pass to the compiler when building for the host
     pub host_rustcflags: Option<String>,
 
-    /// Flags to pass to the compiler when building for the target
+    /// Flags to pass to the compiler when building for the target.
     pub target_rustcflags: Option<String>,
 
-    /// Target system to be tested
+    /// Target system to be tested.
     pub target: String,
 
-    /// Host triple for the compiler being invoked
+    /// Host triple for the compiler being invoked.
     pub host: String,
 
-    /// Path to / name of the GDB executable
+    /// Path to / name of the GDB executable.
     pub gdb: Option<String>,
 
-    /// Version of GDB, encoded as ((major * 1000) + minor) * 1000 + patch
+    /// Version of GDB, encoded as `((major * 1000) + minor) * 1000 + patch`.
     pub gdb_version: Option<u32>,
 
-    /// Whether GDB has native rust support
+    /// Whether GDB has native rust support.
     pub gdb_native_rust: bool,
 
-    /// Version of LLDB
+    /// Version of LLDB.
     pub lldb_version: Option<String>,
 
-    /// Whether LLDB has native rust support
+    /// `true` if LLDB has native rust support.
     pub lldb_native_rust: bool,
 
-    /// Version of LLVM
+    /// Version of LLVM.
     pub llvm_version: Option<String>,
 
-    /// Is LLVM a system LLVM
+    /// `true` if LLVM is system LLVM.
     pub system_llvm: bool,
 
-    /// Path to the android tools
+    /// Path to the Android tools.
     pub android_cross_path: PathBuf,
 
-    /// Extra parameter to run adb on arm-linux-androideabi
+    /// Extra parameter to run adb on arm-linux-androideabi.
     pub adb_path: String,
 
-    /// Extra parameter to run test suite on arm-linux-androideabi
+    /// Extra parameter to run test suite on arm-linux-androideabi.
     pub adb_test_dir: String,
 
-    /// status whether android device available or not
+    /// Status whether android device available or not.
     pub adb_device_status: bool,
 
-    /// the path containing LLDB's Python module
+    /// Path containing LLDB's Python module.
     pub lldb_python_dir: Option<String>,
 
-    /// Explain what's going on
+    /// `true` to enable verbose output.
     pub verbose: bool,
 
-    /// Print one character per test instead of one line
+    /// Print one character per test instead of one line.
     pub quiet: bool,
 
-    /// Whether to use colors in test.
+    /// `true` to use colors in test.
     pub color: ColorConfig,
 
-    /// where to find the remote test client process, if we're using it
+    /// Where to find the remote test client process, if we're using it.
     pub remote_test_client: Option<PathBuf>,
 
-    /// mode describing what file the actual ui output will be compared to
+    /// Mode describing what file the actual ui output will be compared to.
     pub compare_mode: Option<CompareMode>,
 
-    // Configuration for various run-make tests frobbing things like C compilers
+    // Configuration for various run-make tests-frobbing things like C compilers,
     // or querying about various LLVM component information.
     pub cc: String,
     pub cxx: String,

src/tools/compiletest/src/header.rs

@@ -304,7 +304,7 @@ pub struct TestProps {
     pub dont_check_compiler_stderr: bool,
     // Don't force a --crate-type=dylib flag on the command line
     pub no_prefer_dynamic: bool,
-    // Run --pretty expanded when running pretty printing tests
+    // Run --pretty expanded when running pretty-printing tests
     pub pretty_expanded: bool,
     // Which pretty mode are we testing with, default to 'normal'
     pub pretty_mode: String,
@@ -314,7 +314,7 @@ pub struct TestProps {
     pub forbid_output: Vec<String>,
     // Revisions to test for incremental compilation.
     pub revisions: Vec<String>,
-    // Directory (if any) to use for incremental compilation.  This is
+    // Directory (if any) to use for incremental compilation. This is
     // not set by end-users; rather it is set by the incremental
     // testing harness and used when generating compilation
     // arguments. (In particular, it propagates to the aux-builds.)
@@ -678,7 +678,7 @@ impl Config {
 
     fn parse_env(&self, line: &str, name: &str) -> Option<(String, String)> {
         self.parse_name_value_directive(line, name).map(|nv| {
-            // nv is either FOO or FOO=BAR
+            // `nv` is either `"FOO"` or `"FOO=BAR"`.
             let mut strs: Vec<String> = nv.splitn(2, '=').map(str::to_owned).collect();
 
             match strs.len() {
@@ -867,7 +867,7 @@ fn expand_variables(mut value: String, config: &Config) -> String {
 /// assert_eq!(s, " -> \"something ($WORD bits)\".");
 /// ```
 fn parse_normalization_string(line: &mut &str) -> Option<String> {
-    // FIXME support escapes in strings.
+    // FIXME: support escapes in strings.
     let begin = line.find('"')? + 1;
     let end = line[begin..].find('"')? + begin;
     let result = line[begin..end].to_owned();

src/tools/compiletest/src/json.rs

@@ -93,7 +93,7 @@ pub fn parse_output(file_name: &str, output: &str, proc_res: &ProcRes) -> Vec<Er
 
 fn parse_line(file_name: &str, line: &str, output: &str, proc_res: &ProcRes) -> Vec<Error> {
     // The compiler sometimes intermingles non-JSON stuff into the
-    // output.  This hack just skips over such lines. Yuck.
+    // output. This hack just skips over such lines. Yuck.
     if line.starts_with('{') {
         match serde_json::from_str::<Diagnostic>(line) {
             Ok(diagnostic) => {
@@ -147,7 +147,7 @@ fn push_expected_errors(
     };
 
     // We break the output into multiple lines, and then append the
-    // [E123] to every line in the output. This may be overkill.  The
+    // [E123] to every line in the output. This may be overkill. The
     // intention was to match existing tests that do things like "//|
     // found `i32` [E123]" and expect to match that somewhere, and yet
     // also ensure that `//~ ERROR E123` *always* works. The
@@ -156,7 +156,7 @@ fn push_expected_errors(
     let with_code = |span: &DiagnosticSpan, text: &str| {
         match diagnostic.code {
             Some(ref code) =>
-                // FIXME(#33000) -- it'd be better to use a dedicated
+                // FIXME(#33000): it'd be better to use a dedicated
                 // UI harness than to include the line/col number like
                 // this, but some current tests rely on it.
                 //
@@ -169,7 +169,7 @@ fn push_expected_errors(
                         span.line_end, span.column_end,
                         text, code.code.clone()),
             None =>
-                // FIXME(#33000) -- it'd be better to use a dedicated UI harness
+                // FIXME(#33000): it'd be better to use a dedicated UI harness.
                 format!("{}:{}: {}:{}: {}",
                         span.line_start, span.column_start,
                         span.line_end, span.column_end,

src/tools/compiletest/src/main.rs

@@ -470,14 +470,14 @@ pub fn run_tests(config: &Config) {
         _ => { /* proceed */ }
     }
 
-    // FIXME(#33435) Avoid spurious failures in codegen-units/partitioning tests.
+    // FIXME(#33435): avoid spurious failures in codegen-units/partitioning tests.
     if let Mode::CodegenUnits = config.mode {
         let _ = fs::remove_dir_all("tmp/partitioning-tests");
     }
 
     let opts = test_opts(config);
     let tests = make_tests(config);
-    // sadly osx needs some file descriptor limits raised for running tests in
+    // sadly macOS needs some file descriptor limits raised for running tests in
     // parallel (especially when we have lots and lots of child processes).
     // For context, see #8904
     unsafe {
@@ -619,7 +619,7 @@ pub fn make_test(config: &Config, testpaths: &TestPaths) -> Vec<test::TestDescAn
     };
 
     // The `should-fail` annotation doesn't apply to pretty tests,
-    // since we run the pretty printer across all tests by default.
+    // since we run the pretty-printer across all tests by default.
     // If desired, we could add a `should-fail-pretty` annotation.
     let should_panic = match config.mode {
         Pretty => test::ShouldPanic::No,
@@ -640,7 +640,7 @@ pub fn make_test(config: &Config, testpaths: &TestPaths) -> Vec<test::TestDescAn
     revisions
         .into_iter()
         .map(|revision| {
-            // Debugging emscripten code doesn't make sense today
+            // Debugging Emscripten code doesn't make sense today.
             let ignore = early_props.ignore == Ignore::Ignore
                 || !up_to_date(
                     config,
@@ -702,7 +702,7 @@ fn up_to_date(
                 Stamp::from_path(&testpaths.file.parent().unwrap().join("auxiliary").join(aux))
             }),
     );
-    // Relevant pretty printer files
+    // Relevant pretty-printer files.
     let pretty_printer_files = [
         "src/etc/debugger_pretty_printers_common.py",
         "src/etc/gdb_load_rust_pretty_printers.py",
@@ -949,7 +949,7 @@ fn extract_lldb_version(full_version_line: Option<String>) -> (Option<String>, b
     // There doesn't seem to be a way to correlate the Apple version
     // with the upstream version, and since the tests were originally
     // written against Apple versions, we make a fake Apple version by
-    // multiplying the first number by 100.  This is a hack, but
+    // multiplying the first number by 100. This is a hack, but
     // normally fine because the only non-Apple version we test is
     // rust-enabled.
 

src/tools/compiletest/src/read2.rs

@@ -1,5 +1,5 @@
-// FIXME: This is a complete copy of `cargo/src/cargo/util/read2.rs`
-// Consider unify the read2() in libstd, cargo and this to prevent further code duplication.
+// FIXME: this is a complete copy of `cargo/src/cargo/util/read2.rs`
+// Consider unifying the `read2()` in libstd, cargo and this to prevent further code duplication.
 
 pub use self::imp::read2;
 
@@ -56,7 +56,7 @@ mod imp {
         let mut errfd = 1;
 
         while nfds > 0 {
-            // wait for either pipe to become readable using `select`
+            // Wait for either pipe to become readable using `select`.
             let r = unsafe { libc::poll(fds.as_mut_ptr(), nfds, -1) };
             if r == -1 {
                 let err = io::Error::last_os_error();