Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Byteslice class #490

Draft
wants to merge 2 commits into
base: dev
Choose a base branch
from
Draft

feat: Byteslice class #490

wants to merge 2 commits into from

Conversation

joe-p
Copy link
Contributor

@joe-p joe-p commented Jun 4, 2024

Due to the complexities of properly supporting EcmaScript utf-16 strings, a new class should be used to represent byteslices and utf-8 ABI strings. This PR is the implementation of the Byteslice class.

Migration

Literals

String literals must now use the Bytes tag

const x = 'foo'
const x = Bytes`foo`

String Type

The string type is no longer supported. Instead, use utf8 which is an alias for Byteslice

foo(x: string) {
foo(x: utf8) {

Prototype Methods

bytes, bytes<N>, and utf8 no longer support the string prototype methods. Most notably, you cannot access specific bytes/characters within a string via [idx]. This will likely be supported by a method on Byteslice such as getByte()

Copy link

netlify bot commented Jun 4, 2024

Deploy Preview for tealscript ready!

Name Link
🔨 Latest commit 2898394
🔍 Latest deploy log https://app.netlify.com/sites/tealscript/deploys/665f14be262e9600079e7cb1
😎 Deploy Preview https://deploy-preview-490--tealscript.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@@ -33,12 +33,12 @@ class Calculator extends Contract {
*
* @returns The result of the operation
*/
doMath(a: uint64, b: uint64, operation: string): uint64 {
doMath(a: uint64, b: uint64, operation: utf8): uint64 {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if str or utf8string is a clearer type name for this?

Copy link
Contributor Author

@joe-p joe-p Jun 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I avoided str because I think it's a bit confusing since it's so close to string. I'm not opposed to utf8string but just went with utf8 since it's shorter

let result: uint64;

if (operation === 'sum') {
if (operation === Bytes`sum`) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be great to have a separate constructor function for utf-8 strings rather than using Bytes otherwise it's a bit confusing (e.g. "what does comparing a string with bytes mean?").

Suggested change
if (operation === Bytes`sum`) {
if (operation === Str`sum`) {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The answer to "what does comparing a string with bytes mean?" is "the same thing as comparing bytes to bytes". The underlying data is the exact same, utf8 here is just a signal of what the ABI type should be in the signature, nothing more or less.

@@ -252,10 +252,17 @@ declare function Uint<N extends widths>(value: number | string): uint<N>;

declare type ufixed<N extends widths, M extends precisions> = Brand<number, `ufixed${N}x${M}`>;

declare class Byte {}

declare class Byteslice<Length = void> {}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Byteslice isn't user-facing is it? It feels like a weird classname, but if users always use bytes instead then it doesn't matter

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah the user facing type is bytes<N>. I had to go with Byteslice here because I wanted Bytes for the tag function

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants