8000
refactor(bytesview): change bytesview back to concrete type by Yu-zh · Pull Request #2104 · moonbitlang/core · GitHub
More Web Proxy on the site http://driver.im/
You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Direct field access instead of function calls may improve performance
Category
Performance Code Snippet
pub fn View::length(self : View) -> Int { self.len } Recommendation
Consider making this an inline function with #inline annotation if performance is critical Reasoning
While direct field access is already efficient, if this is a frequently called method, marking it as inline could help in hot paths
The struct definition comment could be more descriptive
Category
Maintainability Code Snippet
#builtin.valtype
struct View {
bytes : Bytes
start : Int
len : Int
} Recommendation
Add field-level documentation explaining the invariants:
#builtin.valtypestructView {
/// The underlying bytes bufferbytes : Bytes/// Starting offset in the buffer (>= 0)start : Int/// Length of the view (>= 0)len : Int
}```
**Reasoning**Documentingfieldinvariantshelpspreventmisuseandmakesmaintenanceeasier</details><details><summary>Novalidationofstructfieldinvariantsduringconstruction</summary>**Category**Correctness**CodeSnippet**pubfnBytes::op_as_view(self : Bytes, start~ : Int=0, end? : Int) ->View {
...
{ bytes: self, start, len: end-start }
}
**Recommendation**ConsideraddingruntimechecksormakingtheViewconstructorprivatewithavalidatedfactorymethod**Reasoning**Invalidfieldvaluescouldleadtooutofboundsaccess. Whilethepublicmethodsdovalidate, preventinginvalidconstructionwouldbesafer</details>
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.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The compiler does not need
@bytes.Viewto be abstract to perform optimizations. So it's changed back.