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

Change to automatically insert #include of header files when they are specified #104

Merged
merged 8 commits into from
Oct 17, 2023

Conversation

ydah
Copy link
Collaborator

@ydah ydah commented Oct 5, 2023

Resolve: #14 (comment)

This PR change to automatically insert #include of header files when they are specified.
Bison will automatically embed #include header files when run with the -H, --header=[FILE] or -d options.
So, we make header file is include automatically when -h, --header or -d option is passed.

We found that if Lrama was made to behave the same as Bison as is, it would fail to build ruby.
The use of the -H, --header=[FILE] or -d options to include header files seemed to cause problems in cases like ripper.y, where ripper.h was not included from ripper.c.
So we added the --auto-include option to automatically add an include statement to the header file when this option is specified. How about it?

@ydah ydah force-pushed the feature-embed-include branch 3 times, most recently from cb656b5 to 34bdf29 Compare October 6, 2023 15:28
@ydah ydah changed the title Change to embed #include of header files when they are specified Change to automatically insert #include of header files when they are specified Oct 6, 2023
@ydah ydah force-pushed the feature-embed-include branch 2 times, most recently from 8283db1 to 0e0343a Compare October 6, 2023 15:49
@ydah ydah changed the title Change to automatically insert #include of header files when they are specified [WIP] Change to automatically insert #include of header files when they are specified Oct 6, 2023
@ydah ydah marked this pull request as draft October 6, 2023 17:37
@ydah ydah force-pushed the feature-embed-include branch from 44a37bf to 929f27b Compare October 14, 2023 23:03
@ydah ydah changed the title [WIP] Change to automatically insert #include of header files when they are specified Change to automatically insert #include of header files when they are specified Oct 15, 2023
@ydah ydah marked this pull request as ready for review October 15, 2023 00:13
@yui-knk
Copy link
Collaborator

yui-knk commented Oct 17, 2023

Thanks for the PR. My expectation is that a header file is included automatically when -h, --header or -d option is passed.
I suspect the cause of ripper build failure is that #include is placed on very beginning of output file so that a lot of declarations, e.g. VALUE, NODE and so on, are not defined when the header file is included.
Could you try the below?

  1. Rebase or merge current master to reflect Sync yacc.c to yacc.h #133
  2. Update
    /* Use api.header.include to #include this header
    like below?
<%# b4_header_include_if -%>
<%- if output.header_file_path -%>
#include <%= option.include_header %>
<%- else -%>
/* Use api.header.include to #include this header
   instead of duplicating it here.  */
...
  <%-# b4_percent_code_get([[provides]]). %code is not supported -%>
  <%-# b4_cpp_guard_close([b4_spec_mapped_header_file]) -%>
    <%- if output.spec_mapped_header_file -%>
#endif /* !<%= output.b4_cpp_guard__b4_spec_mapped_header_file %>  */
    <%- end -%>
<%- end -%>
<%# b4_declare_symbol_enum -%>

@ydah ydah force-pushed the feature-embed-include branch from 7322bbf to 180dbad Compare October 17, 2023 15:11
@ydah ydah force-pushed the feature-embed-include branch from 180dbad to 71b82bc Compare October 17, 2023 15:12
@ydah
Copy link
Collaborator Author

ydah commented Oct 17, 2023

Thank you so much! I updated this PR.

@yui-knk
Copy link
Collaborator

yui-knk commented Oct 17, 2023

:shipit:

@yui-knk yui-knk merged commit da4a6cd into ruby:master Oct 17, 2023
14 checks passed
@ydah ydah deleted the feature-embed-include branch October 17, 2023 23:06
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