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

AST: use inline record for Ptyp_arrow. #7250

Merged
merged 1 commit into from
Jan 17, 2025
Merged

Conversation

cristianoc
Copy link
Collaborator

No description provided.

@cristianoc cristianoc force-pushed the typ-arrow-inline-record branch from 276b975 to 30b0d78 Compare January 17, 2025 09:35
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Syntax Benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.05.

Benchmark suite Current: 30b0d78 Previous: e1b7fb7 Ratio
Print RedBlackTreeNoComments.res - time/run 2.210543746666666 ms 2.10057036 ms 1.05
Print Napkinscript.res - time/run 81.68034058666667 ms 77.00100409999999 ms 1.06

This comment was automatically generated by workflow using github-action-benchmark.

@cristianoc cristianoc requested review from cknitt and zth January 17, 2025 09:43
Ptyp_arrow
{
lbl = argumentLabel;
typ1 = argumentTypeExpr;
Copy link
Member

Choose a reason for hiding this comment

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

Should we maybe use more descriptive names than typ1 and typ2?
Something like argument_type and return_type (or arg_typ and ret_typ or similar)?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That'd be a good addition indeed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The fact that they are types is redundant, so typ1 and typ2 convey very little information.
arg and ret could work well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Renamed.

@cristianoc cristianoc force-pushed the typ-arrow-inline-record branch from 30b0d78 to b3ce244 Compare January 17, 2025 12:48
Copy link
Member

@cknitt cknitt left a comment

Choose a reason for hiding this comment

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

👍

@cristianoc cristianoc merged commit 31fe30b into master Jan 17, 2025
20 of 21 checks passed
@cristianoc cristianoc deleted the typ-arrow-inline-record branch January 17, 2025 14:40
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.

3 participants