provide String::sub and String::substring, both raise when it hit surrogate pair

[bobzhang]

No description provided.

[peter-jerry-ye-code-review]

Potential breaking change without proper error handling in caller code

Category
Correctness
Code Snippet
fn String::substring(
self : String,
start? : Int = 0,
end? : Int,
) -> String raise CreatingSubStringError
Recommendation
Ensure all existing callers of String::substring are updated to handle the new CreatingSubStringError exception, or provide a migration guide for users
Reasoning
The function signature change from returning String to String raise CreatingSubStringError is a breaking change that will cause compilation errors in existing code that doesn't handle the exception

Inconsistent naming pattern between `op_as_view` and `sub` function aliases

Category
Maintainability
Code Snippet
#alias(op_as_view)
fn String::sub(String, start? : Int, end? : Int) -> View raise CreatingViewError
Recommendation
Consider using a more descriptive alias name like substring_view or as_view instead of the generic sub to maintain consistency with other string operations
Reasoning
The alias sub is too generic and doesn't clearly indicate it returns a View type, which could lead to confusion for developers expecting a String return type

Unnecessary surrogate checking in substring operations may impact performance

Category
Performance
Code Snippet
if start < len && self.unsafe_charcode_at(start).is_trailing_surrogate() {
raise InvalidIndex
}
if end < len && self.unsafe_charcode_at(end).is_trailing_surrogate() {
raise InvalidIndex
}
Recommendation
Consider adding a fast-path for ASCII-only strings or caching UTF-16 surrogate information to avoid repeated character code checks
Reasoning
The surrogate validation requires two additional character code lookups that could be expensive for frequently called substring operations, especially when most strings don't contain surrogates

[coveralls]

Pull Request Test Coverage Report for Build 1005

Details

• 4 of 4 (100.0%) changed or added relevant lines in 3 files are covered.
• 59 unchanged lines in 7 files lost coverage.
• Overall coverage decreased (-0.1%) to 88.605%

---

Files with Coverage Reduction	New Missed Lines	%
builtin/array.mbt	1	98.29%
ref/ref.mbt	1	90.0%
rational/rational.mbt	2	0.0%
hashset/hashset.mbt	6	95.12%
bytes/bitstring.mbt	7	87.36%
set/linked_hash_set.mbt	14	88.74%
list/list.mbt	28	90.24%

Totals
Change from base Build 999:	-0.1%
Covered Lines:	8670
Relevant Lines:	9785

---

💛 - Coveralls