Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jul 24, 2025

Stack traces were displaying both line and column numbers using 0-based indexing instead of the standard 1-based indexing expected by users and editors.

Problem

When a runtime error occurred, the stack trace would report incorrect line and column numbers that were off by one:

function Main() : Unit {
    fail "line 2";  // This is line 2, column 5 (1-based counting)
}

Before fix:

Error: program failed: line 2
Call stack:
    at Main in test.qs:1:4  // Wrong! Shows line 1, column 4

After fix:

Error: program failed: line 2
Call stack:
    at Main in test.qs:2:5  // Correct! Shows line 2, column 5

Root Cause

The format_call_stack function in source/compiler/qsc/src/interpret/debug.rs was displaying pos.line and pos.column (which are 0-based for internal calculations) directly to users. Both line and column numbers should be 1-based for user-facing display as this is the standard convention used by editors and IDEs.

Solution

Updated both line and column number display to add 1 to convert from internal 0-based indexing to user-expected 1-based indexing:

// Before
pos.line,
pos.column,

// After  
pos.line + 1,
pos.column + 1,

This change ensures that both line and column numbers are displayed correctly to users while preserving the internal 0-based indexing used throughout the codebase.

Testing

  • Updated existing stack trace test expectations to reflect correct 1-based line and column numbers
  • All 158 qsc package tests pass
  • All debug-specific tests pass

Fixes #2627.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

@Copilot Copilot AI changed the title [WIP] Off by one error in stack traces Fix off-by-one error in stack traces by making line numbers 1-based for user display Jul 24, 2025
@Copilot Copilot AI requested a review from minestarks July 24, 2025 21:11
Copilot finished work on behalf of minestarks July 24, 2025 21:11
@minestarks
Copy link
Member

@copilot can you add 1 to the column as well? Also, I don't think you need to add any new tests for this, just update the existing ones

Copy link
Contributor Author

Copilot AI commented Jul 30, 2025

@copilot can you add 1 to the column as well? Also, I don't think you need to add any new tests for this, just update the existing ones

Done! Added 1 to column positions as well to make them 1-based and removed the new tests, keeping only the updated existing test. All 157 qsc tests pass. (commit 62e0d53)

@Copilot Copilot AI changed the title Fix off-by-one error in stack traces by making line numbers 1-based for user display Fix off-by-one error in stack traces by making line and column numbers 1-based for user display Jul 30, 2025
Copilot finished work on behalf of minestarks July 30, 2025 17:42
Copilot finished work on behalf of minestarks August 6, 2025 23:11
@Copilot Copilot AI requested a review from minestarks August 6, 2025 23:11
…ons with 1-based line/column numbers

Co-authored-by: minestarks <[email protected]>
Copilot finished work on behalf of minestarks August 8, 2025 19:17
@minestarks
Copy link
Member

@copilot can you pull from main and resolve conflicts?

Copy link
Contributor Author

Copilot AI commented Aug 8, 2025

@copilot can you pull from main and resolve conflicts?

Pulled from main and resolved merge conflicts in the test snapshots. Updated the test expectations to reflect the correct 1-based line and column numbers after the merge. All 159 qsc tests pass. (commit e00c183)

Copilot finished work on behalf of minestarks August 8, 2025 21:31
@minestarks minestarks marked this pull request as ready for review August 8, 2025 21:47
@minestarks minestarks added this pull request to the merge queue Aug 13, 2025
Merged via the queue into main with commit a3590c8 Aug 13, 2025
19 checks passed
@minestarks minestarks deleted the copilot/fix-2627 branch August 13, 2025 17:09
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.

Off by one error in stack traces
3 participants