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

Fix line directives for ifdef and ifndef when default hooks are used #140

Merged

Conversation

jefftrull
Copy link
Collaborator

At some point in the past the handling for #if and #ifdef diverged. The code that handles emitting a line directive when a conditional section is skipped worked for #if but not #ifdef/#ifndef.

This problem was not observable when the eat_whitespace_hooks were used instead of the default_preprocessing_hooks, because the former signals skipped newlines through the may_skip_whitespace hook, hiding the problem. Furthermore, the majority of Wave tests use the
eat_whitespace hooks, so it wasn't visible in testing.

This change restores #ifdef/#ifndef to the same section as #if, so any future changes to conditional handling will happen uniformly. Also, a test case is added to cover the default hooks and this particular case.

If merged, this will resolve #138

At some point in the past the handling for #if and #ifdef diverged.
The code that handles emitting a line directive when a conditional
section is skipped worked for #if but not ifdef/ifndef.

This problem was not observable when the eat_whitespace hooks were
used instead of the default_preprocessing hooks, because the former
signals skipped newlines through the may_skip_whitespace hook,
hiding the problem. Furthermore, the majority of Wave tests use the
eat_whitespace hooks, so it wasn't visible there.

This change restores ifdef/ifndef to the same section as #if, so any
changes to conditional handling will happen uniformly. Also, a test
case is added to cover the default hooks and this particular case.
@jefftrull jefftrull requested a review from hkaiser January 2, 2022 06:24
Copy link
Collaborator

@hkaiser hkaiser left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

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.

Empty ifdef block does not emit line directive for missing whitespace.
2 participants