misc cleanup suggestions

crates/compiler/derive/src/inspect.rs

@@ -111,8 +111,6 @@ pub(crate) fn derive_to_inspector(
 
 fn to_inspector_list(env: &mut Env<'_>, fn_name: Symbol) -> (Expr, Variable) {
     // Build \lst -> list, List.walk, (\elem -> Inspect.toInspector elem)
-    //
-    // TODO eta reduce this baby       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
     use Expr::*;
 
@@ -954,11 +952,6 @@ fn to_inspector_tag_union(
 }
 
 /// Lift `inspector` to `Inspect.custom \fmt -> Inspect.apply inspector fmt`
-///
-/// TODO: currently it appears that just `inspector` is not isomorphic to the lift, on the
-/// monomorphization level, even though we would think it is. In particular, unspecialized lambda
-/// sets fail to resolve when we use the non-lifted version.
-/// More investigation is needed to figure out why.
 fn wrap_in_inspect_custom(
     env: &mut Env,
     inspector: Expr,

crates/compiler/derive_key/src/inspect.rs

@@ -205,7 +205,6 @@ impl FlatInspectable {
             Symbol::NUM_F32 | Symbol::NUM_BINARY32 => Some(Immediate(Symbol::INSPECT_F32)),
             Symbol::NUM_F64 | Symbol::NUM_BINARY64 => Some(Immediate(Symbol::INSPECT_F64)),
             Symbol::NUM_NAT | Symbol::NUM_NATURAL => Some(Immediate(Symbol::INSPECT_NAT)),
-            // TODO List, Dict, Set, etc.
             _ => None,
         }
     }

crates/compiler/solve/src/ability.rs

@@ -381,7 +381,6 @@ impl ObligationCache {
         let ImplKey { opaque, ability } = impl_key;
 
         // Every type has the Inspect ability automatically, even opaques with no `implements` declaration.
-        // (Those opaques get a default implementation that returns something along the lines of "<opaque>")
         let is_inspect = ability == Symbol::INSPECT_INSPECT_ABILITY;
         let has_known_impl =
             is_inspect || abilities_store.has_declared_implementation(opaque, ability);
@@ -865,9 +864,8 @@ impl DerivableVisitor for DeriveInspect {
     const ABILITY_SLICE: SubsSlice<Symbol> = Subs::AB_INSPECT;
 
     #[inline(always)]
-    fn is_derivable_builtin_opaque(symbol: Symbol) -> bool {
-        // TODO: Should this just be true? All values are always inspectable.
-        is_builtin_number_alias(symbol) || is_builtin_bool_alias(symbol)
+    fn is_derivable_builtin_opaque(_: Symbol) -> bool {
+        true
     }
 
     #[inline(always)]
@@ -876,18 +874,8 @@ impl DerivableVisitor for DeriveInspect {
     }
 
     #[inline(always)]
-    fn visit_apply(var: Variable, symbol: Symbol) -> Result<Descend, NotDerivable> {
-        if matches!(
-            symbol,
-            Symbol::LIST_LIST | Symbol::SET_SET | Symbol::DICT_DICT | Symbol::STR_STR,
-        ) {
-            Ok(Descend(true))
-        } else {
-            Err(NotDerivable {
-                var,
-                context: NotDerivableContext::NoContext,
-            })
-        }
+    fn visit_apply(_: Variable, _: Symbol) -> Result<Descend, NotDerivable> {
+        Ok(Descend(true))
     }
 
     #[inline(always)]

crates/compiler/solve/src/specialize.rs

@@ -628,38 +628,7 @@ fn make_specialization_decision<P: Phase>(
             } else {
                 // Solving within a module.
                 phase.with_module_abilities_store(opaque.module_id(), |abilities_store| {
-                    let impl_key = ImplKey {
-                        opaque: *opaque,
-                        ability_member,
-                    };
-                    match abilities_store.get_implementation(impl_key) {
-                        None => {
-                            match ability_member {
-                                // Inspect is special - if there is no implementation for the
-                                // opaque type, we always emit a default implementation.
-                                Symbol::INSPECT_TO_INSPECTOR => SpecializeDecision::Specialize(
-                                    Immediate(Symbol::INSPECT_OPAQUE),
-                                ),
-                                _ => {
-                                    // Doesn't specialize; an error will already be reported for this.
-                                    SpecializeDecision::Drop
-                                }
-                            }
-                        }
-                        Some(MemberImpl::Error) => {
-                            // TODO: probably not right, we may want to choose a derive decision!
-                            SpecializeDecision::Specialize(Opaque(*opaque))
-                        }
-                        Some(MemberImpl::Impl(specialization_symbol)) => {
-                            match abilities_store.specialization_info(*specialization_symbol) {
-                                Some(_) => SpecializeDecision::Specialize(Opaque(*opaque)),
-
-                                // If we expect a specialization impl but don't yet know it, we must hold off
-                                // compacting the lambda set until the specialization is well-known.
-                                None => SpecializeDecision::PendingSpecialization(impl_key),
-                            }
-                        }
-                    }
+                    make_ability_specialization_decision(*opaque, ability_member, abilities_store)
                 })
             }
         }
@@ -707,6 +676,46 @@ fn make_specialization_decision<P: Phase>(
     }
 }
 
+fn make_ability_specialization_decision(
+    opaque: Symbol,
+    ability_member: Symbol,
+    abilities_store: &AbilitiesStore,
+) -> SpecializeDecision {
+    use SpecializationTypeKey::*;
+    let impl_key = ImplKey {
+        opaque,
+        ability_member,
+    };
+    match abilities_store.get_implementation(impl_key) {
+        None => {
+            match ability_member {
+                // Inspect is special - if there is no implementation for the
+                // opaque type, we always emit a default implementation.
+                Symbol::INSPECT_TO_INSPECTOR => {
+                    SpecializeDecision::Specialize(Immediate(Symbol::INSPECT_OPAQUE))
+                }
+                _ => {
+                    // Doesn't specialize; an error will already be reported for this.
+                    SpecializeDecision::Drop
+                }
+            }
+        }
+        Some(MemberImpl::Error) => {
+            // TODO: probably not right, we may want to choose a derive decision!
+            SpecializeDecision::Specialize(Opaque(opaque))
+        }
+        Some(MemberImpl::Impl(specialization_symbol)) => {
+            match abilities_store.specialization_info(*specialization_symbol) {
+                Some(_) => SpecializeDecision::Specialize(Opaque(opaque)),
+
+                // If we expect a specialization impl but don't yet know it, we must hold off
+                // compacting the lambda set until the specialization is well-known.
+                None => SpecializeDecision::PendingSpecialization(impl_key),
+            }
+        }
+    }
+}
+
 #[allow(clippy::too_many_arguments)]
 fn get_specialization_lambda_set_ambient_function<P: Phase>(
     subs: &mut Subs,

crates/compiler/types/src/subs.rs

@@ -1748,7 +1748,7 @@ impl Subs {
         tag_names.push(TagName("OutOfBounds".into()));
         // END INIT-TagNames
 
-        // IFTTT INIT-SymbolNames
+        // IFTTT INIT-SymbolSubsSlice
         let mut symbol_names = Vec::with_capacity(32);
 
         symbol_names.push(Symbol::ENCODE_ENCODING);
@@ -1757,7 +1757,7 @@ impl Subs {
         symbol_names.push(Symbol::HASH_HASH_ABILITY);
         symbol_names.push(Symbol::BOOL_EQ);
         symbol_names.push(Symbol::INSPECT_INSPECT_ABILITY);
-        // END INIT-SymbolNames
+        // END INIT-SymbolSubsSlice
 
         // IFTTT INIT-VariableSubsSlice
         let variables = vec![Variable::STR];