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

Various fixes and additions #17

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

whizzter
Copy link

Added code to find and include files from frameworks
Added code to support the #import directive and only process files once when using that directive
Added code to handle ?: condition expression evaluation
Fixed a bug when macro arguments spanned over multiple lines
Fixed a bug where SQSTRING's was not passed through
Fixed a bug where something that started as a number could span an entire line eating up unrelated tokens

@shevek
Copy link
Owner

shevek commented Sep 11, 2014

The lexing to end-of-line on error was relatively deliberate. I'm not sure that returning IDENTIFIER is any better. Perhaps the INVALID should only reach until the next whitespace?

@shevek
Copy link
Owner

shevek commented Sep 11, 2014

shevek@shadow:~/java/jcpp$ git ci -m "Preprocessor: Fix SQSTRING."
[master 9c8391a] Preprocessor: Fix SQSTRING.
2 files changed, 16 insertions(+), 6 deletions(-)

@shevek
Copy link
Owner

shevek commented Sep 11, 2014

Re: Frameworks, all the file I/O including the pre-checks for existence have to be done through VirtualFileSystem - breaking out and using java.io.File will break the VFS abstraction.

@shevek
Copy link
Owner

shevek commented Sep 11, 2014

"Fixed a bug when macro arguments spanned over multiple lines" - I'm not seeing this... where should I look?

@shevek
Copy link
Owner

shevek commented Sep 11, 2014

[master 2db1eaf] LexerSource: Handle invalid number as a single INVALID token and don't consume the entire line.
3 files changed, 44 insertions(+), 15 deletions(-)

Handled using unicode rules rather than simple C rules.

@whizzter
Copy link
Author

As to the errornous number suffix i think there was some version information in some of Apples iOS headers that was in a form similiar to 5_1_0 and only used by the preprocessor, since jcpp took the entire line when faulting on the suffix this broke things. (It was a few months ago i was working on this, i'll try taking in your code later today and test if it still parses through everything)

@whizzter
Copy link
Author

Again, not remembering everything exactly but i think that it was without the "case NL" around line 743 in commit(d67cee2) that macro arguments spanning over multiple lines broke since the preprocessor would back-off when seeing something "unknown".

@whizzter
Copy link
Author

I think there was something that made working with the VFS hard in this case and i think i reasoned that the abstraction would be to allow compile servers that abstracted parsing user code as f.ex. coming from uploaded zip's but still used local "system" packages. I'll take a peek later to see if i can figure out what made it tricky to use the VFS for this.

@whizzter
Copy link
Author

(As the INVALID vs IDENTIFIER thing, i think i initially changed to IDENTIFIER when i didn't notice what was wrong but changed it back to invalid in a4ce2c8 )

@shevek
Copy link
Owner

shevek commented Sep 11, 2014

Will this work?
#20

@shevek
Copy link
Owner

shevek commented Sep 11, 2014

[master cccd60f] Handle conditionals in preprocessor statements.
2 files changed, 16 insertions(+), 3 deletions(-)

@shevek
Copy link
Owner

shevek commented Sep 11, 2014

I think that given #20 and a re-look at #import, I think that will cover everything here?

@shevek
Copy link
Owner

shevek commented Sep 11, 2014

What happens if I hash #import on VirtualFile instead of String?

Can you #import <> and then #import "" the same thing? The implementation in this PR implies 'yes', but that's not quite logical.

@whizzter
Copy link
Author

The code for the frameworks seemingly looks fine to me, simpler than my variant also that was overly defensive and kinda went in the wrong way (I think my variant was inspired by one of the other forks)

@whizzter
Copy link
Author

hashing on virtualfile sounds more sane but i remember some weirdness described somewhere (as to why i separated the import variants, i'll try to find the source for this).

@whizzter
Copy link
Author

Actually, i think that you're right about just going with the VFS identity. I separated <> and "" because i didn't think i could rely on simple names (and didn't know that VFS entries were supposed to be reliable for identity checking) in the case someone included a local copy after a system copy and so i erred on the side of extra inclusions.

@shevek
Copy link
Owner

shevek commented Sep 12, 2014

OK, so I'll put the framework code into master, please let me know whether it works as I have no objc code to play with.

I'll also code up import to use VFS identity (and perhaps to share the cache with #pragma once).

@shevek
Copy link
Owner

shevek commented Sep 12, 2014

When I've done the #import work, I'd love your opinion on whether there's anything in your commit that I missed?

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