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

Tree: store cast kind as part of implicit/explicit cast nodes #244

Merged
merged 6 commits into from
Jun 23, 2022

Conversation

ehaas
Copy link
Collaborator

@ehaas ehaas commented Feb 16, 2022

Briefly discussed here: #232 (comment)

If you like this approach I can fill out the fromExplicitCast function.

@Vexu
Copy link
Owner

Vexu commented Feb 16, 2022

Looks good. I'd prefer if explicit casts were dumped the same way as implicit ones (cast kind, no "operand:").

If you like this approach I can fill out the fromExplicitCast function.

It'd probably be best to unify that, the various if's in castExpr and all the T1ToT2 functions in Result into one castType function.

@ehaas
Copy link
Collaborator Author

ehaas commented Feb 25, 2022

I was thinking it might make sense to add some testing around the AST layout before I try to have implicitCast and the initialization code use castType - what would you think of something like a directory called ast in test/cases that contains the expected AST for the corresponding file of the same name in test/cases (basically, like expanded but for the AST). If the file exists, the test runner would check that the parsed AST matches what is expected.

@Vexu
Copy link
Owner

Vexu commented Feb 25, 2022

what would you think of something like a directory called ast in test/cases that contains the expected AST for the corresponding file of the same name in test/cases

Sounds good to me. I'm guessing the plan is to do a string comparison on the output of Tree.dump?

@ehaas
Copy link
Collaborator Author

ehaas commented Feb 25, 2022

I was thinking just strings compare initially; perhaps something more sophisticated if minor formatting changes make the strings too annoying

@Vexu
Copy link
Owner

Vexu commented Mar 5, 2022

Test failure caused by Git messing with the line ends, fixed by adding test/cases/ast/* text eol=lf to .gitattributes.

@Vexu
Copy link
Owner

Vexu commented Jun 20, 2022

Is this blocked on something?

@ehaas
Copy link
Collaborator Author

ehaas commented Jun 20, 2022

I think I set it aside to work on some other things I needed to complete it and then forgot about it - I can look into resolving the conflicts

@ehaas ehaas marked this pull request as ready for review June 20, 2022 18:58
@ehaas
Copy link
Collaborator Author

ehaas commented Jun 20, 2022

There's still the potential cleanup for castExpr / T1ToT2 but I think that can be done as a separate PR if this one looks good; it should be easier to test now that we can make sure the AST is correct

@Vexu Vexu merged commit c4d1f98 into Vexu:master Jun 23, 2022
@ehaas ehaas deleted the castkind branch June 15, 2023 19:34
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