Auto merge of #44071 - alexcrichton:no-cycles, r=nikomatsakis

rustc: Start moving toward "try_get is a bug" for incremental

This PR is an effort to burn down some of the work items on #42633. The basic change here was to leave the `try_get` function exposed but have it return a `DiagnosticBuilder` instead of a `CycleError`. This means that it should be a compiler bug to *not* handle the error as dropping a diagnostic should result in a complier panic.

After that change it was then necessary to update the compiler's callsites of `try_get` to handle the error coming out. These were handled as:

* The `sized_constraint` and `needs_drop_raw` checks take the diagnostic and defer it as a compiler bug. This was a new piece of functionality added to the error handling infrastructure, and the idea is that for both these checks a "real" compiler error should be emitted elsewhere, so it's only a bug if we don't actually emit the complier error elsewhere.
* MIR inlining was updated to just ignore the diagnostic. This is being tracked by #43542 which sounded like it either already had some work underway or was planning to change regardless.
* The final case, `item_path`, is still sort of up for debate. At the time of this writing this PR simply removes the invocations of `try_get` there, assuming that the query will always succeed. This turns out to be true for the test suite anyway! It sounds like, though, that this logic was intended to assist in "weird" situations like `RUST_LOG` where debug implementations can trigger at any time. This PR would therefore, however, break those implementations.

I'm unfortunately sort of out of ideas on how to handle `item_path`, but other thoughts would be welcome!

Closes #42633

Author: bors

src/librustc/ty/item_path.rs

@@ -13,7 +13,6 @@ use hir::def_id::{CrateNum, DefId, CRATE_DEF_INDEX, LOCAL_CRATE};
 use ty::{self, Ty, TyCtxt};
 use syntax::ast;
 use syntax::symbol::Symbol;
-use syntax_pos::DUMMY_SP;
 
 use std::cell::Cell;
 
@@ -222,11 +221,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
         let use_types = !self.is_default_impl(impl_def_id) && (!impl_def_id.is_local() || {
             // Otherwise, use filename/line-number if forced.
             let force_no_types = FORCE_IMPL_FILENAME_LINE.with(|f| f.get());
-            !force_no_types && {
-                // Otherwise, use types if we can query them without inducing a cycle.
-                ty::queries::impl_trait_ref::try_get(self, DUMMY_SP, impl_def_id).is_ok() &&
-                    ty::queries::type_of::try_get(self, DUMMY_SP, impl_def_id).is_ok()
-            }
+            !force_no_types
         });
 
         if !use_types {

src/librustc/ty/maps.rs

@@ -212,13 +212,15 @@ impl<M: QueryDescription> QueryMap<M> {
     }
 }
 
-pub struct CycleError<'a, 'tcx: 'a> {
+struct CycleError<'a, 'tcx: 'a> {
     span: Span,
     cycle: RefMut<'a, [(Span, Query<'tcx>)]>,
 }
 
 impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
-    pub fn report_cycle(self, CycleError { span, cycle }: CycleError) {
+    fn report_cycle(self, CycleError { span, cycle }: CycleError)
+        -> DiagnosticBuilder<'a>
+    {
         // Subtle: release the refcell lock before invoking `describe()`
         // below by dropping `cycle`.
         let stack = cycle.to_vec();
@@ -247,8 +249,8 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
             err.note(&format!("...which then again requires {}, completing the cycle.",
                               stack[0].1.describe(self)));
 
-            err.emit();
-        });
+            return err
+        })
     }
 
     fn cycle_check<F, R>(self, span: Span, query: Query<'gcx>, compute: F)
@@ -704,8 +706,11 @@ macro_rules! define_maps {
             }
 
             pub fn try_get(tcx: TyCtxt<'a, $tcx, 'lcx>, span: Span, key: $K)
-                           -> Result<$V, CycleError<'a, $tcx>> {
-                Self::try_get_with(tcx, span, key, Clone::clone)
+                           -> Result<$V, DiagnosticBuilder<'a>> {
+                match Self::try_get_with(tcx, span, key, Clone::clone) {
+                    Ok(e) => Ok(e),
+                    Err(e) => Err(tcx.report_cycle(e)),
+                }
             }
 
             pub fn force(tcx: TyCtxt<'a, $tcx, 'lcx>, span: Span, key: $K) {
@@ -714,7 +719,7 @@ macro_rules! define_maps {
 
                 match Self::try_get_with(tcx, span, key, |_| ()) {
                     Ok(()) => {}
-                    Err(e) => tcx.report_cycle(e)
+                    Err(e) => tcx.report_cycle(e).emit(),
                 }
             }
         })*
@@ -751,8 +756,8 @@ macro_rules! define_maps {
         impl<'a, $tcx, 'lcx> TyCtxtAt<'a, $tcx, 'lcx> {
             $($(#[$attr])*
             pub fn $name(self, key: $K) -> $V {
-                queries::$name::try_get(self.tcx, self.span, key).unwrap_or_else(|e| {
-                    self.report_cycle(e);
+                queries::$name::try_get(self.tcx, self.span, key).unwrap_or_else(|mut e| {
+                    e.emit();
                     Value::from_cycle_error(self.global_tcx())
                 })
             })*

src/librustc/ty/mod.rs

@@ -1684,12 +1684,15 @@ impl<'a, 'gcx, 'tcx> AdtDef {
     pub fn sized_constraint(&self, tcx: TyCtxt<'a, 'gcx, 'tcx>) -> &'tcx [Ty<'tcx>] {
         match queries::adt_sized_constraint::try_get(tcx, DUMMY_SP, self.did) {
             Ok(tys) => tys,
-            Err(_) => {
+            Err(mut bug) => {
                 debug!("adt_sized_constraint: {:?} is recursive", self);
                 // This should be reported as an error by `check_representable`.
                 //
                 // Consider the type as Sized in the meanwhile to avoid
-                // further errors.
+                // further errors. Delay our `bug` diagnostic here to get
+                // emitted later as well in case we accidentally otherwise don't
+                // emit an error.
+                bug.delay_as_bug();
                 tcx.intern_type_list(&[tcx.types.err])
             }
         }

src/librustc/ty/util.rs

@@ -1069,11 +1069,15 @@ fn needs_drop_raw<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
     let needs_drop = |ty: Ty<'tcx>| -> bool {
         match ty::queries::needs_drop_raw::try_get(tcx, DUMMY_SP, param_env.and(ty)) {
             Ok(v) => v,
-            Err(_) => {
+            Err(mut bug) => {
                 // Cycles should be reported as an error by `check_representable`.
                 //
-                // Consider the type as not needing drop in the meanwhile to avoid
-                // further errors.
+                // Consider the type as not needing drop in the meanwhile to
+                // avoid further errors.
+                //
+                // In case we forgot to emit a bug elsewhere, delay our
+                // diagnostic to get emitted as a compiler bug.
+                bug.delay_as_bug();
                 false
             }
         }

src/librustc_errors/diagnostic_builder.rs

@@ -110,6 +110,22 @@ impl<'a> DiagnosticBuilder<'a> {
         // }
     }
 
+    /// Delay emission of this diagnostic as a bug.
+    ///
+    /// This can be useful in contexts where an error indicates a bug but
+    /// typically this only happens when other compilation errors have already
+    /// happened. In those cases this can be used to defer emission of this
+    /// diagnostic as a bug in the compiler only if no other errors have been
+    /// emitted.
+    ///
+    /// In the meantime, though, callsites are required to deal with the "bug"
+    /// locally in whichever way makes the most sense.
+    pub fn delay_as_bug(&mut self) {
+        self.level = Level::Bug;
+        *self.handler.delayed_span_bug.borrow_mut() = Some(self.diagnostic.clone());
+        self.cancel();
+    }
+
     /// Add a span/label to be included in the resulting snippet.
     /// This is pushed onto the `MultiSpan` that was created when the
     /// diagnostic was first built. If you don't call this function at
@@ -182,8 +198,10 @@ impl<'a> DiagnosticBuilder<'a> {
         DiagnosticBuilder::new_diagnostic(handler, diagnostic)
     }
 
-    /// Creates a new `DiagnosticBuilder` with an already constructed diagnostic.
-    pub fn new_diagnostic(handler: &'a Handler, diagnostic: Diagnostic) -> DiagnosticBuilder<'a> {
+    /// Creates a new `DiagnosticBuilder` with an already constructed
+    /// diagnostic.
+    pub fn new_diagnostic(handler: &'a Handler, diagnostic: Diagnostic)
+                         -> DiagnosticBuilder<'a> {
         DiagnosticBuilder { handler, diagnostic }
     }
 }

src/librustc_errors/lib.rs

@@ -272,7 +272,7 @@ pub struct Handler {
     pub can_emit_warnings: bool,
     treat_err_as_bug: bool,
     continue_after_error: Cell<bool>,
-    delayed_span_bug: RefCell<Option<(MultiSpan, String)>>,
+    delayed_span_bug: RefCell<Option<Diagnostic>>,
     tracked_diagnostics: RefCell<Option<Vec<Diagnostic>>>,
 }
 
@@ -439,8 +439,9 @@ impl Handler {
         if self.treat_err_as_bug {
             self.span_bug(sp, msg);
         }
-        let mut delayed = self.delayed_span_bug.borrow_mut();
-        *delayed = Some((sp.into(), msg.to_string()));
+        let mut diagnostic = Diagnostic::new(Level::Bug, msg);
+        diagnostic.set_span(sp.into());
+        *self.delayed_span_bug.borrow_mut() = Some(diagnostic);
     }
     pub fn span_bug_no_panic<S: Into<MultiSpan>>(&self, sp: S, msg: &str) {
         self.emit(&sp.into(), msg, Bug);
@@ -507,14 +508,9 @@ impl Handler {
         let s;
         match self.err_count.get() {
             0 => {
-                let delayed_bug = self.delayed_span_bug.borrow();
-                match *delayed_bug {
-                    Some((ref span, ref errmsg)) => {
-                        self.span_bug(span.clone(), errmsg);
-                    }
-                    _ => {}
+                if let Some(bug) = self.delayed_span_bug.borrow_mut().take() {
+                    DiagnosticBuilder::new_diagnostic(self, bug).emit();
                 }
-
                 return;
             }
             1 => s = "aborting due to previous error".to_string(),

src/librustc_mir/transform/inline.rs

@@ -115,8 +115,13 @@ impl<'a, 'tcx> Inliner<'a, 'tcx> {
                     Ok(ref callee_mir) if self.should_inline(callsite, callee_mir) => {
                         callee_mir.subst(self.tcx, callsite.substs)
                     }
+                    Ok(_) => continue,
 
-                    _ => continue,
+                    Err(mut bug) => {
+                        // FIXME(#43542) shouldn't have to cancel an error
+                        bug.cancel();
+                        continue
+                    }
                 };
 
                 let start = caller_mir.basic_blocks().len();