add Clone trait

[illusory0x0]

• add Clone trait #1429

I apologize for turning off the previous pull request by mistake.

[peter-jerry-ye-code-review]

‼️ This code review is generated by a bot. Please verify the content before trusting it.

Here are three observations and potential issues from the provided git diff output:

1. Inconsistent Copyright Year:

   • In builtin/traits.mbt, the copyright year was updated from 2024 to 2025. However, the copyright year in the newly created file builtin/clone.mbt is already set to 2025. This inconsistency might not cause functional issues, but it could lead to confusion or legal discrepancies. It would be better to ensure all files have the same copyright year.

2. Redundant Enum Definition:

   • The Mutability enum is defined twice: once in builtin/builtin.mbt and again in builtin/traits.mbt. This redundancy could lead to maintenance issues or conflicts if the definitions diverge in the future. It would be better to define the Mutability enum in a single location and import it where needed.

3. Potential Issue with clone Implementation for Array[T]:

   • In builtin/clone.mbt, the clone implementation for Array[T] uses self.copy_array() when T::mutability() is Immutable. However, the copy_array function is defined to create a new array and copy elements, which might not be necessary for immutable types. This could lead to unnecessary performance overhead. It might be more efficient to directly return self for immutable types, as they do not need to be copied.

These issues should be reviewed and addressed to improve code quality and maintainability.

[coveralls]

Pull Request Test Coverage Report for Build 4692

Details

• 29 of 50 (58.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.1%) to 82.844%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
builtin/clone.mbt	29	50	58.0%

Totals
Change from base Build 4690:	-0.1%
Covered Lines:	4795
Relevant Lines:	5788

---

💛 - Coveralls

[peter-jerry-ye]

We will switch to immutable @Yu-zh

[Yu-zh]

You also need to check E::mutability() here. I'd suggest

match self {
  Ok(x) => if T is mutable ...
  Err(x) => if E is mutable ...
}

[Yu-zh]

we have a function physical_equal but be aware that its behavior is backend-dependent.

[bobzhang]

what's the motivation for this API? it looks to me we are adding lots of API for a niche use case?

[illusory0x0]

what's the motivation for this API? it looks to me we are adding lots of API for a niche use case?

• sorted_set.add is not appropriate #1448
• FixedArray::deep_clone is NOT really a deep clone #1423
• Array::deep_clone is not appropriate #527

[FlammeShadow]

It seems the type Mutability is only used by mutability(). Why not just use is_mutable() -> Bool ?

[illusory0x0]

stm32 use RESET SET, ENABLE DISABLE
enum, rather than only true and false.

add new enum for type safe, and more meaningful

It seems the type Mutability is only used by mutability(). Why not just use is_mutable(Self) -> Bool ?

... this might be api preference

this is just compiletime constant, not depend instance, below is enough
is_mutable() -> Bool

[FlammeShadow]

this is just compiletime constant, not depend instance, below is enough
is_mutable() -> Bool

My fault. It was my mistake.

[illusory0x0]

cc @Guest0x0

[Lampese]

what's the motivation for this API? it looks to me we are adding lots of API for a niche use case?

• sorted_set.add is not appropriate #1448
• FixedArray::deep_clone is NOT really a deep clone #1423
• Array::deep_clone is not appropriate #527

cc @bobzhang @illusory0x0

Is this referring to discussing the necessity of the mutable API rather than the necessity of the Clone trait?

I think the original intention of distinguishing between immutable and mutable in the design was because the Clone behavior of immutable is unreasonable, but there may be some problems with the API design. Users are already explicitly using this API when using Clone, which proves that the cost of deep replication of immutable data structures is visible and affordable for users (we do have such a need, if there is a lot of mutable data stored in an immutable data structure) Sacrificing convenience to distinguish the need for variability may not be ideal.

[illusory0x0]

Is this referring to discussing the necessity of the mutable API rather than the necessity of the Clone trait?

• sorted_set.add is not appropriate #1448

cc @Lampese