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

Don't use an invalid srcInfo to construct json source info. #588

Merged
merged 1 commit into from
May 10, 2017

Conversation

ChrisDodd
Copy link
Contributor

  • Also fix off-by-one error tracking stdin line number

I think there are more off-by-one errors in the linenumber tracking code, combined with various +1/-1 adjustments in random places to correct for them. Much of this comes from #line directives containing the line number of the next line, not the line number of the #line directive itself.

@ChrisDodd
Copy link
Contributor Author

ChrisDodd commented May 9, 2017

The code for getting the source fragment of assignments also seems broken to me.

@jafingerhut
Copy link
Contributor

I am aware that the code for getting the source fragment of assignments often produces weird results, at least for assignments synthesized by the compiler.

It does produce good results for most assignments that appear in the original source code. I'm happy to open a separate issue to remember that those should be improved in some way.

@ChrisDodd
Copy link
Contributor Author

In general, you should be checking the SourceInfo objects with isValid() before trying to map them back to source code -- when isValid() is false, there's no source information (usually because the node was created by the compiler where it had no source info to put in it, but sometimes just because it failed to provide that argument to the constructor).

@ChrisDodd
Copy link
Contributor Author

In addition, you can "concatenate" SourceInfo objects with +. If both are valid the resulting SourceInfo will be from the start of the first one to the end of the second. If only one is valid, that one will be returned unchanged.

Copy link
Contributor

@jafingerhut jafingerhut left a comment

Choose a reason for hiding this comment

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

I haven't dug into the code around the change to the call to mapLine() to understand that change, but I have tried out this patch on the small public program that demonstrates the line == 2^32-1 issue, and it comes out better on that one, as well as on a private program I cannot share that had the same problem.

@jafingerhut
Copy link
Contributor

jafingerhut commented May 9, 2017

@ChrisDodd Thanks for the tip on "+" being defined for SourceInfo objects. I have a patch to the assignment code that uses that technique instead, which I think improves it. I will wait until this one is committed before opening a PR on that, if that is OK.

- Also fix off-by-one error tracking stdin line number
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.

3 participants