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 .line pragma ignoring column information #860

Merged

Conversation

zerbina
Copy link
Collaborator

@zerbina zerbina commented Aug 26, 2023

Summary

Use the column information provided by the tuple value, which fixes,
for example, stack traces produced by assertion failures in compile-time
contexts pointing to the wrong column.

In addition, the tuple passed to the .line pragma is now verified to
have the correct number of elements, which fixes the compiler crashing
when it's a tuple with less than the expected number of elements.

Details

In pragmaLine:

  • don't allow nkPar -- all literal tuple construction expressions have
    to use nkTupleConstr nodes
  • require the provided tuple node to have exactly three elements
  • use more descriptive names instead of x and y
  • use the third tuple element's value (which is required to be an
    integer value) as the column information
  • build the TLineInfo with newLineInfo, which also makes sure
    that out-of-range line and column numbers are properly handled (by
    clamping them)
  • use the correct node when creating errors and use wrapError for
    proper propagation

In addition, the error message for when the provided tuple value doesn't
have the correct shape is slightly improved, now detailing how the
tuple is required to look like.

A test (t9768.nim) that relied on incorrect column being set is
adjusted.

Summary
=======

Use the column information provided by the tuple value, which fixes,
for example, stack traces produced by assertion failures in compile-time
contexts pointing to the wrong column.

In addition, the tuple passed to the `.line` pragma is now verified to
have the correct number of elements, which fixes the compiler crashing
when it's a tuple with only a single element.

Details
=======

In `pragmaLine`:
- don't allow `nkPar` -- all literal tuple construction expressions have
  to use `nkTupleConstr` nodes
- require the provided tuple node to have exactly three elements
- use more descriptive names instead of `x` and `y`
- take the column information from the third tuple element
- build the line information via `newLineInfo`, which also makes sure
  that out-of-range line and column numbers are properly handled (by
  clamping them)
- use the correct node when creating errors and use `wrapError` for
  proper propagation

In addition, the error message for when the provided tuple value doesn't
have the correct shape is slightly improved, now detailing how the
tuple is required to look like.

A test (`t9768.nim`) that relied on incorrect column being set is
adjusted.
@zerbina zerbina added bug Something isn't working refactor Implementation refactor compiler/sem Related to semantic-analysis system of the compiler labels Aug 26, 2023
@zerbina
Copy link
Collaborator Author

zerbina commented Aug 26, 2023

/merge

@saem: The change is relatively small, and in order to unblock #859, I'm merging this already.

@github-actions
Copy link

Merge requested by: @zerbina

Contents after the first section break of the PR description has been removed and preserved below:


Notes for Reviewers

@chore-runner chore-runner bot added this pull request to the merge queue Aug 26, 2023
Merged via the queue into nim-works:devel with commit 9efc16f Aug 26, 2023
18 checks passed
@zerbina zerbina deleted the use-column-info-for-line-pragma branch August 27, 2023 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compiler/sem Related to semantic-analysis system of the compiler refactor Implementation refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant