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

Record initialization with no designators #165

Merged
merged 8 commits into from
Jun 21, 2024

Conversation

leewei05
Copy link
Contributor

@leewei05 leewei05 commented Jun 20, 2024

This PR implement record initialization with no designator code generation. Even though we support initializing multiple elements for union, union member access expression will only return the first element (Similar with gcc and clang, they only throw warning)[1].

[1] https://godbolt.org/z/57fYnn95G

@leewei05 leewei05 self-assigned this Jun 20, 2024
@leewei05 leewei05 mentioned this pull request Jun 20, 2024
13 tasks
@leewei05 leewei05 requested a review from Lai-YT June 20, 2024 08:18
src/type.cpp Outdated Show resolved Hide resolved
src/qbe_ir_generator.cpp Outdated Show resolved Hide resolved
src/qbe_ir_generator.cpp Outdated Show resolved Hide resolved
include/type.hpp Outdated Show resolved Hide resolved
src/type.cpp Outdated Show resolved Hide resolved
include/type.hpp Outdated
Comment on lines 216 to 217
std::size_t offset( // NOLINT(readability-identifier-naming)
const std::size_t index) const override;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suggest dropping the const specifier for pass-by-value parameters from the function declaration, but keep it in the function definition. This does not apply to const-reference parameters, which should be kept in both places.

Suggested change
std::size_t offset( // NOLINT(readability-identifier-naming)
const std::size_t index) const override;
std::size_t offset( // NOLINT(readability-identifier-naming)
std::size_t index) const override;

Note that two function declarations differing only in the constness of their pass-by-value parameters are not considered overloads, as they are indistinguishable when called. Removing const from the function declaration is recommended for the following reasons:

  1. The caller does not need to know whether the parameter is const or not, as it does not affect the caller. Including const only clutters the function declaration.
  2. You might want to change the function's implementation, which could involve removing the const. If const is included in the function declaration, you will have to update it there as well.

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

After applying the current fix, compiler will not initialize exceeding elements.

// struct.c 
int main() {
  struct ss;

  struct birth {
    int date;
    int month;
    int year;
  };

  // Compiler will stop initializing elements at index 4 since we only have
  // 3 members defined in `struct birth`.
  struct birth bd1 = {3, 5, 1998, 2000, 2002};

  __builtin_print(bd1.date);
// struct.ssa
function w $main() {
@.start.1
@.body.1
        %.1 =l alloc4 12
        %.2 =w copy 3
        %.3 =l add %.1, 0
        storew %.2, %.3
        %.4 =w copy 5
        %.5 =l add %.1, 4
        storew %.4, %.5
        %.6 =w copy 1998
        %.7 =l add %.1, 8
        storew %.6, %.7
        %.8 =l copy $__builtin_print
        %.9 =w loadw %.1
        %.10 =l add %.1, 0
        %.11 =w loadw %.10
        %.12 =w call $printf(l $__builtin_print_format, w %.11)

@leewei05 leewei05 requested a review from Lai-YT June 21, 2024 08:52
src/type.cpp Outdated Show resolved Hide resolved
src/type.cpp Outdated Show resolved Hide resolved
src/type.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.

Additionally, the NOLINTs can be removed after the renaming.

include/type.hpp 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! Masterpiece 🤓

@Lai-YT Lai-YT merged commit 1a3ffab into fruits-lab:main Jun 21, 2024
4 checks passed
@leewei05 leewei05 deleted the record-init branch June 21, 2024 10:51
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