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

Clean up suspicious preprocessing logic #21

Open
viluon opened this issue Apr 19, 2023 · 1 comment
Open

Clean up suspicious preprocessing logic #21

viluon opened this issue Apr 19, 2023 · 1 comment
Labels
bug Something isn't working

Comments

@viluon
Copy link
Member

viluon commented Apr 19, 2023

The preprocess method of PreprocessorState has some dubious control flow.

pub fn preprocess(&mut self, input: &mut VecDeque<(FileId, Token, Span)>) -> Vec<(FileId, Token, Span)> {

It looks like it would report a recursive import error if a single file were included non-recursively from multiple locations. The check for that should compare the vertex state of the included file against VertexState::Open, not just see whether it has any state at all (i.e. whether it was seen yet). To make that work, however, preprocess() needs to manage the vertex states reasonably, which it doesn't presently do. The top of the while loop checks whether the incoming token comes from a different file than the tokens up to this point. This is not enough. We need a proper stack of #includes here, because the input stream can mix tokens from different source files, and a change in the source doesn't imply that we're done with said source. The #include stack should correspond to the path of FileId's for the resolved output tokens. We only associate a single FileId and Span with each token at the moment.

Currently, the while loop leaves the last processed file in the open vertex state, which is incorrect (and a possible concern for caching).

@viluon viluon added the bug Something isn't working label Apr 19, 2023
@viluon viluon changed the title Suspicious preprocessing logic Clean up suspicious preprocessing logic Apr 19, 2023
@AndrewF001
Copy link
Contributor

Recursively including files isn't a compiler error but the preprocessor does have a depth limit, here's example code. I also agree that the current code will flag an error if a header file is included twice, which is why we need to support #ifndef and etc in enum PreprocessorDirective, as stated in 6.2 specification.

Proposed solution would be to recursively call preprocessor() on included files with a depth limiter. Just lexing and pasting it into the result vector isn't enough, as it's depended on past #define and checking for redefinition errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants