docgen: column reporting is wrong

[timotheecour]

/cc @a-mr

Example

see example mentioned in nim-lang#17338 (comment)

discussion

I tried to put the line and column of Doc comment itself to n.info but immediately got a problem with nimpretty.

PNode has a info*: TLineInfo so adding fields to TLineInfo is IMO not an option as it'd increase memory use; PNode is the most common type generated by compiler; see:

• [superseded] move comment field out of TNode => sizeof(TNode) = 32 instead of 40 nim-lang/Nim#10054
• use a comments: Table[PNode, string] to store comment fields? nim-lang/compilerdev#13

adding fields conditionally (on nimdoc) would maybe be worth investigating but i have a feeling this won't solve the problem and the problem exists also for nim c

proposal

IMO the "wrong column reporting" problem can be solved as follows:

• remove comment field from PNode (either via finishing [superseded] move comment field out of TNode => sizeof(TNode) = 32 instead of 40 nim-lang/Nim#10054 or use a comments: Table[PNode, string] to store comment fields? nim-lang/compilerdev#13)
• add fields to a comment node so that comment nodes have sufficient information to get column info; for example we could preserve the original un-processed string: the raw string would contain the full comment including the ## or ##[

##[
comment 1
]##

## comment 2

##comment 3

#comment 4 (maybe also non-doc comments)

benefits:

• reduces memory consumption by compiler (40B => 32B) since PNode is the most frequent type
• solves column reporting
• allows faithful rendering of comments via repr since no information is lost
• maybe also for nimpretty (need to check)

links

• Rst test check messages (fix #17280) nim-lang/Nim#17338 discusses this issue in several places
• commentOffsetA (eg Fix commentOffsetA for doc comments nim-lang/Nim#15643) related but different issue

[a-mr]

I actually meant reusing existing fields line and col.

I added code around https://github.com/nim-lang/Nim/blob/4bfc5a955120b90faf07f50b54ae29f2c6e6f123/compiler/lexer.nim#L966 for both lines and columns like

      while L.buf[pos] in {CR, LF}:  # skip blank lines
+       inc tok.line
        pos = handleCRLF(L, pos)
        toStrip = 0
        while L.buf[pos] == ' ':
          inc pos
          inc toStrip
+     tok.col = toStrip

That's actually token but it gets into PNode.info by some way eventually.

That solved the problem of preserving information and I actually checked that nim doc started to work correctly (after removing the previous workaround from docgen.nim), but broke lines (not columns though in nimpretty).

The option to do it only in the nimdoc mode should solve the problem. Though I'm not sure about other problems that may arise: e.g. error reporting for nim syntax related with comments may become broken (just a guess).

[a-mr]

There is also another option, which is a bit ugly but seems quite safe: add convention to pass missing arguments in the string itself at its first line: number of additional lines first, comma, additional columns.

So this doc comment

##[
    comment 1
    comment 2
]##

will be turned into the string

1,4
comment 1
comment 2

After that 1 and 4 are extracted and n.line+1 and n.col+4 calculated in docgen.nim.

Of course, option like skipFirstLine should be added to rst.nim.