[PoC] Make NOTE less viral

Author: petrochenkov

src/tools/compiletest/src/errors.rs

@@ -70,6 +70,7 @@ pub struct Error {
     /// A temporary measure to avoid adding too many `NOTE` and `HELP` annotations in #NNNNNN.
     /// Only makes sense for "expected" errors, not for "found" errors.
     pub viral: bool,
+    pub parent: Option<usize>,
 }
 
 impl Error {
@@ -189,7 +190,7 @@ fn parse_expected(
         kind,
         msg
     );
-    Some((follow_prev, Error { line_num, kind, msg, should_annotate: true, viral }))
+    Some((follow_prev, Error { line_num, kind, msg, should_annotate: true, viral, parent: None }))
 }
 
 #[cfg(test)]

src/tools/compiletest/src/json.rs

@@ -141,25 +141,31 @@ pub fn extract_rendered(output: &str) -> String {
 }
 
 pub fn parse_output(file_name: &str, output: &str, proc_res: &ProcRes) -> Vec<Error> {
-    output.lines().flat_map(|line| parse_line(file_name, line, output, proc_res)).collect()
+    let mut expected_errors = vec![];
+    for line in output.lines() {
+        parse_line(&mut expected_errors, file_name, line, output, proc_res);
+    }
+    expected_errors
 }
 
-fn parse_line(file_name: &str, line: &str, output: &str, proc_res: &ProcRes) -> Vec<Error> {
+fn parse_line(
+    expected_errors: &mut Vec<Error>,
+    file_name: &str,
+    line: &str,
+    output: &str,
+    proc_res: &ProcRes,
+) {
     // The compiler sometimes intermingles non-JSON stuff into the
     // output.  This hack just skips over such lines. Yuck.
     if line.starts_with('{') {
         match serde_json::from_str::<Diagnostic>(line) {
             Ok(diagnostic) => {
-                let mut expected_errors = vec![];
-                push_expected_errors(&mut expected_errors, &diagnostic, &[], file_name);
-                expected_errors
+                push_expected_errors(expected_errors, &diagnostic, &[], file_name, None)
             }
             Err(error) => {
                 // Ignore the future compat report message - this is handled
                 // by `extract_rendered`
-                if serde_json::from_str::<FutureIncompatReport>(line).is_ok() {
-                    vec![]
-                } else {
+                if !serde_json::from_str::<FutureIncompatReport>(line).is_ok() {
                     proc_res.fatal(
                         Some(&format!(
                             "failed to decode compiler output as json: \
@@ -171,8 +177,6 @@ fn parse_line(file_name: &str, line: &str, output: &str, proc_res: &ProcRes) ->
                 }
             }
         }
-    } else {
-        vec![]
     }
 }
 
@@ -181,6 +185,7 @@ fn push_expected_errors(
     diagnostic: &Diagnostic,
     default_spans: &[&DiagnosticSpan],
     file_name: &str,
+    prev_parent: Option<usize>,
 ) {
     // In case of macro expansions, we need to get the span of the callsite
     let spans_info_in_this_file: Vec<_> = diagnostic
@@ -240,6 +245,7 @@ fn push_expected_errors(
     // more structured shortly anyhow.
     let kind = ErrorKind::from_compiler_str(&diagnostic.level);
     let mut message_lines = diagnostic.message.lines();
+    let mut parent = None;
     if let Some(first_line) = message_lines.next() {
         let ignore = |s| {
             static RE: OnceLock<Regex> = OnceLock::new();
@@ -250,21 +256,29 @@ fn push_expected_errors(
         };
 
         if primary_spans.is_empty() && !ignore(first_line) {
+            if parent.is_none() {
+                parent = Some(expected_errors.len());
+            }
             expected_errors.push(Error {
                 line_num: None,
                 kind,
                 msg: with_code(None, first_line),
                 should_annotate: diagnostic.level != "failure-note",
                 viral: true,
+                parent: prev_parent,
             });
         } else {
             for span in primary_spans {
+                if parent.is_none() {
+                    parent = Some(expected_errors.len());
+                }
                 expected_errors.push(Error {
                     line_num: Some(span.line_start),
                     kind,
                     msg: with_code(Some(span), first_line),
                     should_annotate: diagnostic.level != "failure-note",
                     viral: true,
+                    parent: prev_parent,
                 });
             }
         }
@@ -277,6 +291,7 @@ fn push_expected_errors(
                 msg: with_code(None, next_line),
                 should_annotate: false,
                 viral: true,
+                parent: prev_parent,
             });
         } else {
             for span in primary_spans {
@@ -286,6 +301,7 @@ fn push_expected_errors(
                     msg: with_code(Some(span), next_line),
                     should_annotate: false,
                     viral: true,
+                    parent: prev_parent,
                 });
             }
         }
@@ -301,6 +317,7 @@ fn push_expected_errors(
                     msg: line.to_string(),
                     should_annotate: true,
                     viral: true,
+                    parent,
                 });
             }
         }
@@ -309,7 +326,7 @@ fn push_expected_errors(
     // Add notes for the backtrace
     for span in primary_spans {
         if let Some(frame) = &span.expansion {
-            push_backtrace(expected_errors, frame, file_name);
+            push_backtrace(expected_errors, frame, file_name, parent);
         }
     }
 
@@ -321,19 +338,21 @@ fn push_expected_errors(
             msg: span.label.clone().unwrap(),
             should_annotate: true,
             viral: true,
+            parent,
         });
     }
 
     // Flatten out the children.
     for child in &diagnostic.children {
-        push_expected_errors(expected_errors, child, primary_spans, file_name);
+        push_expected_errors(expected_errors, child, primary_spans, file_name, parent);
     }
 }
 
 fn push_backtrace(
     expected_errors: &mut Vec<Error>,
     expansion: &DiagnosticSpanMacroExpansion,
     file_name: &str,
+    parent: Option<usize>,
 ) {
     if Path::new(&expansion.span.file_name) == Path::new(&file_name) {
         expected_errors.push(Error {
@@ -342,10 +361,11 @@ fn push_backtrace(
             msg: format!("in this expansion of {}", expansion.macro_decl_name),
             should_annotate: true,
             viral: true,
+            parent,
         });
     }
 
     if let Some(previous_expansion) = &expansion.span.expansion {
-        push_backtrace(expected_errors, previous_expansion, file_name);
+        push_backtrace(expected_errors, previous_expansion, file_name, parent);
     }
 }

src/tools/compiletest/src/runtest.rs

@@ -713,13 +713,16 @@ impl<'test> TestCx<'test> {
         // Otherwise, all "help" messages reported by the compiler will be ignored.
         // This logic also applies to "note" messages.
         let expect_help = expected_errors.iter().any(|ee| ee.kind == ErrorKind::Help && ee.viral);
-        let expect_note = expected_errors.iter().any(|ee| ee.kind == ErrorKind::Note && ee.viral);
+        // let expect_note = expected_errors.iter().any(|ee| ee.kind == ErrorKind::Note && ee.viral);
 
         // Parse the JSON output from the compiler and extract out the messages.
-        let actual_errors = json::parse_output(&diagnostic_file_name, &proc_res.stderr, proc_res);
+        let mut actual_errors =
+            json::parse_output(&diagnostic_file_name, &proc_res.stderr, proc_res);
         let mut unexpected = Vec::new();
         let mut found = vec![false; expected_errors.len()];
-        for mut actual_error in actual_errors {
+        let mut unexpected_actual_indices = Vec::new();
+        let mut poisoned_parents = HashSet::new();
+        for (actual_index, actual_error) in actual_errors.iter_mut().enumerate() {
             actual_error.msg = self.normalize_output(&actual_error.msg, &[]);
 
             let opt_index =
@@ -735,22 +738,41 @@ impl<'test> TestCx<'test> {
                     // found a match, everybody is happy
                     assert!(!found[index]);
                     found[index] = true;
-                }
 
-                None => {
-                    // If the test is a known bug, don't require that the error is annotated
-                    if self.is_unexpected_compiler_message(&actual_error, expect_help, expect_note)
-                    {
-                        self.error(&format!(
-                            "{}:{}: unexpected {}: '{}'",
-                            file_name,
-                            actual_error.line_num_str(),
-                            actual_error.kind,
-                            actual_error.msg
-                        ));
-                        unexpected.push(actual_error);
+                    if actual_error.kind == ErrorKind::Note {
+                        poisoned_parents.insert(actual_error.parent);
                     }
                 }
+
+                None => unexpected_actual_indices.push(actual_index),
+            }
+        }
+
+        for unexpected_actual_index in unexpected_actual_indices {
+            // If the test is a known bug, don't require that the error is annotated
+            let actual_error = &actual_errors[unexpected_actual_index];
+            let mut expect_note = false;
+            let mut next_parent = actual_error.parent;
+            loop {
+                if poisoned_parents.contains(&next_parent) {
+                    expect_note = true;
+                    break;
+                }
+                match next_parent {
+                    Some(next) => next_parent = actual_errors[next].parent,
+                    None => break,
+                }
+            }
+
+            if self.is_unexpected_compiler_message(&actual_error, expect_help, expect_note) {
+                self.error(&format!(
+                    "{}:{}: unexpected {}: '{}'",
+                    file_name,
+                    actual_error.line_num_str(),
+                    actual_error.kind,
+                    actual_error.msg
+                ));
+                unexpected.push(actual_error);
             }
         }