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

Postfix record member access expression #163

Merged
merged 8 commits into from
Jun 19, 2024

Conversation

leewei05
Copy link
Contributor

@leewei05 leewei05 commented Jun 19, 2024

Changes for types

  • Introduce a new struct Field for storing both id and Type for record members.
  • Introduce a new class RecordType for sharing common methods for struct and union.

AST

  • Introduce a new AST node RecordMemExprNode for postfix record member access expression.

Other

  • Update dump info for InitExprNode.

P.S: I had added additional typecheck tests, but I haven't implemented any code generation for record declaration, so it will throw error src/qbe_ir_generator.cpp:108: int {anonymous}::PrevExprNumRecorder::NumOfPrevExpr(): Assertion num_of_prev_expr_ != kNoRecord. I can add additional expression tests after implementing code generation.

I can start implementing code generation for struct and union after this PR. 😄

@leewei05 leewei05 self-assigned this Jun 19, 2024
@leewei05 leewei05 requested a review from Lai-YT June 19, 2024 12:00
Copy link
Collaborator

@Lai-YT Lai-YT left a comment

Choose a reason for hiding this comment

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

Once again, thank you for supporting the record types. I submitted some changes that I believe can improve the PR even further.

Additionally, if a struct and a union share the same name, such as struct SomeName and union SomeName, would there be a collision in the type table?

include/type.hpp Outdated Show resolved Hide resolved
include/type.hpp Outdated Show resolved Hide resolved
include/type.hpp Outdated Show resolved Hide resolved
include/type.hpp Outdated Show resolved Hide resolved
src/type_checker.cpp Outdated Show resolved Hide resolved
@leewei05
Copy link
Contributor Author

Additionally, if a struct and a union share the same name, such as struct SomeName and union SomeName, would there be a collision in the type table?

Right! I haven't thought about that case. I think it needs an extra type check for LoopUpType.

@Lai-YT
Copy link
Collaborator

Lai-YT commented Jun 19, 2024

Additionally, if a struct and a union share the same name, such as struct SomeName and union SomeName, would there be a collision in the type table?

Right! I haven't thought about that case. I think it needs an extra type check for LoopUpType.

Or maybe we include the "struct " and "union " as the prefix of the id? XD

@leewei05
Copy link
Contributor Author

Additionally, if a struct and a union share the same name, such as struct SomeName and union SomeName, would there be a collision in the type table?

Right! I haven't thought about that case. I think it needs an extra type check for LoopUpType.

Or maybe we include the "struct " and "union " as the prefix of the id? XD

Great suggestion! Let me try to implement with this approach.

@leewei05 leewei05 force-pushed the postfix-record-member branch 3 times, most recently from 5b6fd36 to a365546 Compare June 19, 2024 15:33
@leewei05 leewei05 requested a review from Lai-YT June 19, 2024 15:36
@leewei05 leewei05 mentioned this pull request Jun 19, 2024
13 tasks
src/type_checker.cpp Outdated Show resolved Hide resolved
src/type_checker.cpp Outdated Show resolved Hide resolved
src/type_checker.cpp Outdated Show resolved Hide resolved
src/type_checker.cpp Outdated Show resolved Hide resolved
Copy link
Collaborator

@Lai-YT Lai-YT left a comment

Choose a reason for hiding this comment

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

LGTM! We're moving on to code generation!! Oh yay 🎉

@Lai-YT Lai-YT merged commit 1cf2769 into fruits-lab:main Jun 19, 2024
4 checks passed
@leewei05 leewei05 deleted the postfix-record-member branch June 19, 2024 16:44
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