-
Notifications
You must be signed in to change notification settings - Fork 4
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
Fix missing record declaration tests #162
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like we have the same issue regarding to unnamed primitive types.
We check whether it declares an identifier or not inside the iteration over the init_decl_list
; however, we don't even get into the loop when it doesn't declare an identifier, leading to an empty declaration node:
Lines 398 to 405 in 2f6edee
for (auto& init_decl : init_decl_list) { | |
if (init_decl) { | |
init_decl->type = ResolveType(type->Clone(), std::move(init_decl->type)); | |
} else { // unnamed primitive type | |
init_decl = std::make_unique<VarDeclNode>(Loc(@1), "", type->Clone()); | |
} | |
decl_list.push_back(std::move(init_decl)); | |
} |
vitaminc/test/typecheck/decl.c
Line 4 in 2f6edee
int; |
vitaminc/test/typecheck/decl.exp
Lines 9 to 10 in 2f6edee
DeclStmtNode <4:3> | |
DeclStmtNode <5:3> |
Would you mind addressing them together? Here's a possible modification:
if (std::holds_alternative<std::unique_ptr<Type>>(decl_specifiers)) {
auto type = std::move(std::get<std::unique_ptr<Type>>(decl_specifiers));
if (init_decl_list.empty()) {
// A stand-alone type that doesn't declare any identifier, e.g., `int;`.
decl_list.push_back(std::make_unique<VarDeclNode>(Loc(@1), "", type->Clone()));
}
// Declare primitive identifier.
for (auto& init_decl : init_decl_list) {
if (init_decl) {
init_decl->type = ResolveType(type->Clone(), std::move(init_decl->type));
}
decl_list.push_back(std::move(init_decl));
}
} else {
auto decl = std::move(std::get<std::unique_ptr<DeclNode>>(decl_specifiers));
// A record declaration that doesn't declare any identifier, e.g., `struct point {int x, int y};`.
if (init_decl_list.empty()) {
decl_list.push_back(std::move(decl));
}
auto* rec_decl = dynamic_cast<RecordDeclNode*>(decl.get());
// Declare record identifier.
for (auto& init_decl : init_decl_list) {
if (init_decl) {
init_decl->type = ResolveType(rec_decl->type->Clone(), std::move(init_decl->type));
}
decl_list.push_back(std::move(init_decl));
}
}
DeclStmtNode <4:3>
+ VarDeclNode <4:3> : int
DeclStmtNode <5:3>
I just realized there is another issue. If we build a
|
437cbbe
to
4166ce8
Compare
Suggest fixing this in a future PR since it's related only to unnamed nodes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 😄
We can resolve the code generation issue one day. 🤟
My bad, missed this update on previous reviews. The current implementation only consider the case for
struct
andunion
variable initialization.