Prefer an enum over Option<Option<Result<T>>>

Author: oli-obk

compiler/rustc_interface/src/queries.rs

@@ -30,66 +30,57 @@ use std::sync::Arc;
 /// [`take`]: Self::take
 /// [`compute`]: Self::compute
 pub struct Query<T> {
-    // The outer option is `None` until the query is computed.
-    // Then the whole thing becomes `Some(Some(result))`.
-    // Once the value gets taken/stolen, it is `Some(None)`.
-    result: RefCell<Option<Option<Result<T>>>>,
+    result: RefCell<Result<State<T>>>,
+}
+
+enum State<T> {
+    NotComputedYet,
+    Computed(T),
+    Stolen,
 }
 
 impl<T> Query<T> {
     fn compute<F: FnOnce() -> Result<T>>(&self, f: F) -> Result<&Query<T>> {
-        self.result
-            .borrow_mut()
-            .get_or_insert_with(|| Some(f()))
-            .as_ref()
-            .expect("query already taken")
-            .as_ref()
-            .map(|_| self)
-            .map_err(|&err| err)
+        let mut result = self.result.borrow_mut();
+        if matches!(*result, Ok(State::NotComputedYet)) {
+            *result = f().map(State::Computed);
+        }
+        result.as_ref().map(|_| self).map_err(|e| *e)
     }
 
     /// Takes ownership of the query result. Further attempts to take or peek the query
     /// result will panic unless it is generated by calling the `compute` method.
     pub fn take(&self) -> T {
-        self.result
-            .borrow_mut()
-            .as_mut()
-            .expect("query never computed")
-            .take()
-            .expect("query already taken")
-            .unwrap()
+        match std::mem::replace(&mut *self.result.borrow_mut(), Ok(State::Stolen)).unwrap() {
+            State::NotComputedYet => panic!("query never computed"),
+            State::Computed(val) => val,
+            State::Stolen => panic!("query already taken"),
+        }
     }
 
     /// Borrows the query result using the RefCell. Panics if the result is stolen.
     pub fn peek(&self) -> Ref<'_, T> {
-        Ref::map(self.result.borrow(), |r| {
-            r.as_ref()
-                .unwrap()
-                .as_ref()
-                .expect("query never computed")
-                .as_ref()
-                .expect("query already taken")
+        Ref::map(self.result.borrow(), |r| match r.as_ref().unwrap() {
+            State::NotComputedYet => panic!("query never computed"),
+            State::Computed(val) => val,
+            State::Stolen => panic!("query already taken"),
         })
     }
 }
 
 impl<'tcx> Query<QueryContext<'tcx>> {
     pub fn enter<T>(&self, f: impl FnOnce(TyCtxt<'tcx>) -> T) -> T {
-        self.result
-            .borrow_mut()
-            .as_mut()
-            .unwrap()
-            .as_mut()
-            .expect("query never computed")
-            .as_mut()
-            .expect("query already taken")
-            .enter(f)
+        match self.result.borrow_mut().as_mut().unwrap() {
+            State::NotComputedYet => panic!("query never computed"),
+            State::Computed(qcx) => qcx.enter(f),
+            State::Stolen => panic!("query already taken"),
+        }
     }
 }
 
 impl<T> Default for Query<T> {
     fn default() -> Self {
-        Query { result: RefCell::new(None) }
+        Query { result: RefCell::new(Ok(State::NotComputedYet)) }
     }
 }
 
@@ -407,7 +398,7 @@ impl Compiler {
 
         // NOTE: intentionally does not compute the global context if it hasn't been built yet,
         // since that likely means there was a parse error.
-        if let Some(Some(Ok(gcx))) = &mut *queries.global_ctxt.result.borrow_mut() {
+        if let Ok(State::Computed(gcx)) = &mut *queries.global_ctxt.result.borrow_mut() {
             // We assume that no queries are run past here. If there are new queries
             // after this point, they'll show up as "<unknown>" in self-profiling data.
             {