misc cmpIgnoreStyle + friends

[timotheecour]

links

• refactor cmpNimIdentifier by xflywind · Pull Request #16611 · nim-lang/Nim
• lib/pure/cstrutils.nim
• lib/std/private/strimpl.nim
• system.nim: proc cmp(x, y: string): int
• rfactor and fix the 5 different (buggy) re-implementations of cmpIgnoreStyle #472
• use cstrutils.cmpIgnoreStyle in typeinfo module by xflywind · Pull Request #16391 · nim-lang/Nim
• rfactor and fix the 5 different (buggy) re-implementations of cmpIgnoreStyle · Issue #472 · timotheecour/Nim

[timotheecour]

• lots of procs in std/private/strimpl should use a performance shortcut for the not-uncommon case where a == b (for the cstring case)

[timotheecour]

• cmpIgnoreStyleImpl is an exported template that uses return, IMO it's bad style and should be changed to:

template cmpIgnoreStyleImpl*[T: string | cstring](a, b: T, firstCharCaseSensitive: static bool = false): int =
  var result = 0
  ...
  result

otherwise its use is error prone and violates user expectations for control flow.

(if it were a non-exported template, it would be a bit less problematic since the code is self contained in same module)

[timotheecour]

• all procs taking cstring must handle nil properly, eg this fails:

import std/cstrutils
echo cmpIgnoreStyle("", nil) # SIGSEGV

[timotheecour]

• low priority: check whether some procs can use nimCmpMem + strchr for performance

[timotheecour]

• cstrutils.cmpIgnoreStyle doesn't ignore the case of 1st char, maybe we should deprecated it
  (ditto with friends)

[timotheecour]

• cmpIgnoreStyleImpl: change default firstCharCaseSensitive=false to firstCharCaseSensitive=true (less error prone, more consistent with nim's notion of style insensitivity) and adjust client code accordingly

[timotheecour]

• we should have tests for procs in lib/std/private/strimpl.nim, even if it's a private module

[timotheecour]

• idents.cmpIgnoreStyle should also be refactored; but its implementation seems suspicious eg:

proc cmpIgnoreStyle*(a, b: cstring, blen: int): int =
  if a[0] != b[0]: return 1
  ...

=> why 1?

[timotheecour]

• potential optimization for both cmpIgnoreStyle and cmpIgnoreCaseImpl (same/similar to one as done in getIdent): 1st check if nimCmpMem(a,b) == 0, in which case return 0 immediately