perf!(builtin/string): prevent calculating .length() twice in substring()

[bobzhang]

No description provided.

[peter-jerry-ye-code-review]

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

Here are three observations from the provided git diff output:

1. Parameter Naming Inconsistency:

   • In the builtin.mbti file, the substring method's end parameter is changed from end~ to end?. This change introduces inconsistency in the naming convention. The ~ typically indicates a default value, while ? often signifies an optional parameter. This inconsistency could lead to confusion or errors in usage.

2. Potential Logic Issue:

   • In the string.mbt file, the substring function now uses a match statement to handle the end parameter. If end is None, it defaults to the string's length. However, the guard condition guard start >= 0 && start <= end && end <= len might fail if end is explicitly set to a value greater than the string's length. This could lead to unexpected behavior or runtime errors.

3. Documentation Update Needed:

   • The function's behavior has changed, but the accompanying documentation (comments above the function) does not reflect this change. The example provided in the comments still assumes the old behavior where end had a default value. This could mislead users who rely on the documentation for understanding the function's usage.

These issues should be addressed to ensure consistency, correctness, and clarity in the codebase.

[coveralls]

Pull Request Test Coverage Report for Build 4332

Details

• 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
• 23 unchanged lines in 2 files lost coverage.
• Overall coverage decreased (-0.4%) to 82.949%

---

Files with Coverage Reduction	New Missed Lines	%
bytes/xxhash.mbt	8	55.0%
bytes/bytes.mbt	15	9.09%

Totals
Change from base Build 4330:	-0.4%
Covered Lines:	4602
Relevant Lines:	5548

---

💛 - Coveralls