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

Preprocessor stringify # #456

Merged
merged 10 commits into from
Aug 3, 2020
Merged

Preprocessor stringify # #456

merged 10 commits into from
Aug 3, 2020

Conversation

hdamron17
Copy link
Collaborator

This is an extension of branch in #437. I will rebase when that branch is merged.

Initial attempt seems to work for all cases except stringification of strings which contain escaped characters, in which case the escape is not deep enough. E.g.

#define f(a) #a
f("\n")

preprocesses to


"\"\n\""

but should be


"\"\\n\""

@hdamron17 hdamron17 force-pushed the preprocessor-stringify branch from ec96a1a to 3941074 Compare May 25, 2020 22:14
@hdamron17
Copy link
Collaborator Author

Similarly to #395, this should maintain the original characters. I.e. ? and \x3f should be handled differently even though they are identical semantically.

@hdamron17 hdamron17 force-pushed the preprocessor-stringify branch from 6940104 to c58ef37 Compare May 26, 2020 02:18
jyn514
jyn514 reviewed May 26, 2020
View reviewed changes
src/lex/replace.rs Outdated Show resolved Hide resolved
src/lex/replace.rs Outdated Show resolved Hide resolved
src/lex/replace.rs Outdated Show resolved Hide resolved
src/lex/replace.rs Outdated Show resolved Hide resolved
src/lex/replace.rs Outdated Show resolved Hide resolved
src/lex/replace.rs Outdated Show resolved Hide resolved
@jyn514
Copy link
Owner

jyn514 commented May 26, 2020

Similarly to #395, this should maintain the original characters. I.e. ? and \x3f should be handled differently even though they are identical semantically.

This will be tricky since the lexer currently parses hex escapes immediately. We may have to wait until #384 is implemented.

@jyn514
Copy link
Owner

jyn514 commented May 26, 2020

Also, please add all the examples I gave as unit tests.

@hdamron17 hdamron17 marked this pull request as draft June 5, 2020 22:57
@hdamron17 hdamron17 force-pushed the preprocessor-stringify branch from ac77399 to 4a73420 Compare June 11, 2020 04:20
@hdamron17
Copy link
Collaborator Author

Everything except the issue of str::from_utf8 failing is handled.

@hdamron17 hdamron17 marked this pull request as ready for review June 11, 2020 04:25
@hdamron17 hdamron17 requested a review from jyn514 June 11, 2020 04:25
hdamron17
hdamron17 commented Jun 11, 2020
View reviewed changes
src/lex/cpp.rs Outdated Show resolved Hide resolved
@hdamron17 hdamron17 force-pushed the preprocessor-stringify branch from 1232efe to 4273903 Compare June 11, 2020 05:10
@hdamron17 hdamron17 mentioned this pull request Jun 13, 2020
Closed
jyn514
jyn514 reviewed Jul 3, 2020
View reviewed changes
Copy link
Owner

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

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

Oops, I started a review a few weeks ago but never finished.

src/lex/cpp.rs Show resolved Hide resolved
src/lex/replace.rs Outdated Show resolved Hide resolved
@hdamron17
Copy link
Collaborator Author

No worries- I have to finish the rc-token branch first anyway so I’ll be occupied for a while.

hdamron17 added 6 commits July 27, 2020 18:19
@hdamron17
Initial attempt at implementing preprocessor #
1ed9e2a 
works except on nested strings which contain escape characters
@hdamron17
Add a second level of escape on strings
5c0b5f8
@hdamron17
Added tests for preprocessor_stringify, fails for '
7eaa7d6
@hdamron17
More (currently failing) tests for preprocessor stringify
a6774b6
@hdamron17
Fix several errors in stringify
54e5e8c 
Whitespace between hash and id
Hash with no replacement should keep hash
Put null terminator on stringified value
Add more test cases
@hdamron17
Updated for RcStr
5d94c7f
@hdamron17 hdamron17 force-pushed the preprocessor-stringify branch from 4273903 to 5d94c7f Compare July 27, 2020 23:08
@hdamron17 hdamron17 requested a review from jyn514 July 27, 2020 23:10
@hdamron17 hdamron17 changed the title [WIP] Preprocessor stringify # Preprocessor stringify # Jul 27, 2020
@hdamron17
Copy link
Collaborator Author

Turns out only " and \ are escaped as per 6.10.3.2. This and a few other issues are fixed now.

r? @jyn514

jyn514
jyn514 reviewed Jul 29, 2020
View reviewed changes
Copy link
Owner

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

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

Couple nits, and I'd also like to see a few tests other than #define xstr(a) #a. But overall this looks good :)

src/lex/replace.rs Outdated Show resolved Hide resolved
src/lex/replace.rs Outdated Show resolved Hide resolved
src/lex/replace.rs Outdated Show resolved Hide resolved
@hdamron17
Copy link
Collaborator Author

I made your changes and added a few more tests. If you have anything specific you want, let me know.

r? @jyn514

jyn514
jyn514 approved these changes Aug 3, 2020
View reviewed changes
Copy link
Owner

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

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

Last couple nits. Looks great :)

src/lex/replace.rs Show resolved Hide resolved
src/lex/replace.rs Show resolved Hide resolved
@jyn514 jyn514 merged commit eb8fba3 into jyn514:master Aug 3, 2020
@hdamron17 hdamron17 deleted the preprocessor-stringify branch August 10, 2020 01:52
@hdamron17 hdamron17 mentioned this pull request Aug 17, 2020
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jyn514 jyn514 jyn514 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hdamron17 @jyn514