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

Emit line directives for includes with directive on first line #223

Merged
merged 8 commits into from
Oct 3, 2024

Conversation

njnobles
Copy link
Contributor

@njnobles njnobles commented Sep 20, 2024

Attempts to address #222

Include files that begin with an #if/#ifdef/#define directive now properly emit the line directive that was previously missing.

Because the test suite is using the eat_whitespace context policy, the issue only shows if the directive in the include file occurs on the very first line of the file (hence why the copyright comments have been moved to the end of the test files). If the default_preprocessing_hooks was used, the copyright comment could be at the beginning and still show the issue.

From my debugging, it seems like the line directive was failing to be emitted because it would errantly fall into this portion of the if-block in cpp_iterator: https://github.com/boostorg/wave/blob/develop/include/boost/wave/util/cpp_iterator.hpp#L786

Here's the output for the t_2_032.cpp test that I added prior to the change:


t_2_032_001a
#line 6 "t_2_032_001.hpp"
t_2_032_001b

t_2_032_001c

t_2_032_002

t_2_032_004
#line 2 "t_2_032_003.hpp"
t_2_032_003
#line 16 "t_2_032.cpp"
t_2_032_a

t_2_032_b

And here's the output after this change:

#line 2 "t_2_032_001.hpp"
t_2_032_001a
#line 6 "t_2_032_001.hpp"
t_2_032_001b

t_2_032_001c
#line 2 "t_2_032_002.hpp"
t_2_032_002
#line 2 "t_2_032_004.hpp"
t_2_032_004
#line 2 "t_2_032_003.hpp"
t_2_032_003
#line 16 "t_2_032.cpp"
t_2_032_a

t_2_032_b

@njnobles
Copy link
Contributor Author

Hi @jefftrull,

Apologies for reaching out directly, but you mind reviewing this PR when you have some available time? It seems like one of the GitHub Actions failed, but it looks like it was a container setup issue and I'm not sure if there's anyway I could try to trigger a re-run.

Thank you!

@jefftrull
Copy link
Collaborator

Hi! Sorry I've been traveling for the last week and a half :) I saw this PR and hope to get on it soon. Thanks in advance for your contribution (and for including a test!).

I agree that the one check failure looks like some infrastructure problem and not due to your code changes.

@njnobles
Copy link
Contributor Author

Hi! Not a problem at all. I hope you enjoyed your travel :) Thanks in advance for taking a look at it!

@jefftrull
Copy link
Collaborator

Please review my PR to your PR when you have time :)

@njnobles
Copy link
Contributor Author

njnobles commented Oct 3, 2024

Reviewed and merged :)

@jefftrull jefftrull merged commit ffdc341 into boostorg:develop Oct 3, 2024
7 checks passed
@jefftrull
Copy link
Collaborator

Thank you for the helpful contribution!

jefftrull pushed a commit that referenced this pull request Oct 31, 2024
* Emit line directives for includes with directive on first line
* Add nested include tests

Thanks to Nick Nobles for this bug report and fix
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