Fix AttrResolutionContext lifetimes

Summary: AttrResolutionContext trait should have a lifetime because objects that implement it should live as long as the `Module` and `Heap` inside AttrResolutionContext. Reviewed By: stepancheg Differential Revision: D39348188 fbshipit-source-id: 185633ae4781fc7a34374bf785ab053a3658c5c0
facebook · Sep 8, 2022 · c4dbc53 · c4dbc53
1 parent 9076b59
commit c4dbc53
Show file tree

Hide file tree

Showing 13 changed files with 97 additions and 97 deletions.
diff --git a/buck2_build_api/src/analysis/mod.rs b/buck2_build_api/src/analysis/mod.rs
@@ -113,8 +113,8 @@ struct RuleAnalysisAttrResolutionContext<'v> {
     query_results: HashMap<String, Arc<AnalysisQueryResult>>,
 }
 
-impl<'v> AttrResolutionContext for RuleAnalysisAttrResolutionContext<'v> {
-    fn starlark_module(&self) -> &Module {
+impl<'v> AttrResolutionContext<'v> for RuleAnalysisAttrResolutionContext<'v> {
+    fn starlark_module(&self) -> &'v Module {
         self.module
     }
 

diff --git a/buck2_build_api/src/attrs/resolve/attr_literal.rs b/buck2_build_api/src/attrs/resolve/attr_literal.rs
@@ -96,9 +96,9 @@ impl UnconfiguredAttrLiteralExt for AttrLiteral<CoercedAttr> {
 }
 
 pub(crate) trait ConfiguredAttrLiteralExt {
-    fn resolve_single<'v>(&self, ctx: &'v dyn AttrResolutionContext) -> anyhow::Result<Value<'v>>;
+    fn resolve_single<'v>(&self, ctx: &dyn AttrResolutionContext<'v>) -> anyhow::Result<Value<'v>>;
 
-    fn resolve<'v>(&self, ctx: &'v dyn AttrResolutionContext) -> anyhow::Result<Vec<Value<'v>>>;
+    fn resolve<'v>(&self, ctx: &dyn AttrResolutionContext<'v>) -> anyhow::Result<Vec<Value<'v>>>;
 
     fn starlark_type(&self) -> anyhow::Result<&'static str>;
 
@@ -107,7 +107,7 @@ pub(crate) trait ConfiguredAttrLiteralExt {
 }
 
 impl ConfiguredAttrLiteralExt for AttrLiteral<ConfiguredAttr> {
-    fn resolve_single<'v>(&self, ctx: &'v dyn AttrResolutionContext) -> anyhow::Result<Value<'v>> {
+    fn resolve_single<'v>(&self, ctx: &dyn AttrResolutionContext<'v>) -> anyhow::Result<Value<'v>> {
         match self {
             AttrLiteral::Bool(v) => Ok(Value::new_bool(*v)),
             AttrLiteral::Int(v) => Ok(Value::new_int(*v)),
@@ -157,7 +157,7 @@ impl ConfiguredAttrLiteralExt for AttrLiteral<ConfiguredAttr> {
         }
     }
 
-    fn resolve<'v>(&self, ctx: &'v dyn AttrResolutionContext) -> anyhow::Result<Vec<Value<'v>>> {
+    fn resolve<'v>(&self, ctx: &dyn AttrResolutionContext<'v>) -> anyhow::Result<Vec<Value<'v>>> {
         match self {
             // SourceLabel is special since it is the only type that can be expand to many
             AttrLiteral::SourceLabel(src) => SourceAttrType::resolve_label(ctx, src),

diff --git a/buck2_build_api/src/attrs/resolve/attr_type/arg/mod.rs b/buck2_build_api/src/attrs/resolve/attr_type/arg/mod.rs
@@ -67,11 +67,11 @@ impl CommandLineBuilder for SpaceSeparatedCommandLineBuilder<'_> {
 }
 
 pub(crate) trait ConfiguredStringWithMacrosExt {
-    fn resolve<'v>(&self, ctx: &'v dyn AttrResolutionContext) -> anyhow::Result<Value<'v>>;
+    fn resolve<'v>(&self, ctx: &dyn AttrResolutionContext<'v>) -> anyhow::Result<Value<'v>>;
 }
 
 impl ConfiguredStringWithMacrosExt for ConfiguredStringWithMacros {
-    fn resolve<'v>(&self, ctx: &'v dyn AttrResolutionContext) -> anyhow::Result<Value<'v>> {
+    fn resolve<'v>(&self, ctx: &dyn AttrResolutionContext<'v>) -> anyhow::Result<Value<'v>> {
         ResolvedStringWithMacros::resolved(self, ctx)
     }
 }

diff --git a/buck2_build_api/src/attrs/resolve/attr_type/arg/value.rs b/buck2_build_api/src/attrs/resolve/attr_type/arg/value.rs
@@ -280,7 +280,7 @@ impl ResolvedStringWithMacros {
 
     pub(crate) fn resolved<'v>(
         configured_macros: &ConfiguredStringWithMacros,
-        ctx: &'v dyn AttrResolutionContext,
+        ctx: &dyn AttrResolutionContext<'v>,
     ) -> anyhow::Result<Value<'v>> {
         let resolved_parts = match configured_macros {
             ConfiguredStringWithMacros::StringPart(s) => {

diff --git a/buck2_build_api/src/attrs/resolve/attr_type/configuration_dep.rs b/buck2_build_api/src/attrs/resolve/attr_type/configuration_dep.rs
@@ -15,7 +15,7 @@ use crate::attrs::resolve::ctx::AttrResolutionContext;
 
 pub(crate) trait ConfigurationDepAttrTypeExt {
     fn resolve_single<'v>(
-        ctx: &'v dyn AttrResolutionContext,
+        ctx: &dyn AttrResolutionContext<'v>,
         label: &TargetLabel,
     ) -> anyhow::Result<Value<'v>> {
         Ok(ctx.heap().alloc(label.to_string()))

diff --git a/buck2_build_api/src/attrs/resolve/attr_type/dep.rs b/buck2_build_api/src/attrs/resolve/attr_type/dep.rs
@@ -45,13 +45,13 @@ pub(crate) trait DepAttrTypeExt {
     ) -> Value<'v>;
 
     fn resolve_single_impl<'v>(
-        ctx: &'v dyn AttrResolutionContext,
+        ctx: &dyn AttrResolutionContext<'v>,
         target: &ConfiguredProvidersLabel,
         required_providers: &Option<Arc<ProviderIdSet>>,
     ) -> anyhow::Result<Value<'v>>;
 
     fn resolve_single<'v>(
-        ctx: &'v dyn AttrResolutionContext,
+        ctx: &dyn AttrResolutionContext<'v>,
         dep_attr: &DepAttr<ConfiguredProvidersLabel>,
     ) -> anyhow::Result<Value<'v>>;
 }
@@ -87,7 +87,7 @@ impl DepAttrTypeExt for DepAttrType {
     }
 
     fn resolve_single_impl<'v>(
-        ctx: &'v dyn AttrResolutionContext,
+        ctx: &dyn AttrResolutionContext<'v>,
         target: &ConfiguredProvidersLabel,
         required_providers: &Option<Arc<ProviderIdSet>>,
     ) -> anyhow::Result<Value<'v>> {
@@ -101,7 +101,7 @@ impl DepAttrTypeExt for DepAttrType {
     }
 
     fn resolve_single<'v>(
-        ctx: &'v dyn AttrResolutionContext,
+        ctx: &dyn AttrResolutionContext<'v>,
         dep_attr: &DepAttr<ConfiguredProvidersLabel>,
     ) -> anyhow::Result<Value<'v>> {
         Self::resolve_single_impl(ctx, &dep_attr.label, &dep_attr.attr_type.required_providers)
@@ -110,7 +110,7 @@ impl DepAttrTypeExt for DepAttrType {
 
 pub(crate) trait ExplicitConfiguredDepAttrTypeExt {
     fn resolve_single<'v>(
-        ctx: &'v dyn AttrResolutionContext,
+        ctx: &dyn AttrResolutionContext<'v>,
         dep_attr: &ConfiguredExplicitConfiguredDep,
     ) -> anyhow::Result<Value<'v>> {
         DepAttrType::resolve_single_impl(

diff --git a/buck2_build_api/src/attrs/resolve/attr_type/query.rs b/buck2_build_api/src/attrs/resolve/attr_type/query.rs
@@ -34,11 +34,11 @@ impl ConfiguredQueryAttrBaseExt for QueryAttrBase<ConfiguredAttr> {
 }
 
 pub(crate) trait ConfiguredQueryAttrExt {
-    fn resolve<'v>(&self, ctx: &'v dyn AttrResolutionContext) -> anyhow::Result<Value<'v>>;
+    fn resolve<'v>(&self, ctx: &dyn AttrResolutionContext<'v>) -> anyhow::Result<Value<'v>>;
 }
 
 impl ConfiguredQueryAttrExt for QueryAttr<ConfiguredAttr> {
-    fn resolve<'v>(&self, ctx: &'v dyn AttrResolutionContext) -> anyhow::Result<Value<'v>> {
+    fn resolve<'v>(&self, ctx: &dyn AttrResolutionContext<'v>) -> anyhow::Result<Value<'v>> {
         let query_results = self.query.resolve(ctx)?;
         let mut dependencies = Vec::new();
 

diff --git a/buck2_build_api/src/attrs/resolve/attr_type/source.rs b/buck2_build_api/src/attrs/resolve/attr_type/source.rs
@@ -24,14 +24,14 @@ enum SourceLabelResolutionError {
 }
 
 pub(crate) trait SourceAttrTypeExt {
-    fn resolve_single_file<'v>(ctx: &'v dyn AttrResolutionContext, path: &BuckPath) -> Value<'v> {
+    fn resolve_single_file<'v>(ctx: &dyn AttrResolutionContext<'v>, path: &BuckPath) -> Value<'v> {
         ctx.heap().alloc(StarlarkArtifact::new(
             SourceArtifact::new(path.clone()).into(),
         ))
     }
 
     fn resolve_label<'v>(
-        ctx: &'v dyn AttrResolutionContext,
+        ctx: &dyn AttrResolutionContext<'v>,
         label: &ConfiguredProvidersLabel,
     ) -> anyhow::Result<Vec<Value<'v>>> {
         let dep = ctx.get_dep(label)?;
@@ -47,7 +47,7 @@ pub(crate) trait SourceAttrTypeExt {
     }
 
     fn resolve_single_label<'v>(
-        ctx: &'v dyn AttrResolutionContext,
+        ctx: &dyn AttrResolutionContext<'v>,
         value: &ConfiguredProvidersLabel,
     ) -> anyhow::Result<Value<'v>> {
         let mut resolved = Self::resolve_label(ctx, value)?;

diff --git a/buck2_build_api/src/attrs/resolve/attr_type/split_transition_dep.rs b/buck2_build_api/src/attrs/resolve/attr_type/split_transition_dep.rs
@@ -20,14 +20,14 @@ use crate::attrs::resolve::ctx::AttrResolutionContext;
 
 pub(crate) trait SplitTransitionDepAttrTypeExt {
     fn resolve_single<'v>(
-        ctx: &'v dyn AttrResolutionContext,
+        ctx: &dyn AttrResolutionContext<'v>,
         deps: &ConfiguredSplitTransitionDep,
     ) -> anyhow::Result<Value<'v>>;
 }
 
 impl SplitTransitionDepAttrTypeExt for SplitTransitionDepAttrType {
     fn resolve_single<'v>(
-        ctx: &'v dyn AttrResolutionContext,
+        ctx: &dyn AttrResolutionContext<'v>,
         deps: &ConfiguredSplitTransitionDep,
     ) -> anyhow::Result<Value<'v>> {
         let mut res = SmallMap::with_capacity(deps.deps.len());

diff --git a/buck2_build_api/src/attrs/resolve/configured_attr.rs b/buck2_build_api/src/attrs/resolve/configured_attr.rs
@@ -15,9 +15,9 @@ use crate::attrs::resolve::attr_literal::ConfiguredAttrLiteralExt;
 use crate::attrs::resolve::ctx::AttrResolutionContext;
 
 pub trait ConfiguredAttrExt {
-    fn resolve<'v>(&self, ctx: &'v dyn AttrResolutionContext) -> anyhow::Result<Vec<Value<'v>>>;
+    fn resolve<'v>(&self, ctx: &dyn AttrResolutionContext<'v>) -> anyhow::Result<Vec<Value<'v>>>;
 
-    fn resolve_single<'v>(&self, ctx: &'v dyn AttrResolutionContext) -> anyhow::Result<Value<'v>>;
+    fn resolve_single<'v>(&self, ctx: &dyn AttrResolutionContext<'v>) -> anyhow::Result<Value<'v>>;
 
     fn starlark_type(&self) -> anyhow::Result<&'static str>;
 
@@ -31,13 +31,13 @@ impl ConfiguredAttrExt for ConfiguredAttr {
     /// an inappropriate number of elements is returned. e.g. `attrs.list()` might
     /// accept and merge multiple returned values from `attrs.source()`, but
     /// `attrs.optional()` might only accept a single value, and fail otherwise.
-    fn resolve<'v>(&self, ctx: &'v dyn AttrResolutionContext) -> anyhow::Result<Vec<Value<'v>>> {
+    fn resolve<'v>(&self, ctx: &dyn AttrResolutionContext<'v>) -> anyhow::Result<Vec<Value<'v>>> {
         self.0.resolve(ctx)
     }
 
     /// Resolving a single value is common, so `resolve_single` will validate
     /// this function's output, and return a single value or an error.
-    fn resolve_single<'v>(&self, ctx: &'v dyn AttrResolutionContext) -> anyhow::Result<Value<'v>> {
+    fn resolve_single<'v>(&self, ctx: &dyn AttrResolutionContext<'v>) -> anyhow::Result<Value<'v>> {
         self.0.resolve_single(ctx)
     }
 

diff --git a/buck2_build_api/src/attrs/resolve/ctx.rs b/buck2_build_api/src/attrs/resolve/ctx.rs
@@ -23,10 +23,10 @@ pub type AnalysisQueryResult = Vec<(ConfiguredTargetLabel, FrozenProviderCollect
 
 /// The context for attribute resolution. Provides access to the providers from
 /// dependents.
-pub trait AttrResolutionContext {
-    fn starlark_module(&self) -> &Module;
+pub trait AttrResolutionContext<'v> {
+    fn starlark_module(&self) -> &'v Module;
 
-    fn heap(&self) -> &Heap {
+    fn heap(&self) -> &'v Heap {
         self.starlark_module().heap()
     }
 

diff --git a/buck2_build_api/src/attrs/resolve/testing.rs b/buck2_build_api/src/attrs/resolve/testing.rs
@@ -37,20 +37,21 @@ use crate::interpreter::rule_defs::provider::callable::ValueAsProviderCallableLi
 use crate::interpreter::rule_defs::provider::collection::FrozenProviderCollectionValue;
 use crate::interpreter::rule_defs::provider::registration::register_builtin_providers;
 
-pub(crate) fn resolution_ctx() -> impl AttrResolutionContext {
-    resolution_ctx_with_providers().0
+pub(crate) fn resolution_ctx<'v>(module: &'v Module) -> impl AttrResolutionContext<'v> {
+    resolution_ctx_with_providers(module).0
 }
 
-pub(crate) fn resolution_ctx_with_providers() -> (impl AttrResolutionContext, Vec<Arc<ProviderId>>)
-{
-    struct Ctx {
-        module: Module,
+pub(crate) fn resolution_ctx_with_providers<'v>(
+    module: &'v Module,
+) -> (impl AttrResolutionContext<'v>, Vec<Arc<ProviderId>>) {
+    struct Ctx<'v> {
+        module: &'v Module,
         // This module needs to be kept alive in order for the FrozenValues to stick around
         _deps_env: FrozenModule,
         deps: SmallMap<ConfiguredProvidersLabel, FrozenProviderCollectionValue>,
     }
 
-    impl Ctx {
+    impl<'v> Ctx<'v> {
         fn eval(env: &Module, globals: &Globals) {
             testing::to_value(
                 env,
@@ -60,46 +61,46 @@ pub(crate) fn resolution_ctx_with_providers() -> (impl AttrResolutionContext, Ve
                 // we do that and don't use lists of providers directly.
                 indoc!(
                     r#"
-            foo_a = source_artifact("", "default.cpp")
-            bar_a = source_artifact("", "bar.cpp")
-            bar1_a = source_artifact("", "bar1.cpp")
-            bar2_a = source_artifact("", "bar2.cpp")
-            bar3_a = source_artifact("", "bar3.cpp")
-            bar = DefaultInfo(default_outputs=[bar_a])
-            zero = DefaultInfo(default_outputs=[])
-            single = DefaultInfo(default_outputs=[bar1_a])
-            multiple = DefaultInfo(default_outputs=[bar1_a, bar2_a, bar3_a])
-            foo = DefaultInfo(
-                sub_targets={
-                    "bar": [bar],
-                    "zero": [zero],
-                    "single": [single],
-                    "multiple": [multiple],
-                    "foo_only": [FooInfo(foo="f1")],
-                    "foo_and_bar": [FooInfo(foo="f1"), BarInfo(bar="b1")],
-                },
-                default_outputs=[foo_a],
-            )
-            outer = DefaultInfo(sub_targets={
-                "foo": [foo],
-                "toolchain": [zero, TemplatePlaceholderInfo(unkeyed_variables = {"CXX": "clang++"})],
-                "keyed_placeholder": [zero, TemplatePlaceholderInfo(keyed_variables = {
-                    "user_key": "hello",
-                    "key_with_args": { "DEFAULT": "world", "big": "big world" },
-                })]
-            })
-            ret = {
-                "foo": outer.sub_targets["foo"],
-                "bar": foo.sub_targets["bar"],
-                "zero": foo.sub_targets["zero"],
-                "single": foo.sub_targets["single"],
-                "multiple": foo.sub_targets["multiple"],
-                "foo_only": foo.sub_targets["foo_only"],
-                "foo_and_bar": foo.sub_targets["foo_and_bar"],
-                "toolchain": outer.sub_targets["toolchain"],
-                "keyed_placeholder": outer.sub_targets["keyed_placeholder"],
-            }
-            "#
+             foo_a = source_artifact("", "default.cpp")
+             bar_a = source_artifact("", "bar.cpp")
+             bar1_a = source_artifact("", "bar1.cpp")
+             bar2_a = source_artifact("", "bar2.cpp")
+             bar3_a = source_artifact("", "bar3.cpp")
+             bar = DefaultInfo(default_outputs=[bar_a])
+             zero = DefaultInfo(default_outputs=[])
+             single = DefaultInfo(default_outputs=[bar1_a])
+             multiple = DefaultInfo(default_outputs=[bar1_a, bar2_a, bar3_a])
+             foo = DefaultInfo(
+                 sub_targets={
+                     "bar": [bar],
+                     "zero": [zero],
+                     "single": [single],
+                     "multiple": [multiple],
+                     "foo_only": [FooInfo(foo="f1")],
+                     "foo_and_bar": [FooInfo(foo="f1"), BarInfo(bar="b1")],
+                 },
+                 default_outputs=[foo_a],
+             )
+             outer = DefaultInfo(sub_targets={
+                 "foo": [foo],
+                 "toolchain": [zero, TemplatePlaceholderInfo(unkeyed_variables = {"CXX": "clang++"})],
+                 "keyed_placeholder": [zero, TemplatePlaceholderInfo(keyed_variables = {
+                     "user_key": "hello",
+                     "key_with_args": { "DEFAULT": "world", "big": "big world" },
+                 })]
+             })
+             ret = {
+                 "foo": outer.sub_targets["foo"],
+                 "bar": foo.sub_targets["bar"],
+                 "zero": foo.sub_targets["zero"],
+                 "single": foo.sub_targets["single"],
+                 "multiple": foo.sub_targets["multiple"],
+                 "foo_only": foo.sub_targets["foo_only"],
+                 "foo_and_bar": foo.sub_targets["foo_and_bar"],
+                 "toolchain": outer.sub_targets["toolchain"],
+                 "keyed_placeholder": outer.sub_targets["keyed_placeholder"],
+             }
+             "#
                 ),
             );
         }
@@ -121,10 +122,10 @@ pub(crate) fn resolution_ctx_with_providers() -> (impl AttrResolutionContext, Ve
             let provider_env = Module::new();
             let provider_content = indoc!(
                 r#"
-                FooInfo = provider(fields=["foo"])
-                BarInfo = provider(fields=["bar"])
-                None
-                "#
+                 FooInfo = provider(fields=["foo"])
+                 BarInfo = provider(fields=["bar"])
+                 None
+                 "#
             );
             testing::to_value(&provider_env, &globals, provider_content);
             let frozen_provider_env = provider_env
@@ -206,9 +207,9 @@ pub(crate) fn resolution_ctx_with_providers() -> (impl AttrResolutionContext, Ve
         }
     }
 
-    impl AttrResolutionContext for Ctx {
-        fn starlark_module(&self) -> &Module {
-            &self.module
+    impl<'v> AttrResolutionContext<'v> for Ctx<'v> {
+        fn starlark_module(&self) -> &'v Module {
+            self.module
         }
 
         fn get_dep(
@@ -242,7 +243,6 @@ pub(crate) fn resolution_ctx_with_providers() -> (impl AttrResolutionContext, Ve
         }
     }
     let (deps_env, deps, provider_ids) = Ctx::simple_deps();
-    let module = Module::new();
     (
         Ctx {
             module,