Add label and toolchain method to TargetRef trait. This requires a significant refactor to testutils, so while I was at it I refactored the API of testutils to make it much cleaner. In particular, we now store Rc<Session> instead of Session, allowing us to persist a session across multiple steps of evaluation, just like real scenarios (rule definition -> rule impl). Bug: 528225104 Change-Id: Ibe57f137774a85477f889fff09f499436a6a6964 Reviewed-on: https://gn-review.googlesource.com/c/gn/+/24221 Reviewed-by: Takuto Ikuta <tikuta@google.com> Commit-Queue: Matt Stark <msta@google.com>
diff --git a/src/gn/starlark/crates/attr/src/ctx.rs b/src/gn/starlark/crates/attr/src/ctx.rs index dff6652..f82650a 100644 --- a/src/gn/starlark/crates/attr/src/ctx.rs +++ b/src/gn/starlark/crates/attr/src/ctx.rs
@@ -167,7 +167,7 @@ mod tests { use starlark::{environment::GlobalsBuilder, eval::Evaluator, values::list::UnpackList}; use starlark_derive::starlark_module; - use testutils::{FakeEvalContext, FakeTarget, FakeTargetRef}; + use testutils::{FakeEvalContext, FakeTarget}; use types::{EvaluatorContextExt as _, File, Label, PackageRef}; use super::*; @@ -215,7 +215,7 @@ ) .create_ctx_fields( &fields, - &context.session, + context.session.as_ref(), &context.current_toolchain.as_ref(), None, Vec::new(), @@ -232,11 +232,11 @@ let target_label = Label::new(PackageRef::root().to_owned(), "bar".to_owned()); let file1 = File::intern("out.cc"); - let target_bar = FakeTargetRef::new(FakeTarget { + a.session().insert_target(FakeTarget { outputs: vec![file1.clone()], - ..Default::default() + ..a.session() + .empty_target(target_label.package(), target_label.name()) }); - a.context().session.insert_target(target_label, target_bar); a.modify_globals(|builder| { builder.set("attr", AttrModule { make_attr_schema });
diff --git a/src/gn/starlark/crates/attr/src/value.rs b/src/gn/starlark/crates/attr/src/value.rs index acdf29c..a8075ae 100644 --- a/src/gn/starlark/crates/attr/src/value.rs +++ b/src/gn/starlark/crates/attr/src/value.rs
@@ -239,7 +239,7 @@ values::{list::UnpackList, UnpackValue as _, ValueLike as _}, }; use testutils::{FakeSession, FakeTarget, FakeTargetRef}; - use types::{Label, PackageRef}; + use types::PackageRef; use super::*; use crate::{ @@ -280,10 +280,13 @@ #[test] fn test_to_value_label_no_files() { let session = FakeSession::new(); - let target_label = Label::new( - PackageRef::new("//foo").unwrap().to_owned(), - "bar".to_owned(), - ); + let file1 = File::new("foo.txt"); + + let target_empty = session.insert_empty_target(PackageRef::new("//foo").unwrap(), "empty"); + let target_outputs = session.insert_target(FakeTarget { + outputs: vec![file1.clone()], + ..session.empty_target(PackageRef::new("//foo").unwrap(), "outputs") + }); let schema = AttrSchema { kind: AttrKind::Label, @@ -296,15 +299,17 @@ Module::with_temp_heap(|module| { let heap = module.heap(); - let AttrValue { attr, file, files } = - Attr::Label(Some(crate::LabelOrFile::Label(target_label.clone()))) - .to_value( - &schema, - &session, - &session.default_toolchain.as_ref(), - &heap, - ) - .unwrap(); + + let AttrValue { attr, file, files } = Attr::Label(Some(crate::LabelOrFile::Label( + target_empty.label().to_owned(), + ))) + .to_value( + &schema, + &session, + &session.default_toolchain.as_ref(), + &heap, + ) + .unwrap(); // The resolved value should be the Target object let resolved_target = attr.downcast_ref::<FakeTargetRef>().unwrap(); @@ -321,24 +326,16 @@ // Target has outputs -> they should be collected to files even when allow_files // is None. - let file1 = File::new("foo.txt"); - session.insert_target( - target_label.clone(), - FakeTargetRef::new(FakeTarget { - outputs: vec![file1.clone()], - ..Default::default() - }), - ); - - let AttrValue { files, file, .. } = - Attr::Label(Some(crate::LabelOrFile::Label(target_label.clone()))) - .to_value( - &schema, - &session, - &session.default_toolchain.as_ref(), - &heap, - ) - .unwrap(); + let AttrValue { files, file, .. } = Attr::Label(Some(crate::LabelOrFile::Label( + target_outputs.label().to_owned(), + ))) + .to_value( + &schema, + &session, + &session.default_toolchain.as_ref(), + &heap, + ) + .unwrap(); assert_eq!( UnpackList::<&File>::unpack_value_err(files.unwrap()) @@ -353,20 +350,15 @@ #[test] fn test_to_value_label_allow_files_many() { let session = FakeSession::new(); - let target_label = Label::new( - PackageRef::new("//foo").unwrap().to_owned(), - "bar".to_owned(), - ); let label_only_file = File::new("label_only.cc"); let overlap = File::new("overlap.cc"); let file_only_file = File::new("file_only.cc"); // Target outputs out.cc and overlap.h - let dep = FakeTargetRef::new(FakeTarget { + let dep = session.insert_target(FakeTarget { outputs: vec![label_only_file.clone(), overlap.clone()], - ..Default::default() + ..session.empty_target(PackageRef::new("//foo").unwrap(), "dep") }); - session.insert_target(target_label.clone(), dep.clone()); let schema = AttrSchema { kind: AttrKind::LabelList, @@ -380,7 +372,7 @@ Module::with_temp_heap(|module| { let heap = module.heap(); let AttrValue { attr, file, files } = Attr::LabelList(vec![ - crate::LabelOrFile::Label(target_label.clone()), + crate::LabelOrFile::Label(dep.label().to_owned()), crate::LabelOrFile::File(file_only_file.clone()), crate::LabelOrFile::File(overlap.clone()), ]) @@ -419,12 +411,18 @@ #[test] fn test_to_value_label_allow_files_single() { let session = FakeSession::new(); - let target_label = Label::new( - PackageRef::new("//foo").unwrap().to_owned(), - "bar".to_owned(), - ); let file1 = File::new("out.cc"); + let target_single = session.insert_target(FakeTarget { + outputs: vec![file1.clone()], + ..session.empty_target(PackageRef::new("//foo").unwrap(), "single") + }); + let target_two = session.insert_target(FakeTarget { + outputs: vec![file1.clone(), File::new("out.h")], + ..session.empty_target(PackageRef::new("//foo").unwrap(), "two") + }); + let target_empty = session.insert_empty_target(PackageRef::new("//foo").unwrap(), "empty"); + let schema = AttrSchema { kind: AttrKind::Label, default: None, @@ -438,16 +436,8 @@ let heap = module.heap(); // Target has exactly 1 output file -> succeeds - session.insert_target( - target_label.clone(), - FakeTargetRef::new(FakeTarget { - outputs: vec![file1.clone()], - ..Default::default() - }), - ); - let path_resolver = types::PathResolver::new_for_testing(); - let starlark_val = heap.alloc(":bar"); + let starlark_val = heap.alloc(":single"); let attr = Attr::create( &schema, Some(starlark_val), @@ -456,7 +446,7 @@ ) .unwrap(); - let source_target = FakeTargetRef::default(); + let source_target = session.insert_empty_target(PackageRef::root(), "source"); attr.register_dependencies( &session, source_target.clone(), @@ -465,7 +455,10 @@ assert_eq!( source_target.registered_deps(), - HashSet::from([(target_label.clone(), session.default_toolchain.clone())]) + HashSet::from([( + target_single.label().to_owned(), + session.default_toolchain.clone() + )]) ); let AttrValue { @@ -521,15 +514,10 @@ ); // Target has 2 outputs -> fails - session.insert_target( - target_label.clone(), - FakeTargetRef::new(FakeTarget { - outputs: vec![file1.clone(), File::new("out.h")], - ..Default::default() - }), - ); - - let res = Attr::Label(Some(crate::LabelOrFile::Label(target_label.clone()))).to_value( + let res = Attr::Label(Some(crate::LabelOrFile::Label( + target_two.label().to_owned(), + ))) + .to_value( &schema, &session, &session.default_toolchain.as_ref(), @@ -537,28 +525,22 @@ ); assert_eq!( res.unwrap_err().to_string(), - "target `//foo:bar` must produce a single output file" + "target `//foo:two` must produce a single output file" ); // Target has no outputs -> fails - session.insert_target( - target_label.clone(), - FakeTargetRef::new(FakeTarget { - outputs: vec![], - ..Default::default() - }), + let res_empty = Attr::Label(Some(crate::LabelOrFile::Label( + target_empty.label().to_owned(), + ))) + .to_value( + &schema, + &session, + &session.default_toolchain.as_ref(), + &heap, ); - - let res_empty = Attr::Label(Some(crate::LabelOrFile::Label(target_label.clone()))) - .to_value( - &schema, - &session, - &session.default_toolchain.as_ref(), - &heap, - ); assert_eq!( res_empty.unwrap_err().to_string(), - "target `//foo:bar` must produce a single output file" + "target `//foo:empty` must produce a single output file" ); }); } @@ -566,10 +548,7 @@ #[test] fn test_to_value_label_keyed_string_dict() { let session = FakeSession::new(); - let target_label = Label::new( - PackageRef::new("//foo").unwrap().to_owned(), - "bar".to_owned(), - ); + let target = session.insert_empty_target(PackageRef::new("//foo").unwrap(), "bar"); let schema = AttrSchema { kind: AttrKind::LabelKeyedStringDict, @@ -582,17 +561,10 @@ Module::with_temp_heap(|module| { let heap = module.heap(); - session.insert_target( - target_label.clone(), - FakeTargetRef::new(FakeTarget { - outputs: vec![], - ..Default::default() - }), - ); let mut dict = SmallMap::new(); dict.insert( - crate::LabelOrFile::Label(target_label.clone()), + crate::LabelOrFile::Label(target.label().to_owned()), "value1".to_owned(), ); @@ -619,10 +591,17 @@ #[test] fn test_to_value_label_allow_files_matching() { let session = FakeSession::new(); - let target_label = Label::new( - PackageRef::new("//foo").unwrap().to_owned(), - "bar".to_owned(), - ); + let file1 = File::new("foo.cc"); + let file2 = File::new("foo.h"); + + let target_matching = session.insert_target(FakeTarget { + outputs: vec![file1.clone(), file2.clone()], + ..session.empty_target(PackageRef::new("//foo").unwrap(), "matching") + }); + let target_no_matching = session.insert_target(FakeTarget { + outputs: vec![file2.clone()], + ..session.empty_target(PackageRef::new("//foo").unwrap(), "no_matching") + }); let schema = AttrSchema { kind: AttrKind::Label, @@ -638,25 +617,16 @@ // Target outputs foo.cc and foo.h -> succeeds (at least one matches) and both // files are collected - let file1 = File::new("foo.cc"); - let file2 = File::new("foo.h"); - session.insert_target( - target_label.clone(), - FakeTargetRef::new(FakeTarget { - outputs: vec![file1.clone(), file2.clone()], - ..Default::default() - }), - ); - - let AttrValue { files, .. } = - Attr::Label(Some(crate::LabelOrFile::Label(target_label.clone()))) - .to_value( - &schema, - &session, - &session.default_toolchain.as_ref(), - &heap, - ) - .unwrap(); + let AttrValue { files, .. } = Attr::Label(Some(crate::LabelOrFile::Label( + target_matching.label().to_owned(), + ))) + .to_value( + &schema, + &session, + &session.default_toolchain.as_ref(), + &heap, + ) + .unwrap(); assert_eq!( UnpackList::<&File>::unpack_value_err(files.unwrap()) @@ -666,15 +636,10 @@ ); // Target outputs only foo.h -> fails (no matching outputs) - session.insert_target( - target_label.clone(), - FakeTargetRef::new(FakeTarget { - outputs: vec![file2.clone()], - ..Default::default() - }), - ); - - let res = Attr::Label(Some(crate::LabelOrFile::Label(target_label.clone()))).to_value( + let res = Attr::Label(Some(crate::LabelOrFile::Label( + target_no_matching.label().to_owned(), + ))) + .to_value( &schema, &session, &session.default_toolchain.as_ref(), @@ -683,7 +648,7 @@ assert_eq!( res.unwrap_err().to_string(), - "target `//foo:bar` does not produce any outputs matching allowed extensions: [\".cc\"]" + "target `//foo:no_matching` does not produce any outputs matching allowed extensions: [\".cc\"]" ); }); }
diff --git a/src/gn/starlark/crates/loader/src/loader.rs b/src/gn/starlark/crates/loader/src/loader.rs index 6075689..dd4b19b 100644 --- a/src/gn/starlark/crates/loader/src/loader.rs +++ b/src/gn/starlark/crates/loader/src/loader.rs
@@ -96,7 +96,7 @@ /// Loads, parses, compiles, and evaluates a Starlark module, resolving /// dependencies recursively and caching the result. - pub fn load<'b, C: EvalContext, F: Fn(&PackageRef) -> Box<C>>( + pub fn load<'b, C: EvalContext, F: Fn(&PackageRef) -> C>( &self, label: LabelRef<'b>, path_resolver: &PathResolver, @@ -130,7 +130,7 @@ self.set_complete(&file_status, result) } - fn load_and_evaluate<'b, C: EvalContext, F: Fn(&PackageRef) -> Box<C>>( + fn load_and_evaluate<'b, C: EvalContext, F: Fn(&PackageRef) -> C>( &self, label: LabelRef<'b>, label_str: &str, @@ -174,7 +174,7 @@ let extra = make_eval_context(label.package()); { let mut eval = Evaluator::new(&module); - eval.set_context(&*extra); + eval.set_context(&extra); eval.set_loader(&loader); eval.eval_module(ast, globals)?; } @@ -239,13 +239,15 @@ #[cfg(test)] mod tests { - use testutils::FakeEvalContext; + use std::rc::Rc; + + use testutils::{FakeEvalContext, FakeSession}; use types::Label; use super::*; - fn make_eval_context(pkg: &PackageRef) -> Box<FakeEvalContext> { - Box::new(FakeEvalContext::new(pkg.as_str())) + fn make_eval_context(_pkg: &PackageRef) -> FakeEvalContext { + FakeEvalContext::default_rule_impl(Rc::new(FakeSession::default())) } fn load(loader: &FileLoader, label_str: &str) -> starlark::Result<FrozenModule> {
diff --git a/src/gn/starlark/crates/rule/src/frozen_rule.rs b/src/gn/starlark/crates/rule/src/frozen_rule.rs index 91648df..2b5aac0 100644 --- a/src/gn/starlark/crates/rule/src/frozen_rule.rs +++ b/src/gn/starlark/crates/rule/src/frozen_rule.rs
@@ -155,7 +155,7 @@ use attr::{Attr, LabelOrFile}; use starlark::environment::FrozenModule; use testutils::FakeTarget; - use types::{Label, OutputType, PackageRef}; + use types::{Label, OutputType, PackageRef, Session}; use crate::globals::tests::new_assert; @@ -215,18 +215,18 @@ ); let context = assert.context(); - let targets_lock = context.session.targets.lock().unwrap(); let load = |name: &str| { let label = Label::new(PackageRef::root().to_owned(), name.to_owned()); - targets_lock - .get(&(label, context.session.default_toolchain.clone())) - .unwrap() - .get() + context + .session + .get_target(label.as_ref(), context.session.default_toolchain.as_ref()) }; assert_eq!( *load("shared_library"), FakeTarget { + label: Label::new(PackageRef::root().to_owned(), "shared_library".to_owned()), + toolchain: context.session.default_toolchain.clone(), outputs: vec![], attrs: vec![ Attr::String("optional_val".to_owned()), @@ -242,6 +242,8 @@ assert_eq!( *load("static_library"), FakeTarget { + label: Label::new(PackageRef::root().to_owned(), "static_library".to_owned()), + toolchain: context.session.default_toolchain.clone(), outputs: vec![], attrs: vec![Attr::String("optional_val".to_owned())], output_type: Some(OutputType::StaticLibrary), @@ -259,6 +261,8 @@ assert_eq!( *load("parent_defaulted"), FakeTarget { + label: Label::new(PackageRef::root().to_owned(), "parent_defaulted".to_owned()), + toolchain: toolchain.clone(), outputs: vec![], attrs: vec![ Attr::String("p".to_owned()), @@ -280,6 +284,8 @@ assert_eq!( *load("child_defaulted"), FakeTarget { + label: Label::new(PackageRef::root().to_owned(), "child_defaulted".to_owned()), + toolchain: toolchain.clone(), outputs: vec![], attrs: vec![ Attr::String("p".to_owned()), @@ -302,6 +308,8 @@ assert_eq!( *load("child_override"), FakeTarget { + label: Label::new(PackageRef::root().to_owned(), "child_override".to_owned()), + toolchain: toolchain.clone(), outputs: vec![], attrs: vec![ Attr::String("parent_val".to_owned()),
diff --git a/src/gn/starlark/crates/testutils/src/assert.rs b/src/gn/starlark/crates/testutils/src/assert.rs index 3be4a8e..565240d 100644 --- a/src/gn/starlark/crates/testutils/src/assert.rs +++ b/src/gn/starlark/crates/testutils/src/assert.rs
@@ -2,13 +2,15 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +use std::rc::Rc; + use starlark::{ environment::{FrozenModule, GlobalsBuilder}, values::UnpackValue, }; use types::{EvaluatorContextExt, Label, PathResolver, UnpackedOwnedValue}; -use crate::{register_globals, FakeEvalContext}; +use crate::{register_globals, FakeEvalContext, FakeSession}; type GlobalsConfig = Box<dyn Fn(&mut GlobalsBuilder)>; @@ -22,7 +24,9 @@ impl Default for Assert { fn default() -> Self { - Self::new(FakeEvalContext::default()) + Self::new(FakeEvalContext::default_rule_impl(Rc::new( + FakeSession::default(), + ))) } } @@ -91,6 +95,11 @@ &self.context } + /// Returns the current session. + pub fn session(&self) -> Rc<FakeSession> { + self.context.session.clone() + } + /// Asserts that the result of evaluating code is equal to expected. #[track_caller] pub fn eq<T>(&mut self, code: &str, expected: T)
diff --git a/src/gn/starlark/crates/testutils/src/eval_context.rs b/src/gn/starlark/crates/testutils/src/eval_context.rs index d43fcab..ad2004a 100644 --- a/src/gn/starlark/crates/testutils/src/eval_context.rs +++ b/src/gn/starlark/crates/testutils/src/eval_context.rs
@@ -1,7 +1,7 @@ // Copyright 2026 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -use std::{cell::UnsafeCell, collections::HashMap}; +use std::{cell::UnsafeCell, collections::HashMap, rc::Rc}; use attr::{Attr, EvalContext as AttrEvalContext, EvalContextAttrExt, Session as AttrSession}; use starlark::{ @@ -10,6 +10,7 @@ }; use types::{ CtxState, Label, LabelRef, OutputType, Package, PackageRef, PathResolver, Scope, Session, + TargetRef, }; use crate::{FakeSession, FakeTarget, FakeTargetRef}; @@ -48,7 +49,7 @@ pub current_toolchain: Label, /// The fake starlark session. #[allocative(skip)] - pub session: FakeSession, + pub session: Rc<FakeSession>, /// The fake path resolver. #[allocative(skip)] pub path_resolver: PathResolver, @@ -64,22 +65,26 @@ type StaticType = Self; } -impl Default for FakeEvalContext { - fn default() -> Self { - Self::new("//") - } -} - impl FakeEvalContext { + /// Creates a new eval context for a given session. + pub fn default_rule_impl(session: Rc<FakeSession>) -> Self { + Self::rule_impl(session.clone(), session.default_target()) + } + /// Creates a new `FakeEvalContext` for a given package name. - pub fn new(package: &str) -> Self { - let session = FakeSession::new(); + pub fn new(package: &PackageRef, name: &str) -> Self { + let session = Rc::new(FakeSession::default()); + let dummy_target = session.insert_empty_target(package, name); + Self::rule_impl(session, dummy_target) + } + + pub fn rule_impl(session: Rc<FakeSession>, target: FakeTargetRef) -> Self { Self { - package: PackageRef::new(package).unwrap().to_owned(), - current_toolchain: session.default_toolchain.clone(), + package: target.label().package().to_owned(), + current_toolchain: target.toolchain().to_owned(), session, path_resolver: PathResolver::new_for_testing(), - rule_state: CtxState::new(FakeTargetRef::default()).into(), + rule_state: CtxState::new(target).into(), scope: FakeScope::default(), } } @@ -129,15 +134,16 @@ attrs: Vec<Attr>, ) -> Result<<Self::Session as AttrSession>::TargetRef> { let label = Label::new(self.package.clone(), target_name.to_owned()); - let target = FakeTargetRef::new(FakeTarget { + let toolchain = self.current_toolchain().to_owned(); + Ok(self.session.insert_target(FakeTarget { + label, + toolchain, output_type: target_type, rule, cxx_attrs: scope.0.clone(), outputs: vec![], attrs, - ..Default::default() - }); - self.session.insert_target(label, target.clone()); - Ok(target) + dependencies: Default::default(), + })) } }
diff --git a/src/gn/starlark/crates/testutils/src/session.rs b/src/gn/starlark/crates/testutils/src/session.rs index 26d918e..78d1e94 100644 --- a/src/gn/starlark/crates/testutils/src/session.rs +++ b/src/gn/starlark/crates/testutils/src/session.rs
@@ -2,20 +2,19 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -use std::{collections::HashMap, sync::Mutex}; +use std::{cell::RefCell, collections::HashSet}; use attr::Session; -use types::{Label, LabelRef, PackageRef}; +use types::{Label, LabelRef, PackageRef, TargetRef}; -use crate::FakeTargetRef; +use crate::{FakeTarget, FakeTargetRef}; /// A fake implementation of the `Session` trait for testing. pub struct FakeSession { /// The preconfigured default toolchain label. pub default_toolchain: Label, - /// A map of fake targets populated for testing, indexed by (label, - /// toolchain). - pub targets: Mutex<HashMap<(Label, Label), FakeTargetRef>>, + /// A set of fake targets populated for testing. + pub targets: RefCell<HashSet<FakeTargetRef>>, } impl Default for FakeSession { @@ -28,21 +27,52 @@ /// Creates a new `FakeSession` instance with empty targets and a /// preconfigured default toolchain. pub fn new() -> Self { - Self { + let this = Self { default_toolchain: Label::new( PackageRef::root().to_owned(), "default_toolchain".to_owned(), ), - targets: Mutex::new(HashMap::new()), + targets: RefCell::new(HashSet::new()), + }; + this.insert_empty_target(PackageRef::root(), "default"); + this + } + + pub fn default_target(&self) -> FakeTargetRef { + self.get_target( + LabelRef::new(PackageRef::root(), "default"), + self.default_toolchain.as_ref(), + ) + } + + /// Helper to insert a target. + pub fn insert_target(&self, target: FakeTarget) -> FakeTargetRef { + let target_ref = FakeTargetRef::new(target); + let mut targets = self.targets.borrow_mut(); + assert!( + targets.insert(target_ref.clone()), + "Inserting an already existing target into the map" + ); + target_ref + } + + /// Helper to create an empty target with the default toolchain. + pub fn empty_target(&self, package: &PackageRef, name: &str) -> FakeTarget { + FakeTarget { + label: LabelRef::new(package, name).to_owned(), + toolchain: self.default_toolchain.clone(), + outputs: Default::default(), + attrs: Default::default(), + output_type: Default::default(), + rule: Default::default(), + cxx_attrs: Default::default(), + dependencies: Default::default(), } } - /// Helper to insert a target under the default toolchain. - pub fn insert_target(&self, label: Label, target: FakeTargetRef) { - self.targets - .lock() - .unwrap() - .insert((label, self.default_toolchain.clone()), target); + /// Helper to insert an empty target. + pub fn insert_empty_target(&self, package: &PackageRef, name: &str) -> FakeTargetRef { + self.insert_target(self.empty_target(package, name)) } } @@ -51,11 +81,11 @@ fn get_target(&self, label: LabelRef<'_>, current_toolchain: LabelRef<'_>) -> Self::TargetRef { self.targets - .lock() + .borrow() + .iter() + .find(|target| target.label() == label && target.toolchain() == current_toolchain) .unwrap() - .get(&(label.to_owned(), current_toolchain.to_owned())) - .cloned() - .unwrap_or_default() + .clone() } fn register_dependency<'a>( @@ -65,7 +95,6 @@ toolchain: LabelRef<'a>, ) { source - .get() .dependencies .lock() .unwrap()
diff --git a/src/gn/starlark/crates/testutils/src/target.rs b/src/gn/starlark/crates/testutils/src/target.rs index dfdb571..f1a6ee8 100644 --- a/src/gn/starlark/crates/testutils/src/target.rs +++ b/src/gn/starlark/crates/testutils/src/target.rs
@@ -4,7 +4,7 @@ use std::{ collections::{HashMap, HashSet}, - hash::Hasher, + ops::Deref, sync::{Arc, Mutex}, }; @@ -20,8 +20,10 @@ }; /// A fake target struct for testing. -#[derive(Debug, Allocative, Default)] +#[derive(Allocative, Debug)] pub struct FakeTarget { + pub label: Label, + pub toolchain: Label, /// A list of fake files returned as outputs of the target. pub outputs: Vec<File>, /// A list of attributes. @@ -37,7 +39,9 @@ impl PartialEq for FakeTarget { fn eq(&self, other: &Self) -> bool { - self.outputs == other.outputs + self.label == other.label + && self.toolchain == other.toolchain + && self.outputs == other.outputs && self.attrs == other.attrs && self.output_type == other.output_type && self.rule == other.rule @@ -51,18 +55,13 @@ && *self.dependencies.lock().unwrap() == *other.dependencies.lock().unwrap() } } + impl Eq for FakeTarget {} /// A reference to a fake target. #[derive(Debug, ProvidesStaticType, NoSerialize, Allocative, Clone)] pub struct FakeTargetRef(#[allocative(skip)] Arc<FakeTarget>); -impl Default for FakeTargetRef { - fn default() -> Self { - Self::new(FakeTarget::default()) - } -} - impl FakeTargetRef { /// Creates a new `FakeTargetRef` containing the given `FakeTarget`. pub fn new(target: FakeTarget) -> Self { @@ -76,20 +75,21 @@ /// Returns the registered dependencies of this target. pub fn registered_deps(&self) -> HashSet<(Label, Label)> { - self.get().dependencies.lock().unwrap().clone() + self.dependencies.lock().unwrap().clone() } } impl PartialEq for FakeTargetRef { fn eq(&self, other: &Self) -> bool { - Arc::ptr_eq(&self.0, &other.0) + self.label == other.label && self.toolchain == other.toolchain } } impl Eq for FakeTargetRef {} impl std::hash::Hash for FakeTargetRef { fn hash<H: std::hash::Hasher>(&self, state: &mut H) { - Arc::as_ptr(&self.0).hash(state); + self.label.hash(state); + self.toolchain.hash(state); } } @@ -107,7 +107,7 @@ impl<'v> StarlarkValue<'v> for FakeTargetRef { fn equals(&self, other: Value<'v>) -> starlark::Result<bool> { if let Some(other) = other.downcast_ref::<Self>() { - Ok(Arc::ptr_eq(&self.0, &other.0)) + Ok(self == other) } else { Ok(false) } @@ -117,13 +117,29 @@ &self, hasher: &mut starlark::collections::StarlarkHasher, ) -> starlark::Result<()> { - let ptr = Arc::as_ptr(&self.0) as usize; - hasher.write_usize(ptr); + use std::hash::Hash as _; + self.hash(hasher); Ok(()) } } +impl Deref for FakeTargetRef { + type Target = FakeTarget; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + impl TargetRef for FakeTargetRef { + fn label(&self) -> LabelRef<'_> { + self.get().label.as_ref() + } + + fn toolchain(&self) -> LabelRef<'_> { + self.get().toolchain.as_ref() + } + fn outputs(&self) -> Vec<File> { self.get().outputs.clone() }
diff --git a/src/gn/starlark/crates/types/src/target_ref.rs b/src/gn/starlark/crates/types/src/target_ref.rs index 8b6c06c..98c7b2c 100644 --- a/src/gn/starlark/crates/types/src/target_ref.rs +++ b/src/gn/starlark/crates/types/src/target_ref.rs
@@ -18,6 +18,11 @@ pub trait TargetRef: for<'v> StarlarkValue<'v> + for<'v> AllocValue<'v> + Clone + IPromiseToImplementStarlarkEqAndHash { + /// Returns the label of the target. + fn label(&self) -> LabelRef<'_>; + /// Returns the toolchain the label was defined in. + fn toolchain(&self) -> LabelRef<'_>; + /// Returns the output files produced by this target. fn outputs(&self) -> Vec<File>;