Nim: Generated c code calls function twice

[ghost]

Consider this code:

import strutils

var i = "foo/bar/baz/"
var k = i.split('/')[^1]

The nim compiler produces this C code:

NIM_EXTERNC N_NOINLINE(void, NimMainModule)(void) {
    NimStringDesc* T1_;
    tySequence_sM4lkSb7zS6F7OVMvW9cffQ* T2_;
    tySequence_sM4lkSb7zS6F7OVMvW9cffQ* T3_;
    NI T4_;
    NimStringDesc* T5_;
nimRegisterGlobalMarker(TM_nQgtNLQ9ao78R09bsFZSJvZw_2);
nimRegisterGlobalMarker(TM_nQgtNLQ9ao78R09bsFZSJvZw_4);
    T1_ = (NimStringDesc*)0;
    T1_ = i_H9azGQoUQj9biodAAFcnb5QQ; i_H9azGQoUQj9biodAAFcnb5QQ = copyStringRC1(((NimStringDesc*) &TM_nQgtNLQ9ao78R09bsFZSJvZw_3));
    if (T1_) nimGCunrefNoCycle(T1_);
    T2_ = (tySequence_sM4lkSb7zS6F7OVMvW9cffQ*)0;
    T2_ = nsuSplitChar(i_H9azGQoUQj9biodAAFcnb5QQ, 47, ((NI) -1));
    T3_ = (tySequence_sM4lkSb7zS6F7OVMvW9cffQ*)0;
    T3_ = nsuSplitChar(i_H9azGQoUQj9biodAAFcnb5QQ, 47, ((NI) -1));
    T4_ = (T3_ ? T3_->Sup.len : 0);
    T5_ = (NimStringDesc*)0;
    T5_ = k_NTt5pf6wTYEuyE9bCTJmA4w; k_NTt5pf6wTYEuyE9bCTJmA4w = copyStringRC1(T2_->data[(NI)(T4_ - ((NI) 1))]);
    if (T5_) nimGCunrefNoCycle(T5_);
}

As you can see the nsuSplitChar function is called twice.
After commit 920f4ac the compiler produces an extra temp variable (T4_ in this case) so I think the second function call can be omitted.

[Yardanico]

@konqoro well yeah, it needs to be optimized, but C compiler itself can easily optimize this way

[ghost]

@Yardanico unfortunately it doesn't look this way try this:

proc split(s: string, sep: char, maxsplit: int = -1): seq[string] =
  result = @[]
  ## Common code for split procedures
  var last = 0
  var splits = maxsplit

  if len(s) > 0:
    while last <= len(s):
      var first = last
      while last < len(s) and not (s[last] == sep):
        inc(last)
      if splits == 0: last = len(s)
      result.add substr(s, first, last-1)
      if splits == 0: break
      dec(splits)
      inc(last)
  echo result

var i = "foo/bar/baz/"
var k = i.split('/')[^1]

I get: @[foo, bar, baz, ]\n@[foo, bar, baz, ]

[yglukhov]

Imo, this is a pretty serious issue, since procs with side effects are also called twice.

[andreaferretti]

Just recording that @Araq suggested on IRC to move ^ in the stdlib, by making it returm something like Reverse[T], and then have accessors [] that take a Reverse[T].

Probably something like (untested)

type Reverse[T] = distinct T

proc `^`[T](t: T): Reverse[T] = Reverse[T](t)

template `[]`[T](s: untyped, r: Reverse[T]) =
  let temp = s # force evaluation
  temp[temp.len - T(r)]

Also, Reverse[T] should work with slices .. and so on