Skip to content

Conversation

Copy link

Copilot AI commented Feb 10, 2026

The Copy trait was implemented on BuildEvent<'a> and PackageTarballInfo<'a> despite these being event types containing borrowed references. While technically valid (all fields are Copy), this is semantically incorrect—events represent discrete state changes and shouldn't be trivially duplicated.

Changes

  • Removed Copy from event types: BuildEvent<'a> and PackageTarballInfo<'a> now derive only Clone
  • Made duplication explicit: Updated PipelineReceiver::on_event to explicitly clone the event when forwarding to inner receiver
// Before: implicit copy due to Copy trait
self.inner.on_event(event);
match event { ... }

// After: explicit clone makes cost visible
self.inner.on_event(event.clone());
match event { ... }

This follows Rust idioms where Clone without Copy signals that duplication has semantic weight, appropriate for event types.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Address feedback on pipeline deps resolver refactor for Copy implementation Remove Copy trait from BuildEvent and PackageTarballInfo Feb 10, 2026
Copilot AI requested a review from xusd320 February 10, 2026 06:30
@elrrrrrrr
Copy link
Contributor

Copilot 这个说法有点教条了。它的逻辑是:"event 代表一次性的状态变更,不应该被随意复制"。

但这里的 BuildEvent<'a> 并不拥有任何数据——它全是借用引用(&'a str、&'a Path),本质上就是一个只读视图。Copy 它不会复制任何实际状态,只是多了一个指向相同数据的指针。这跟复制一个拥有
owned data 的事件(比如 Event { data: String }) 完全不同。

如果是 BuildEvent 持有 String、PathBuf 这些 owned 类型,那 Copilot 说的有道理——随意 Copy/Clone 会有性能和语义问题。但对一个全是引用的轻量 enum,Copy 就是最自然的选择。

实际上你也可以问 Copilot:不用 Copy 的话怎么解决 PipelineReceiver::on_event 里 event 被用两次的问题?它只能建议你 .clone()——对一个全是引用的类型手动 clone,反而更别扭。

@xusd320 😄 他们打起来了

@xusd320 xusd320 closed this Feb 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants