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

viper: error source mappings in moc.js #3501

Merged
merged 9 commits into from
Oct 21, 2022
Merged

viper: error source mappings in moc.js #3501

merged 9 commits into from
Oct 21, 2022

Conversation

rvanasa
Copy link
Contributor

@rvanasa rvanasa commented Oct 20, 2022

Passes the lookup function through moc.js to facilitate source mapping in VS Code.

@rvanasa rvanasa changed the base branch from master to viper October 20, 2022 04:46
@rvanasa
Copy link
Contributor Author

rvanasa commented Oct 21, 2022

There is currently a bug hiding somewhere in the way that moc.js initializes or calls the lookup function from Viper.Pretty.prog_mapped.

I expect this to be a quick fix tomorrow, but I would appreciate a second pair of eyes in case anyone gets a chance to skim this PR. This is most likely the final step to a working demo for source locations.

Here's a command to reproduce the issue:

make -C src moc.js && node -e "const {Motoko} = require('./src/moc.js'); Motoko.saveFile('File.mo', 'actor { public func abc(): async () { } };'); console.log('Result:', Motoko.viper(['File.mo']).code.lookup('', [0,0,0,0]))"

Replacing ...lookup('', [0,0,0,0])) with any other values (filename, [leftLine, leftCol, rightLine, rightCol]) always seems to return null (indicating None is returned from the lookup function). This probably has something to do with this PR's changes in Pipeline.viper_files', since I checked that the js_of_ocaml conversions work as expected.

Does anything stand out as the issue? Otherwise I'll continue narrowing this down tomorrow morning.

@@ -494,14 +494,12 @@ let viper_files' parsefn files : viper_result =
let prog = CompUnit.combine_progs progs in
let u = CompUnit.comp_unit_of_prog false prog in
let v = Viper.Trans.unit u in
let s = Viper.Pretty.prog v in
let s = Viper.Pretty.prog_mapped "" v in
Copy link
Contributor

@ggreif ggreif Oct 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The filename passed in must be the one that the Viper-side diagnostic points to.

@ggreif
Copy link
Contributor

ggreif commented Oct 21, 2022

Does anything stand out as the issue?

My hunch is that the let inside function in Pretty.prog_mapped takes filenames in account, so that may fail. We could beef up inside to skip the name comparison when the filename passed to Pretty.prog_mapped is empty.

EDIT: Oh, I see, passing "" in already effectively deactivates the comparison.

2nd hunch: reverse mappings only exist for Motoko/Viper statements, but your code has an empty body.

Found a hit:

make -C src moc.js && node -e "const {Motoko} = require('./src/moc.js'); Motoko.saveFile('File.mo', 'actor { var v = -1; public func abc(): async () { v := 1; v := 12; v := 14; v := 15; v := 16; v := 19; } };'); console.log('Result:', Motoko.viper(['File.mo']).code.lookup('', [13,6,13,6]))"

gives

Result: { start: { line: 0, character: 94 }, end: { line: 0, character: 101 } }

Which corresponds to the statement v := 19. Previous lines accordingly (e.g. [8,5,8,19] --> v := 1). So there is no bug at all, you only have to poke at the right place. Good work!

@ggreif ggreif closed this Oct 21, 2022
@ggreif ggreif reopened this Oct 21, 2022
src/exes/moc.ml Outdated Show resolved Hide resolved
@ggreif
Copy link
Contributor

ggreif commented Oct 21, 2022

@rvanasa can this go in?

@rvanasa rvanasa marked this pull request as ready for review October 21, 2022 17:31
Copy link
Contributor

@ggreif ggreif left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ggreif ggreif merged commit fee72de into viper Oct 21, 2022
@rvanasa
Copy link
Contributor Author

rvanasa commented Oct 21, 2022

Merging. If anything needs a quick fix, I'll just commit directly to the viper branch.

@rvanasa rvanasa deleted the ryan/viper branch October 21, 2022 17:36
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.

4 participants