NEP: Source Mapping for NeoVM

[lightszero]

No description provided.

[erikzhang]

Needs to translate into English.

[Robinfang000]

https://github.com/Robinfang000/translation/blob/master/AVMMap-translated.mediawiki

[erikzhang]

Similar proposal: #87

[lock9]

Hello @lightszero,
Do you mind taking a look in #87? It was based on a known source mapping standard.
I understand that you already have this implemented, we also have done it differently and will have to change our code too.
Could we work together to approve a standard as soon as possible? Please? 🙏
Thanks!

[lightszero]

@lock9 sounds good,i will check #87

[lightszero]

@lock9 I see， you use "JavaScript Source Map" directly,It is fine to me.
I agree with you，a known standard is better.
I will change the proposal and add your name.

[lightszero]

@lock9 had fix,i agree most part of it.
with 2 change:
1.delete the reject col/line part, i think it is free for who make this file.
2.change this file end to .map.json

[erikzhang]

@lightszero @lock9 Any updates?

[lock9]

SORRY, I COMPLETELY FORGOT ABOUT THIS SUBJECT!
I agree with @lightszero, I'm going to add the changes he requested

[lock9]

Just one thing:
@lightszero in the docs, they mention why use columns and lines:
"Notes
Using file offsets were considered but rejected in favor of using line/column data to avoid becoming misaligned with the original due to platform specific line endings. "
However, our code uses offsets too, so it's totally fine for me!

Should we just replace the term "column" with "offset"? Should we force a specific line ending? (Unix?)

[lightszero]

@lock9
my friend

line ending could not cause line problem.

If you use line/cloumn to find a pos,whatever the line ending, is fine.

line always line,if you got wrong line,that is a code bug.

but you use offset to find a pos,line ending will cause problem,so you need to force a specific line ending.

I think no need to change line/cloumn thing
no need to force people use a specific line ending.

We can force the coder who parse txtfile to care about the BOM.
We can force the coder who parse txtfile to care about \n and \r\n.

No need to change format,that is a coder bussiness at all.

[lock9]

I'm sure you know a lot more about this subject than me, let's keep the way you think it is best.
Do you want me to submit changes to your PR? Do you want to send the changes yourself?

[lightszero]

@lock9 yes welcome to submit changes

[devhawk]

I've reviewed @lightszero PR neo-project/neo-devpack-dotnet#74 and updated myVSCode debugger to consume the source map format instead of my custom debug info format I have been experimenting with.I recommended that we DO NOT accept that PR at this time for reasons I will detail below. I also recommend that we remove the "accepted" tag from this PR for now.

Until we understand better what information is needed by the debugger to provide a good experience, it would be premature to settle on a file format to store that information.

The core problem is that the source map format isn't really designed to store the information that a NEO-VM debugger needs. Source maps are primarily designed for mapping across text formats, such as mapping between original and minimized JS or from TypeScript to compiled JS. For binary instruction to source mapping, especially with types, the source map format is missing a bunch information that the debugger needs to provide a good experience.

For example:

• A source map has no way to record parameter/variable information. This means there is no way for the debugger to map alt stack values back to local symbols that will be meaningful to the user.
• A source map has no way to record any type information. This means the debugger can only rely on the type information the VM has for displaying variable values. However, there are cases where a type in the VM can be interpreted different ways. For example, since integers greater that 16 are stored as byte arrays, there's no way for the debugger to know if the developer is expecting a byte array value to be shown as an integer or not.
• A PDB sequence point maps an IL instruction to a range of text in a source code file (start line/column and end line/column). Source maps can only map to a specific position (line/column) in the source code file. This means the debugger cannot highlight the range of text the instruction represents the way a C# developer would expect.
• There is no way for the debugger to know what functions instructions map to if there isn't a mapping entry for that specific instruction. If an instruction falls between two map points, each associated with a different function, how is the debugger supposed to tell which function it's in? It needs to know in order to be able to select the next mapping entry to step to.

[erikzhang]

Don't worry. We can modify the design before the Final status.

[lightszero]

@devhawk I Agree all your suggestion.
maybe we can use some special filenames in sourcemap file to storage that
like:
abc.cs = default addr->line/col map info
!abc.cs_params = params info
!abc.cs_ranges = code range

[devhawk]

Can we close this? The format used by NEON/Neo Smart Contract debugger is documented elsewhere

[ixje]

Can we close this? The format used by NEON/Neo Smart Contract debugger is documented elsewhere

Can we move/merge them into a NEP? Otherwise we're going to have multiple places describing enhancements. It would be nice to keep them all in one place. NEPs have been the default up to now.

[devhawk]

Can we move/merge them into a NEP?

Happy to write up a new NEP for the debug format. Do I just pick the next available number and submit a PR?

[erikzhang]

Do I just pick the next available number and submit a PR?

I will assign a number when the proposal is accepted.

[erikzhang]

@lightszero @lock9 @devhawk Should we merge it?

[devhawk]

This PR should not be merged. The debug info format used by the Neo Smart Contract Debugger in the Neo Blockchain Toolkit is documented here: https://github.com/ngdenterprise/design-notes/blob/master/NDX-DN11%20-%20NEO%20Debug%20Info%20Specification.md.

Happy to bring that over and create a new NEP.

[erikzhang]

Happy to bring that over and create a new NEP.

Yes, please.

[erikzhang]

@devhawk Any update?

[devhawk]

#140