String element length #2854
String element length #2854
Conversation
|
You have successfully added a new CodeQL configuration |
|
@superboyiii Do you have any idea what is |
Jim8y
changed the title
|
We have CodeQL enabled for NeoGo repo, it's a nice thing to have in general (another type of static analysis), even though we have some false positives from it. |
I haven't tried CodeQL before, I have no authority to modify the config. |
|
My local commit does not contain that file, what a weird thing. Forget it, not a big issue. |
|
LMAO, maybe it is me configured my forked repo or something, forget it. |
| @@ -222,5 +223,39 @@ private static string[] StringSplit([MaxLength(MaxInputLength)] string str, stri | |||
| StringSplitOptions options = removeEmptyEntries ? StringSplitOptions.RemoveEmptyEntries : StringSplitOptions.None; | |||
| return str.Split(separator, options); | |||
| } | |||
|
|
|||
| [ContractMethod(CpuFee = 1 << 8)] | |||
| private static int StringByteLength([MaxLength(MaxInputLength)] string str) | |||
There was a problem hiding this comment.
It is same as
OpCode.SIZE?
Exactly, just thought it would be better to make the naming style consistent.
There was a problem hiding this comment.
It is same as
OpCode.SIZE?Exactly, just thought it would be better to make the naming style consistent.
Is not the same because this take in care about the unicode chars
…g-length # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
| private static int StringCharLength([MaxLength(MaxInputLength)] string str) | ||
| { | ||
| // return the length of the string in characters | ||
| // it should return 2 for "🦆" and 1 for "ã" |
There was a problem hiding this comment.
Do we need it? It's tied to .NET-specific definition of Char and UTF-16 which is very niche. At the moment I just don't know how it could be implemented in Go. utf8.RuneCountInString works fine or StringElementLength, but StringCharLength looks very much C#/.NET specific and not very useful at the first glance.
There was a problem hiding this comment.
Will remove this one then.
There was a problem hiding this comment.
@roman-khimov updated, may you please review again
There was a problem hiding this comment.
Looks OK, just one simple method.
|
@superboyiii also it would be awesome if you could kindly test the performance of this function to correctly and precisely set the |
Co-authored-by: Shargon <[email protected]>
Co-authored-by: Shargon <[email protected]>
|
@roman-khimov #2854 (comment) could you replicate the logic in Go now? |
There was a problem hiding this comment.
@roman-khimov #2854 (comment) could you replicate the logic in Go now?
I think so. Is this one for 3.6?
I think that it could be included |
Fix #2851