refactor(bytesview): change bytesview back to concrete type

[Yu-zh]

The compiler does not need @bytes.View to be abstract to perform optimizations. So it's changed back.

[peter-jerry-ye-code-review]

Redundant internal fields access via public API methods

Category
Maintainability
Code Snippet
pub fn View::length(self : View) -> Int { self.len() }
Recommendation
Remove redundant methods that simply wrap field access. Direct field access is preferable when there's no additional logic.
Reasoning
The methods length(), data(), and start_offset() are just wrappers around field access without any additional logic. This adds an unnecessary layer of indirection. Since they're public APIs though, they may need to be kept for backwards compatibility.

Missing validation in struct instantiation

Category
Correctness
Code Snippet
{ bytes: self, start, len: end - start }
Recommendation
Consider adding a constructor function that validates the inputs:

fn View::new(bytes: Bytes, start: Int, len: Int) -> View {
  guard start >= 0 && len >= 0 && start + len <= bytes.length() else {
    abort("Invalid View parameters")
  }
  { bytes, start, len }
}

Reasoning
Direct struct instantiation bypasses any validation of the start and length parameters. This could lead to invalid View objects being created. A constructor function would ensure all Views are valid.

Potential redundant bounds checking

Category
Performance
Code Snippet
pub fn View::op_get(self : View, index : Int) -> Byte {
guard index >= 0 && index < self.len else {...}
self.bytes[self.start + index]
}
Recommendation
Consider adding a comment explaining that self.bytes[..] includes its own bounds checking, or use unsafe_get if that's not needed
Reasoning
There might be double bounds checking happening here - once in the guard statement and once in the array access. If the underlying Bytes implementation also does bounds checking, this could be redundant.

[coveralls]

Pull Request Test Coverage Report for Build 6908

Details

• 14 of 14 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.001%) to 92.453%

---

Totals
Change from base Build 6906:	-0.001%
Covered Lines:	8685
Relevant Lines:	9394

---

💛 - Coveralls

[bobzhang]

is there any benefit changing it back? Note we still did some magic operations on View, so it may make sense to keep it abstract

[Yu-zh]

is there any benefit changing it back? Note we still did some magic operations on View, so it may make sense to keep it abstract

• To be consistent with @string.View and @array.View since there will be no benefit to change these two types to abstract type.
• Less hardcoded magic in the compiler, which makes it more maintainable. @bytes.View and other views are all treated as value types but they trigger different handling logic.