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

Fix incorrect line numbers in linemarkers #608

Merged
merged 1 commit into from
Dec 30, 2023

Conversation

squeek502
Copy link
Contributor

@squeek502 squeek502 commented Dec 29, 2023

Line numbers are 1-indexed, not 0-indexed. Before this commit, include_start/include_resume tokens were being added with a mix of 1-indexed and 0-indexed line numbers. After this commit, all line numbers are 1-indexed.

Closes #604

I would have added test cases, but there's nothing in place for testing linemarkers (if I add a test case, the file paths in the line markers end up as absolute). For example, this test case:

//aro-args -E -fuse-line-directives
this is line 2







this is line 10

Expects this output:

#line 2 "/home/ryan/Programming/zig/arocc/test/cases/linemarkers.c"
this is line 2
#line 10 "/home/ryan/Programming/zig/arocc/test/cases/linemarkers.c"
this is line 10

Let me know if you have an idea of how to add test cases for this.


Fixes all the reproductions of the problem I was able to find.


#include "a.h"


foo

Now correctly outputs:

#line 1 "test.c"
#line 1 "<builtin>"
#line 293 "<builtin>"
#line 1 "<command line>"
#line 1 "test.c"
#line 1 "./a.h"

#line 4 "test.c"
foo

this is line 1








this is line 10

Now correctly outputs:

#line 1 "test.c"
#line 1 "<builtin>"
#line 293 "<builtin>"
#line 1 "<command line>"
#line 1 "test.c"
this is line 1
#line 10 "test.c"
this is line 10

Line numbers are 1-indexed, not 0-indexed. Before this commit, include_start/include_resume tokens were being added with a mix of 1-indexed and 0-indexed line numbers. After this commit, all line numbers are 1-indexed.

Closes Vexu#604
@Vexu Vexu merged commit 27411b5 into Vexu:master Dec 30, 2023
3 checks passed
@Vexu
Copy link
Owner

Vexu commented Dec 30, 2023

Thanks, looks like I confused myself by not being able to decide whether to zero or one index the line numbers when implementing linemarkers.

squeek502 added a commit to squeek502/zig that referenced this pull request Mar 2, 2024
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.

Incorrect line directives after #include line
2 participants