extend doc comment for reachability set computation

Author: RalfJung

compiler/rustc_passes/src/reachable.rs

@@ -1,9 +1,13 @@
-// Finds items that are externally reachable, to determine which items
-// need to have their metadata (and possibly their AST) serialized.
-// All items that can be referred to through an exported name are
-// reachable, and when a reachable thing is inline or generic, it
-// makes all other generics or inline functions that it references
-// reachable as well.
+//! Finds local items that are externally reachable, to determine which items
+//! need to have their metadata (and possibly their AST) serialized.
+//!
+//! An item is "externally reachable" if it is relevant for other crates. This obviously includes
+//! all public items. However, when a reachable function `f` of this crate has its code generated as
+//! part of another crate's compilation, that other crate must be able to access everything needed
+//! to compile `f` -- in particular, every function called by `f`! Therefore, when a reachable thing
+//! is cross-crate inlinable or generic, it makes all other functions that it references reachable
+//! as well. Similarly, `const fn` may be evaluated by other crates, so everything they call is also
+//! "externally reachable". This is how private items can become "externally reachable".
 
 use hir::def_id::LocalDefIdSet;
 use rustc_data_structures::stack::ensure_sufficient_stack;
@@ -21,6 +25,9 @@ use rustc_privacy::DefIdVisitor;
 use rustc_session::config::CrateType;
 use rustc_target::spec::abi::Abi;
 
+/// Determines whether this function may be codegen'd or evaluated as part of another crate's
+/// compilation. In that case, things that show up in its body are relevant for other crates as
+/// well.
 fn item_might_be_inlined(tcx: TyCtxt<'_>, def_id: DefId) -> bool {
     tcx.generics_of(def_id).requires_monomorphization(tcx)
         || tcx.cross_crate_inlinable(def_id)
@@ -54,12 +61,20 @@ impl<'tcx> Visitor<'tcx> for ReachableContext<'tcx> {
     fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) {
         let res = match expr.kind {
             hir::ExprKind::Path(ref qpath) => {
+                // This covers fn ptr casts but also "non-method" calls.
                 Some(self.typeck_results().qpath_res(qpath, expr.hir_id))
             }
-            hir::ExprKind::MethodCall(..) => self
-                .typeck_results()
-                .type_dependent_def(expr.hir_id)
-                .map(|(kind, def_id)| Res::Def(kind, def_id)),
+            hir::ExprKind::MethodCall(..) => {
+                // Method calls don't involve a full "path", so we need to determine the callee
+                // based on the receiver type.
+                // If this is a method call on a generic type, we might not be able to find the
+                // callee. That's why `reachable_set` also adds all potential callees for such
+                // calls, i.e. all trait impl items, to the reachable set. So here we only worry
+                // about the calls we can identify.
+                self.typeck_results()
+                    .type_dependent_def(expr.hir_id)
+                    .map(|(kind, def_id)| Res::Def(kind, def_id))
+            }
             hir::ExprKind::Closure(&hir::Closure { def_id, .. }) => {
                 self.reachable_symbols.insert(def_id);
                 None
@@ -394,6 +409,7 @@ fn has_custom_linkage(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
         || codegen_attrs.flags.contains(CodegenFnAttrFlags::USED_LINKER)
 }
 
+/// See module-level doc comment above.
 fn reachable_set(tcx: TyCtxt<'_>, (): ()) -> LocalDefIdSet {
     let effective_visibilities = &tcx.effective_visibilities(());
 
@@ -427,10 +443,10 @@ fn reachable_set(tcx: TyCtxt<'_>, (): ()) -> LocalDefIdSet {
         }
     }
     {
-        // Some methods from non-exported (completely private) trait impls still have to be
-        // reachable if they are called from inlinable code. Generally, it's not known until
-        // monomorphization if a specific trait impl item can be reachable or not. So, we
-        // conservatively mark all of them as reachable.
+        // As explained above, we have to mark all functions called from reachable
+        // `item_might_be_inlined` items as reachable. The issue is, when those functions are
+        // generic and call a trait method, we have no idea where that call goes! So, we
+        // conservatively mark all trait impl items as reachable.
         // FIXME: One possible strategy for pruning the reachable set is to avoid marking impl
         // items of non-exported traits (or maybe all local traits?) unless their respective
         // trait items are used from inlinable code through method call syntax or UFCS, or their