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

Variable intialization at declare time #908

Merged
merged 4 commits into from
Sep 26, 2024

Conversation

MrDaiki
Copy link
Collaborator

@MrDaiki MrDaiki commented Sep 19, 2024

Issue

This is a duplicate of issue #899 based on #901 refactoring of pre-typing.
The feature requested is to add a new syntax that authorize variable initialization when they are declared.

The syntax is as follow :

reg u64 x=1,y=2,z,t=3;

Implementation

With the proposed implementation, we can also initialize multiple variables at the same time:

reg u64 x=1,y=2,z,t=3;

The current implementation doesn't support array initializing with this feature because array can currently be explicitly created only at top level. Allowing it would require reworking a large part of the grammar and pre-typing analysis.

In terms of semantic, the proposed implementation works as follow :

reg u64 x=3;

is abstractly equivalent to :

reg u64 x;
x=3;

Copy link
Member

@vbgl vbgl left a comment

Choose a reason for hiding this comment

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

Please rebase on top of recent main branch.

compiler/src/latex_printer.ml Outdated Show resolved Hide resolved
compiler/src/latex_printer.ml Outdated Show resolved Hide resolved
@vbgl
Copy link
Member

vbgl commented Sep 24, 2024

I did some cleaning and spotted a bug (see the fresh test case): checking of the LHS should occur before the RHS is added to the environment.

@MrDaiki
Copy link
Collaborator Author

MrDaiki commented Sep 24, 2024

I did some cleaning and spotted a bug (see the fresh test case): checking of the LHS should occur before the RHS is added to the environment.

To solve this case, I think we will need to call tt_expr instead of tt_assign (or we consider that tt_assign checks can be redundant)

Copy link
Member

@vbgl vbgl left a comment

Choose a reason for hiding this comment

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

Just add a changelog entry and this will be ready.

bgregoir
bgregoir previously approved these changes Sep 26, 2024
Copy link
Contributor

@bgregoir bgregoir left a comment

Choose a reason for hiding this comment

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

This is nice, I approve it.
If you don't want to do the small suggested changes, I will merge it as it is.

CHANGELOG.md Show resolved Hide resolved
@@ -214,7 +214,15 @@ type align = [`Align | `NoAlign]

type plvals = annotations L.located option * plvalue list

type vardecls = pstotype * pident list

type vardecl = InitVarDecl of pident * pexpr | NotInitVarDecl of pident
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the definition
type vardecl = pident * pexpr option
will be more convenient.
You can do the change if you want.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That was my first type definition (in fact I am not sure why I changed it). I will change it back

compiler/tests/fail/common/var_initialize_undef.jazz Outdated Show resolved Hide resolved
compiler/src/pretyping.ml Outdated Show resolved Hide resolved
@vbgl
Copy link
Member

vbgl commented Sep 26, 2024

I’ve done the small changes you suggested. I let Alexandre decide whether he wants to change the data-type.

Copy link
Member

@vbgl vbgl left a comment

Choose a reason for hiding this comment

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

Seems ready!

@bgregoir bgregoir merged commit 1bd46f6 into jasmin-lang:main Sep 26, 2024
1 check passed
@MrDaiki MrDaiki deleted the new-var-init branch September 27, 2024 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants