Unimpl AllocValue for Option

Summary:
This impl has two drawbacks:
- there's no `UnpackValue` for `Option` (because `Option` in parameter indicates optional parameter)
- we may accidentally allocate `None` where we don't mean to, for example, when data is stored as `None` meaning "does not mean to go into starlark". One of such cases is D63708443, another time I accidentally almost did it in `dynamic_actions` refactoring (that diff is not published yet)

Reviewed By: JakobDegen

Differential Revision: D63717899

fbshipit-source-id: 19ae37c367be2ac0b1424cdc6dbfa734c3d1d1af

Author: stepancheg

starlark/src/stdlib/call_stack.rs

@@ -89,10 +89,10 @@ fn stack_frame_methods(builder: &mut MethodsBuilder) {
 
     /// Returns a path of the module from which the entry was called, or [`None`] for native Rust functions.
     #[starlark(attribute)]
-    fn module_path(this: &StackFrame) -> anyhow::Result<Option<String>> {
+    fn module_path(this: &StackFrame) -> anyhow::Result<NoneOr<String>> {
         match this.location {
-            Some(ref location) => Ok(Some(location.file.filename().to_owned())),
-            None => Ok(None),
+            Some(ref location) => Ok(NoneOr::Other(location.file.filename().to_owned())),
+            None => Ok(NoneOr::None),
         }
     }
 }

starlark/src/values/alloc_value.rs

@@ -96,18 +96,6 @@ impl<'v> AllocValue<'v> for Value<'v> {
     }
 }
 
-impl<'v, T> AllocValue<'v> for Option<T>
-where
-    T: AllocValue<'v>,
-{
-    fn alloc_value(self, heap: &'v Heap) -> Value<'v> {
-        match self {
-            Some(v) => v.alloc_value(heap),
-            None => Value::new_none(),
-        }
-    }
-}
-
 impl<'v, A: AllocValue<'v>, B: AllocValue<'v>> AllocValue<'v> for Either<A, B> {
     #[inline]
     fn alloc_value(self, heap: &'v Heap) -> Value<'v> {