Skip to content

Conversation

@TIHan
Copy link
Contributor

@TIHan TIHan commented Jun 18, 2019

Resolves this issue: #7000

Lexing a line directive would consume a newline which would put the LexBuffer in an invalid state, but is adjusted once the line directive was applied. In tooling, we disable applying the line directive which means the LexBuffer would remain in an invalid state. Adding a newline corrects this in cases where we don't apply the line directive.

Copy link
Contributor

@cartermp cartermp left a comment

Choose a reason for hiding this comment

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

Stupid question time: this doesn't lead to big allocations, right? Since this is just adding a newline to the buffer, not the whole file?

@TIHan
Copy link
Contributor Author

TIHan commented Jun 18, 2019

Stupid question time: this doesn't lead to big allocations, right? Since this is just adding a newline to the buffer, not the whole file?

It doesn't add it to the file, just the buffer. We also do this sort of thing in other places in lex.fsl so it's normal.

if not skip then (HASH_LINE (LexCont.Token !args.ifdefStack)) else token args skip lexbuf }
else
// add a newline when we don't apply a directive since we consumed a newline getting here
newline lexbuf
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm actually unsure about putting this here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can confirm this is needed as well.

I tested by turning "skip" off.


else
// add a newline when we don't apply a directive since we consumed a newline getting here
newline lexbuf
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the line that really fixes the problem.

@realvictorprm
Copy link
Contributor

This is great, you see that @smoothdeveloper :)?

@TIHan TIHan merged commit cb880d5 into dotnet:master Jun 18, 2019
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.

4 participants