nspcc-dev / neo-go

Go Node and SDK for the Neo blockchain
MIT License
123 stars 79 forks source link

Sequence point bounds don't respect indentation #3623

Closed AnnaShaleva closed 1 month ago

AnnaShaleva commented 1 month ago

Current Behavior

Take PutNamed method of NeoFS Container contract as an example: https://github.com/nspcc-dev/neofs-contract/blob/36e7d0728025db698379188116d5983bf46926c8/contracts/container/contract.go#L258

The first line of this method (L255): image Goland shows that ctx declaration starts at 255:5 and ends at 255:32. Debug info generated by our compiler shows that the boundaries of corresponding sequence point are "2056[38]255:2-255:29", i.e. 255:2 and 255:29. The more tabs in the line, the more debug info diverges from the real number of characters.

Try it with nested statements like this:

    if name != "" {
        if needRegister {
            res := contract.Call(nnsContractAddr, "register", contract.All,
                domain, runtime.GetExecutingScriptHash(), "ops@nspcc.ru",
                defaultRefresh, defaultRetry, defaultExpire, defaultTTL).(bool)
            if !res {
                panic("can't register the domain " + domain)
            }
        }

Expected Behavior

Sequence point boundaries from DebugInfo should match the source code boundaries in real life.

Possible Solution

I suppose that TABs and other non-space indent symbols should be blamed. But it needs to be checked. This but is not critical but it's not pleasant and complicates the debugging process.

roman-khimov commented 1 month ago

Goland shows that ctx declaration starts at 255:5

It's just wrong and we have correct data in debug info. NOBUG.

AnnaShaleva commented 1 month ago

It's just wrong

Well, Goland is not wrong, it actually shows the number of columns, not the number of characters. And the same thing does VS code, whereas C# compiler generates debug info that reflects the number of columns instead the number of characters (or may be these two numbers are equal for C# because they use a space as tabulation symbol).

But I took a look at real Go coverage output and turns out that Go coverage tool also outputs the number of characters instead of the number of columns. From this point NeoGo compiler behaviour matches Go coverage tool behaviour, thus it's really not a bug.

roman-khimov commented 1 month ago

Well, Goland is not wrong

It is wrong. Text files (source code is a text file) have bytes and characters. "Columns" are characters and nothing more than that. Your text editor chooses to display TAB as four spaces (which is also wrong, it SHOULD be eight), it's this editor problem that doesn't affect the file in any way. We can not refer to any display-level things in debug info, we can only operate the same bytes and characters because only they exist in text files.