Skip to content
This repository has been archived by the owner on Jun 3, 2021. It is now read-only.

Skip whitespaces in preprocessor #462

Merged
merged 6 commits into from
Jun 5, 2020
Merged

Skip whitespaces in preprocessor #462

merged 6 commits into from
Jun 5, 2020

Conversation

Stupremee
Copy link
Contributor

This PR will skip any white spaces that occur in the lex::replace::replace method, to support macro calls where
the identifier and the arguments are separated by spaces (e.g. f ().

Resolves #457

@jyn514
Copy link
Owner

jyn514 commented Jun 1, 2020

r? @hdamron17

I'm worried this will lose whitespace again while printing with -E but I'll defer to Hunter's judgement.

@jyn514
Copy link
Owner

jyn514 commented Jun 1, 2020

As an alternative you could store the whitespace tokens and yield those tokens if no replacement was found.

@Stupremee
Copy link
Contributor Author

That sounds like a good idea

@hdamron17
Copy link
Collaborator

As long as the macro is actually replaced then it is fine to consume the whitespace. The issue is if you have an id that is the name of a function macro but has is not used as one because it is not followed by a parenthesis. Can you peek at the token following the whitespace?

@hdamron17
Copy link
Collaborator

Make a test using assert_same_exact that has both cases of f(...) being replaced and f left as is.

src/lex/cpp.rs Outdated Show resolved Hide resolved
@Stupremee Stupremee requested review from jyn514 and hdamron17 June 2, 2020 21:16
// If this branch is matched, this is not a macro call,
// since all other cases are covered above.
// So just append the identifier and the current token to the stack.
Some(Ok(Locatable { data: _, .. })) => {
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
Some(Ok(Locatable { data: _, .. })) => {
Some(Ok(_)) => {

src/lex/replace.rs Outdated Show resolved Hide resolved
@jyn514

This comment has been minimized.

@Stupremee
Copy link
Contributor Author

The test is still failing with the following error:

left: `"\n\n1\n"`,
right: `"\n1\n"`',

I can't figure out why so you have to look over it.

@jyn514 jyn514 merged commit 5aa9522 into jyn514:master Jun 5, 2020
@Stupremee Stupremee deleted the skip-whitespaces-in-replace branch June 5, 2020 23:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Space between function macro name and args
3 participants