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

TEAL: Report columns in TEAL source maps #5776

Merged
merged 7 commits into from
Oct 11, 2023

Conversation

jasonpaulos
Copy link
Contributor

@jasonpaulos jasonpaulos commented Oct 5, 2023

Summary

Prior to this, TEAL source maps would only map PCs to line numbers, always assuming the column was 0. Since multiple opcodes can now be placed on a single line, separated by ;, it makes sense to also report the source column as well.

Additionally, I also made the path to the source file relative to the source map file, which is what the specification requires (see Resolving Sources). Prior to this PR, we were just using whatever the user input as the source program path.

Test Plan

Existing unit tests updated and added an additional e2e test to expand coverage of more complicated cases.

@jasonpaulos jasonpaulos self-assigned this Oct 5, 2023
@codecov
Copy link

codecov bot commented Oct 5, 2023

Codecov Report

Merging #5776 (43f4090) into master (1a6d53e) will increase coverage by 3.94%.
The diff coverage is 65.27%.

@@            Coverage Diff             @@
##           master    #5776      +/-   ##
==========================================
+ Coverage   51.51%   55.45%   +3.94%     
==========================================
  Files         473      473              
  Lines       66688    66708      +20     
==========================================
+ Hits        34354    36993    +2639     
+ Misses      29742    27194    -2548     
+ Partials     2592     2521      -71     
Files Coverage Δ
cmd/tealdbg/local.go 73.88% <100.00%> (ø)
data/transactions/logic/sourcemap.go 100.00% <100.00%> (+52.94%) ⬆️
daemon/algod/api/server/v2/handlers.go 0.79% <0.00%> (ø)
cmd/tealdbg/debugger.go 74.41% <81.81%> (+8.97%) ⬆️
data/transactions/logic/assembler.go 93.98% <75.00%> (+4.88%) ⬆️
cmd/goal/clerk.go 10.25% <37.50%> (+0.83%) ⬆️

... and 150 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

jannotti
jannotti previously approved these changes Oct 5, 2023
cmd/tealdbg/debugger_test.go Outdated Show resolved Hide resolved
* Use the OpStream's OffSetToSource directly

* Use deltas properly
jannotti
jannotti previously approved these changes Oct 6, 2023
@jannotti jannotti requested a review from gmalouf October 10, 2023 15:11
@jasonpaulos jasonpaulos force-pushed the teal-source-map-report-columns branch from aff88d2 to 93ae432 Compare October 10, 2023 19:55
@jasonpaulos jasonpaulos reopened this Oct 10, 2023
@jasonpaulos jasonpaulos requested a review from jannotti October 10, 2023 20:50
jannotti
jannotti previously approved these changes Oct 10, 2023
cmd/goal/clerk.go Outdated Show resolved Hide resolved
cmd/goal/clerk.go Outdated Show resolved Hide resolved
gmalouf
gmalouf previously approved these changes Oct 11, 2023
cmd/goal/clerk.go Outdated Show resolved Hide resolved
data/transactions/logic/assembler.go Show resolved Hide resolved
@jasonpaulos jasonpaulos dismissed stale reviews from gmalouf and jannotti via 43f4090 October 11, 2023 16:33
@gmalouf gmalouf merged commit 92ba7ee into algorand:master Oct 11, 2023
@jasonpaulos jasonpaulos deleted the teal-source-map-report-columns branch October 11, 2023 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants