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 reported file in error message when missing a required semicolon. #331

Merged
merged 3 commits into from May 7, 2019

Conversation

@Fyren
Copy link
Contributor

@Fyren Fyren commented May 7, 2019

This would only happen with #pragma semicolon 1 if a semicolon was missing after some other file was included.

Without the change, the error reported for the test case would be in the empty include (though with the correct line number).

@peace-maker
Copy link
Member

@peace-maker peace-maker commented May 7, 2019

Are you sure the test is correct? You're including a nonexistent file. Maybe I'm not understanding the bug here.

@Fyren
Copy link
Contributor Author

@Fyren Fyren commented May 7, 2019

I'm including an almost empty file. Try the test against master (i.e. without this PR) and the error reported will be in the empty file.

Almost empty because apparently if you try to include an actual 0-byte file, spcomp will error.

@asherkin
Copy link
Member

@asherkin asherkin commented May 7, 2019

I'm including an almost empty file. Try the test against master (i.e. without this PR) and the error reported will be in the empty file.

I think it got renamed at one point, the filename here does indeed seem wrong.

@Fyren
Copy link
Contributor Author

@Fyren Fyren commented May 7, 2019

I did rename it, but the test still passed because the check was lax. Fixed, thanks.

@Fyren Fyren merged commit ac7b6b7 into master May 7, 2019
2 checks passed
@Fyren Fyren deleted the fix-reported-file-semicolon-error branch May 7, 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
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants