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

Support designated initialization style. #35

Merged
merged 1 commit into from
May 14, 2021

Conversation

roptat
Copy link
Contributor

@roptat roptat commented May 14, 2021

Fixes #32.

  • frontc/cabs.ml: Add DESIGNATED expression type for struct
    initialization.
  • frontc/cparser.mly: Add rules for compounds (struct initialization). A
    compound contains a list of expressions or designated, but a standard
    initialization cannot be a designated outside a compound.
  • frontc/cprint.ml: Print new AST type.
  • frontc/ctoxml.ml: Print XML for new designated AST type.
  • ctoxml/test.t/c99-struct-initializers.c: New test.
  • ctoxml/test.t/run.t: Update expected output for new test.

@roptat
Copy link
Contributor Author

roptat commented May 14, 2021

I don't know if adding a type to the AST is the best thing to do, but I couldn't think of anything else. If you have a better idea, I'd be glad to remove that new type :)

@ivg
Copy link
Member

ivg commented May 14, 2021

OK, looks good to merge, but can you please first rebase with master (or merge master, doesn't really matter) so that we can sure that the new set of tests is still passing. There is a small conflict in run.t but it is trivial.

@ivg
Copy link
Member

ivg commented May 14, 2021

Concerning the AST changes, I think we have no other choice (though having it in the expression is a little bit weird, but it looks like that it is indeed where it should be). I have only question about the name DESIGNATED, shouldn't it be a DESIGNATOR, I can't find the authoritative source right away, but it looks like that they are called designators, e.g., from here

Designated initializers, a C99 feature, are supported for aggregate types, including arrays, structures, and unions. A designated initializer, or designator, points out a particular element to be initialized. A designator list is a comma-separated list of one or more designators. A designator list followed by an equal sign constitutes a designation.

Do you have some better reference?

@roptat
Copy link
Contributor Author

roptat commented May 14, 2021

Thanks, will rename and rebase :)

@ivg
Copy link
Member

ivg commented May 14, 2021

You know, after deeper reading, I think your naming is correct (I still can't find any reference in the C99 standard about designated intializers) but at least this document (it is C++ though) states that the deisgnator is the left-hand side of the designated clause (basically the name part), so DESIGNATED sounds more correct.

Please keep it :)

@roptat
Copy link
Contributor Author

roptat commented May 14, 2021

ah, oops :)

Fixes BinaryAnalysisPlatform#32.

* frontc/cabs.ml: Add `DESIGNATED` expression type for struct
initialization.
* frontc/cparser.mly: Add rules for compounds (struct initialization).  A
compound contains a list of expressions or designated, but a standard
initialization cannot be a designated outside a compound.
* frontc/cprint.ml: Print new AST type.
* frontc/ctoxml.ml: Print XML for new designated AST type.
* ctoxml/test.t/c99-struct-initializers.c: New test.
* ctoxml/test.t/run.t: Update expected output for new test.
@roptat
Copy link
Contributor Author

roptat commented May 14, 2021

There, rebased and kept DESIGNATED

@ivg
Copy link
Member

ivg commented May 14, 2021

awesome, thanks!

@ivg ivg merged commit 0d845ec into BinaryAnalysisPlatform:master May 14, 2021
@ivg
Copy link
Member

ivg commented May 14, 2021

@roptat, I noticed a new warning,

File "frontc/cparser.mly", line 788, characters 0-21:
Warning: symbol init_comma_expression is unreachable from any of the start symbol(s).

and indeed it is not used anywhere. Is that expected? Then I will delete it if you don't mind.

@roptat
Copy link
Contributor Author

roptat commented May 17, 2021

Ha sorry for the lack of reply, I didn't notice yours. I simply copied the rules, and didn't notice init_comma_expression was only used once, so indeed, you can remove it. Thank you!

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.

designated initializer style
2 participants