add no-copy ast.getStrVal for {nkIdent, nkSym, kStrLit..nkTripleStrLit}

[timotheecour]

(migrated from #17500 (comment))

add a no-copy getStrVal; see discussion in #17500 (comment)

[saem]

I don't see why we need to add this to ast, also introducing a global for it?

[timotheecour]

I don't see why we need to add this to ast, also introducing a global for it?

it simplifies client code, as you can see in compiler/renderer.nim, and macros.strVal defines something similar (i'm not considering the comments for now because comment node API may change in future as was discussed elsewhere).

The global is harmless, it's just private symbol which simplifies client code and allows safe zero-copy access.

[Clyybber]

I don't think it's worth it. The cases where you "need" a proc that gives you the strval regardless of wether it's a sym or and ident are rare (only two instances in renderer.nim).

[timotheecour]

no there are many other cases, I just didn't want to do the changes in the same PR.
eg:

    errorUndeclaredIdentifier(c, info.info, if n.kind == nkIdent: n.ident.s
      else: n.strVal)
=>
    errorUndeclaredIdentifier(c, info.info, n.getStrVal)

it'll also simplify cases like this for eg:

    case x.kind
    of nkIdent: id.add(x.ident.s)
    of nkSym: id.add(x.sym.name.s)

    let x = n[0]
    let ident = case x.kind
                of nkIdent: x.ident
                of nkSym: x.sym.name
                of nkClosedSymChoice, nkOpenSymChoice: x[0].sym.name
                else: nil
    if ident != nil and ident.s == "[]":
      let b = newNodeI(nkBracketExpr, n.info)

      # These vars are of type `cstring` to prevent unnecessary string copy.
      var aStrVal: cstring = nil
      var bStrVal: cstring = nil
      # extract strVal from argument ``a``
      case aNode.kind
      of nkStrLit..nkTripleStrLit:
        aStrVal = aNode.strVal.cstring
      of nkIdent:
        aStrVal = aNode.ident.s.cstring
      of nkSym:
        aStrVal = aNode.sym.name.s.cstring
      of nkOpenSymChoice, nkClosedSymChoice:
        aStrVal = aNode[0].sym.name.s.cstring
      else:
        discard
      # extract strVal from argument ``b``
      case bNode.kind
      of nkStrLit..nkTripleStrLit:
        bStrVal = bNode.strVal.cstring
      of nkIdent:
        bStrVal = bNode.ident.s.cstring
      of nkSym:
        bStrVal = bNode.sym.name.s.cstring
      of nkOpenSymChoice, nkClosedSymChoice:
        bStrVal = bNode[0].sym.name.s.cstring
      else:
        discard

as well as debugging

using accessors instead of raw field access is not something particularly surprising.

[Clyybber]

I think I'll retract #17500 (comment) then, because the template is still much better than this approach.

as well as debugging

You'd use $ for debugging.

[timotheecour]

the template causes a copy, whereas this PR doesn't.

You'd use $ for debugging.

different meaning, different semantics (eg for sym $ may show different results), both are useful; and $ also causes a copy.

[Araq]

The accessor could simply return the PIdent if it weren't for the strVal case which is however also much rarer. In general you cannot use string literals where an identifier is required in Nim.

[timotheecour]

The accessor could simply return the PIdent

=> #17684

In general you cannot use string literals where an identifier is required in Nim.

but there are plenty of existing, valid, cases in compiler where we do end up using the underlying string