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

[Feature] String Abstract Syntax #943

Closed
1 task done
gluax opened this issue May 11, 2021 · 6 comments · Fixed by #974
Closed
1 task done

[Feature] String Abstract Syntax #943

gluax opened this issue May 11, 2021 · 6 comments · Fixed by #974
Assignees
Labels
feature A new feature.

Comments

@gluax
Copy link
Contributor

gluax commented May 11, 2021

🚀 Feature

The abstract syntax tree needs to be extended with a string type.

Motivation

This helps the implementation goal of #929.

Implementation

As for reducers, a dummy implementation will be needed, or when this branch is ready, we can pull this one in. Till the reducers are resolved, compilation will fail, though.

The string type will be converted to an array at the canonicalization phase.

The Following is necessary:

AST Side

  • A ValueExpression extension that contains the string Node type leo/ast/src/expression/value.rs.

ASG Side

The ASG expects a canonicalized version; therefore, the string should already be an array of characters by this point, and nothing new should be necessary here.

Tests

No test possible till both reducers and parser + lexer is in.

@acoglio
Copy link
Collaborator

acoglio commented May 12, 2021

I don't believe we need to extend the Type enum, because we are not introducing a type for strings. We do add string literals, but just (re)use [char; N] types for strings.

https:/AleoHQ/leo/blob/master/docs/rfc/001-initial-strings.md#strings

@gluax
Copy link
Contributor Author

gluax commented May 12, 2021

The reason I made this issue was based on the slack discussion. When seeing the syntax const hello = "hello"; the Parser shouldn't be the one to identify and convert that to an array. A string should just be a string was what was agreed on there. So it should just create an AST Node that can then be converted to a [char; N] during canonicalization - #945. At least that was my understanding.

@acoglio
Copy link
Collaborator

acoglio commented May 12, 2021

Yes, we need an AST node for string literals (which canonicalization will convert to an array inline expression), which you list as 'A ValueExpression extension that contains the char Node type leo/ast/src/expression/value.rs.'. I was referring to the first item, 'New addition to the Type Enum leo/ast/src/types/types_.rs'.

@gluax
Copy link
Contributor Author

gluax commented May 12, 2021

That second part should say string Value Expression actually, let me fix that. The char one would already be added by #939. We still need string Nodes in the AST so we can parse and canonicalize on something.

@gluax
Copy link
Contributor Author

gluax commented May 12, 2021

Ah, wait, I see what you mean. Hmmm, I think that should be fine. I won't really know till implementation if that will cause an issue or not.

@damirka damirka self-assigned this May 13, 2021
@gluax gluax linked a pull request May 22, 2021 that will close this issue
@damirka
Copy link
Contributor

damirka commented May 26, 2021

Closed by #957.

@damirka damirka closed this as completed May 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants