Rollup merge of rust-lang#26452 - michaelsproul:the-second-coming, r=pnkfelix

Manishearth · Manishearth · commit 7effb31fa201 · 2015-06-20T21:40:37.000+05:30
As per rust-lang#26009 this PR implements a new collation system for extended-error metadata. I've tried to keep it as simple as possible for now, so there's no uniqueness checking and minimal modularity. Although using a lint was discussed in rust-lang#26009 I decided against this because it would require converting the AST output from the plugin back into an internal data-structure. Emitting the metadata from within the plugin prevents this double-handling. We also didn't identify this as the source of the failures last time, although something untoward was definitely happening... With that in mind I would like as much feedback as possible on this before it's merged, I don't want to break the bots again! I've successfully built for my host architecture and I'm building an ARM cross-compiler now to test my assumptions about the various `CFG` variables. Despite the confusing name of `CFG_COMPILER_HOST_TRIPLE` it is actually the compile time target triple, as explained in `mk/target.mk`. ``` # This is the compile-time target-triple for the compiler. For the compiler at # runtime, this should be considered the host-triple. More explanation for why # this exists can be found on issue rust-lang#2400 export CFG_COMPILER_HOST_TRIPLE ``` CC @pnkfelix @brson @nrc @alexcrichton Closes rust-lang#25705, closes rust-lang#26009.
diff --git a/mk/docs.mk b/mk/docs.mk
@@ -77,8 +77,7 @@ ERR_IDX_GEN = $(RPATH_VAR2_T_$(CFG_BUILD)_H_$(CFG_BUILD)) $(ERR_IDX_GEN_EXE)
 
 D := $(S)src/doc
 
-# FIXME (#25705) eventually may want to put error-index target back here.
-DOC_TARGETS := trpl style
+DOC_TARGETS := trpl style error-index
 COMPILER_DOC_TARGETS :=
 DOC_L10N_TARGETS :=
 
diff --git a/src/error-index-generator/main.rs b/src/error-index-generator/main.rs
@@ -17,6 +17,7 @@ extern crate serialize as rustc_serialize;
 use std::collections::BTreeMap;
 use std::fs::{read_dir, File};
 use std::io::{Read, Write};
+use std::env;
 use std::path::Path;
 use std::error::Error;
 
@@ -106,7 +107,8 @@ r##"<!DOCTYPE html>
 }
 
 fn main_with_result() -> Result<(), Box<Error>> {
-    let metadata_dir = get_metadata_dir();
+    let build_arch = try!(env::var("CFG_BUILD"));
+    let metadata_dir = get_metadata_dir(&build_arch);
     let err_map = try!(load_all_errors(&metadata_dir));
     try!(render_error_page(&err_map, Path::new("doc/error-index.html")));
     Ok(())
diff --git a/src/librustc_typeck/diagnostics.rs b/src/librustc_typeck/diagnostics.rs
@@ -730,9 +730,11 @@ struct ListNode {
 This type cannot have a well-defined size, because it needs to be arbitrarily
 large (since we would be able to nest `ListNode`s to any depth). Specifically,
 
-    size of `ListNode` = 1 byte for `head`
-                       + 1 byte for the discriminant of the `Option`
-                       + size of `ListNode`
+```plain
+size of `ListNode` = 1 byte for `head`
+                   + 1 byte for the discriminant of the `Option`
+                   + size of `ListNode`
+```
 
 One way to fix this is by wrapping `ListNode` in a `Box`, like so:
 
diff --git a/src/libsyntax/diagnostics/metadata.rs b/src/libsyntax/diagnostics/metadata.rs
@@ -14,24 +14,18 @@
 //! currently always a crate name.
 
 use std::collections::BTreeMap;
-use std::env;
 use std::path::PathBuf;
-use std::fs::{read_dir, create_dir_all, OpenOptions, File};
-use std::io::{Read, Write};
+use std::fs::{remove_file, create_dir_all, File};
+use std::io::Write;
 use std::error::Error;
-use rustc_serialize::json::{self, as_json};
+use rustc_serialize::json::as_json;
 
 use codemap::Span;
 use ext::base::ExtCtxt;
 use diagnostics::plugin::{ErrorMap, ErrorInfo};
 
-pub use self::Uniqueness::*;
-
 // Default metadata directory to use for extended error JSON.
-const ERROR_METADATA_DIR_DEFAULT: &'static str = "tmp/extended-errors";
-
-// The name of the environment variable that sets the metadata dir.
-const ERROR_METADATA_VAR: &'static str = "ERROR_METADATA_DIR";
+const ERROR_METADATA_PREFIX: &'static str = "tmp/extended-errors";
 
 /// JSON encodable/decodable version of `ErrorInfo`.
 #[derive(PartialEq, RustcDecodable, RustcEncodable)]
@@ -61,84 +55,32 @@ impl ErrorLocation {
     }
 }
 
-/// Type for describing the uniqueness of a set of error codes, as returned by `check_uniqueness`.
-pub enum Uniqueness {
-    /// All errors in the set checked are unique according to the metadata files checked.
-    Unique,
-    /// One or more errors in the set occur in another metadata file.
-    /// This variant contains the first duplicate error code followed by the name
-    /// of the metadata file where the duplicate appears.
-    Duplicate(String, String)
-}
-
-/// Get the directory where metadata files should be stored.
-pub fn get_metadata_dir() -> PathBuf {
-    match env::var(ERROR_METADATA_VAR) {
-        Ok(v) => From::from(v),
-        Err(_) => From::from(ERROR_METADATA_DIR_DEFAULT)
-    }
-}
-
-/// Get the path where error metadata for the set named by `name` should be stored.
-fn get_metadata_path(name: &str) -> PathBuf {
-    get_metadata_dir().join(format!("{}.json", name))
+/// Get the directory where metadata for a given `prefix` should be stored.
+///
+/// See `output_metadata`.
+pub fn get_metadata_dir(prefix: &str) -> PathBuf {
+    PathBuf::from(ERROR_METADATA_PREFIX).join(prefix)
 }
 
-/// Check that the errors in `err_map` aren't present in any metadata files in the
-/// metadata directory except the metadata file corresponding to `name`.
-pub fn check_uniqueness(name: &str, err_map: &ErrorMap) -> Result<Uniqueness, Box<Error>> {
-    let metadata_dir = get_metadata_dir();
-    let metadata_path = get_metadata_path(name);
-
-    // Create the error directory if it does not exist.
-    try!(create_dir_all(&metadata_dir));
-
-    // Check each file in the metadata directory.
-    for entry in try!(read_dir(&metadata_dir)) {
-        let path = try!(entry).path();
-
-        // Skip any existing file for this set.
-        if path == metadata_path {
-            continue;
-        }
-
-        // Read the metadata file into a string.
-        let mut metadata_str = String::new();
-        try!(
-            File::open(&path).and_then(|mut f|
-            f.read_to_string(&mut metadata_str))
-        );
-
-        // Parse the JSON contents.
-        let metadata: ErrorMetadataMap = try!(json::decode(&metadata_str));
-
-        // Check for duplicates.
-        for err in err_map.keys() {
-            let err_code = err.as_str();
-            if metadata.contains_key(err_code) {
-                return Ok(Duplicate(
-                    err_code.to_string(),
-                    path.to_string_lossy().into_owned()
-                ));
-            }
-        }
-    }
-
-    Ok(Unique)
+/// Map `name` to a path in the given directory: <directory>/<name>.json
+fn get_metadata_path(directory: PathBuf, name: &str) -> PathBuf {
+    directory.join(format!("{}.json", name))
 }
 
-/// Write metadata for the errors in `err_map` to disk, to a file corresponding to `name`.
-pub fn output_metadata(ecx: &ExtCtxt, name: &str, err_map: &ErrorMap)
+/// Write metadata for the errors in `err_map` to disk, to a file corresponding to `prefix/name`.
+///
+/// For our current purposes the prefix is the target architecture and the name is a crate name.
+/// If an error occurs steps will be taken to ensure that no file is created.
+pub fn output_metadata(ecx: &ExtCtxt, prefix: &str, name: &str, err_map: &ErrorMap)
     -> Result<(), Box<Error>>
 {
-    let metadata_path = get_metadata_path(name);
+    // Create the directory to place the file in.
+    let metadata_dir = get_metadata_dir(prefix);
+    try!(create_dir_all(&metadata_dir));
 
-    // Open the dump file.
-    let mut dump_file = try!(OpenOptions::new()
-        .write(true)
-        .create(true)
-        .open(&metadata_path)
-    );
+    // Open the metadata file.
+    let metadata_path = get_metadata_path(metadata_dir, name);
+    let mut metadata_file = try!(File::create(&metadata_path));
 
     // Construct a serializable map.
     let json_map = err_map.iter().map(|(k, &ErrorInfo { description, use_site })| {
@@ -150,6 +92,10 @@ pub fn output_metadata(ecx: &ExtCtxt, name: &str, err_map: &ErrorMap)
         (key, value)
     }).collect::<ErrorMetadataMap>();
 
-    try!(write!(&mut dump_file, "{}", as_json(&json_map)));
-    Ok(())
+    // Write the data to the file, deleting it if the write fails.
+    let result = write!(&mut metadata_file, "{}", as_json(&json_map));
+    if result.is_err() {
+        try!(remove_file(&metadata_path));
+    }
+    Ok(try!(result))
 }
diff --git a/src/libsyntax/diagnostics/plugin.rs b/src/libsyntax/diagnostics/plugin.rs
@@ -10,6 +10,7 @@
 
 use std::cell::RefCell;
 use std::collections::BTreeMap;
+use std::env;
 
 use ast;
 use ast::{Ident, Name, TokenTree};
@@ -20,6 +21,8 @@ use parse::token;
 use ptr::P;
 use util::small_vector::SmallVector;
 
+use diagnostics::metadata::output_metadata;
+
 // Maximum width of any line in an extended error description (inclusive).
 const MAX_DESCRIPTION_WIDTH: usize = 80;
 
@@ -154,7 +157,7 @@ pub fn expand_build_diagnostic_array<'cx>(ecx: &'cx mut ExtCtxt,
                                           token_tree: &[TokenTree])
                                           -> Box<MacResult+'cx> {
     assert_eq!(token_tree.len(), 3);
-    let (_crate_name, name) = match (&token_tree[0], &token_tree[2]) {
+    let (crate_name, name) = match (&token_tree[0], &token_tree[2]) {
         (
             // Crate name.
             &ast::TtToken(_, token::Ident(ref crate_name, _)),
@@ -164,9 +167,18 @@ pub fn expand_build_diagnostic_array<'cx>(ecx: &'cx mut ExtCtxt,
         _ => unreachable!()
     };
 
-    // FIXME (#25705): we used to ensure error code uniqueness and
-    // output error description JSON metadata here, but the approach
-    // employed was too brittle.
+    // Output error metadata to `tmp/extended-errors/<target arch>/<crate name>.json`
+    let target_triple = env::var("CFG_COMPILER_HOST_TRIPLE")
+        .ok().expect("unable to determine target arch from $CFG_COMPILER_HOST_TRIPLE");
+
+    with_registered_diagnostics(|diagnostics| {
+        if let Err(e) = output_metadata(ecx, &target_triple, crate_name, &diagnostics) {
+            ecx.span_bug(span, &format!(
+                "error writing metadata for triple `{}` and crate `{}`, error: {}, cause: {:?}",
+                target_triple, crate_name, e.description(), e.cause()
+            ));
+        }
+    });
 
     // Construct the output expression.
     let (count, expr) =
