From ed5b7cbac004551d77db889eca044fdda6f75790 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arne=20D=C3=B6ring?= Date: Sun, 15 Apr 2018 23:38:43 +0200 Subject: [PATCH] move eqIdent to vm.nim (#7585) * Strutils comment changes. * fix typo --- compiler/condsyms.nim | 1 + compiler/idents.nim | 2 +- compiler/vm.nim | 32 ++++++++++++++-- lib/core/macros.nim | 79 +++++++++++++++++++++++++--------------- lib/pure/cstrutils.nim | 6 ++- lib/pure/strutils.nim | 11 +++--- tests/macros/tmacro1.nim | 42 +++++++++++++++++++++ 7 files changed, 132 insertions(+), 41 deletions(-) diff --git a/compiler/condsyms.nim b/compiler/condsyms.nim index 85ebdd7bf3..028aedb5b5 100644 --- a/compiler/condsyms.nim +++ b/compiler/condsyms.nim @@ -114,3 +114,4 @@ proc initDefines*() = defineSymbol("nimNewDot") defineSymbol("nimHasNilChecks") defineSymbol("nimSymKind") + defineSymbol("nimVmEqIdent") diff --git a/compiler/idents.nim b/compiler/idents.nim index 2cce4710e4..34cf5bf1eb 100644 --- a/compiler/idents.nim +++ b/compiler/idents.nim @@ -37,7 +37,7 @@ proc resetIdentCache*() = for i in low(legacy.buckets)..high(legacy.buckets): legacy.buckets[i] = nil -proc cmpIgnoreStyle(a, b: cstring, blen: int): int = +proc cmpIgnoreStyle*(a, b: cstring, blen: int): int = if a[0] != b[0]: return 1 var i = 0 var j = 0 diff --git a/compiler/vm.nim b/compiler/vm.nim index 7e2a171a2e..09226988a6 100644 --- a/compiler/vm.nim +++ b/compiler/vm.nim @@ -1392,10 +1392,36 @@ proc rawExecute(c: PCtx, start: int, tos: PStackFrame): TFullReg = regs[ra].node.typ = n.typ of opcEqIdent: decodeBC(rkInt) - if regs[rb].node.kind == nkIdent and regs[rc].node.kind == nkIdent: - regs[ra].intVal = ord(regs[rb].node.ident.id == regs[rc].node.ident.id) + # aliases for shorter and easier to understand code below + let aNode = regs[rb].node + let bNode = regs[rc].node + # these are cstring to prevent string copy, and cmpIgnoreStyle from + # takes cstring arguments + var aStrVal: cstring + var bStrVal: cstring + # 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 else: - regs[ra].intVal = 0 + stackTrace(c, tos, pc, errFieldXNotFound, "strVal") + # 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 + else: + stackTrace(c, tos, pc, errFieldXNotFound, "strVal") + # set result + regs[ra].intVal = + ord(idents.cmpIgnoreStyle(aStrVal,bStrVal,high(int)) == 0) of opcStrToIdent: decodeB(rkNode) if regs[rb].node.kind notin {nkStrLit..nkTripleStrLit}: diff --git a/lib/core/macros.nim b/lib/core/macros.nim index e76e9241f4..7217475c61 100644 --- a/lib/core/macros.nim +++ b/lib/core/macros.nim @@ -1186,38 +1186,57 @@ proc copy*(node: NimNode): NimNode {.compileTime.} = ## An alias for copyNimTree(). return node.copyNimTree() -proc cmpIgnoreStyle(a, b: cstring): int {.noSideEffect.} = - proc toLower(c: char): char {.inline.} = - if c in {'A'..'Z'}: result = chr(ord(c) + (ord('a') - ord('A'))) - else: result = c - var i = 0 - var j = 0 - # first char is case sensitive - if a[0] != b[0]: return 1 - while true: - while a[i] == '_': inc(i) - while b[j] == '_': inc(j) # BUGFIX: typo - var aa = toLower(a[i]) - var bb = toLower(b[j]) - result = ord(aa) - ord(bb) - if result != 0 or aa == '\0': break - inc(i) - inc(j) +when defined(nimVmEqIdent): + proc eqIdent*(a: string; b: string): bool {.magic: "EqIdent", noSideEffect.} + ## Style insensitive comparison. -proc eqIdent*(a, b: string): bool = cmpIgnoreStyle(a, b) == 0 - ## Check if two idents are identical. + proc eqIdent*(a: NimNode; b: string): bool {.magic: "EqIdent", noSideEffect.} + ## Style insensitive comparison. + ## ``a`` can be an identifier or a symbol. -proc eqIdent*(node: NimNode; s: string): bool {.compileTime.} = - ## Check if node is some identifier node (``nnkIdent``, ``nnkSym``, etc.) - ## is the same as ``s``. Note that this is the preferred way to check! Most - ## other ways like ``node.ident`` are much more error-prone, unfortunately. - case node.kind - of nnkSym, nnkIdent: - result = eqIdent(node.strVal, s) - of nnkOpenSymChoice, nnkClosedSymChoice: - result = eqIdent($node[0], s) - else: - result = false + proc eqIdent*(a: string; b: NimNode): bool {.magic: "EqIdent", noSideEffect.} + ## Style insensitive comparison. + ## ``b`` can be an identifier or a symbol. + + proc eqIdent*(a: NimNode; b: NimNode): bool {.magic: "EqIdent", noSideEffect.} + ## Style insensitive comparison. + ## ``a`` and ``b`` can be an identifier or a symbol. + +else: + # this procedure is optimized for native code, it should not be compiled to nimVM bytecode. + proc cmpIgnoreStyle(a, b: cstring): int {.noSideEffect.} = + proc toLower(c: char): char {.inline.} = + if c in {'A'..'Z'}: result = chr(ord(c) + (ord('a') - ord('A'))) + else: result = c + var i = 0 + var j = 0 + # first char is case sensitive + if a[0] != b[0]: return 1 + while true: + while a[i] == '_': inc(i) + while b[j] == '_': inc(j) # BUGFIX: typo + var aa = toLower(a[i]) + var bb = toLower(b[j]) + result = ord(aa) - ord(bb) + if result != 0 or aa == '\0': break + inc(i) + inc(j) + + + proc eqIdent*(a, b: string): bool = cmpIgnoreStyle(a, b) == 0 + ## Check if two idents are identical. + + proc eqIdent*(node: NimNode; s: string): bool {.compileTime.} = + ## Check if node is some identifier node (``nnkIdent``, ``nnkSym``, etc.) + ## is the same as ``s``. Note that this is the preferred way to check! Most + ## other ways like ``node.ident`` are much more error-prone, unfortunately. + case node.kind + of nnkSym, nnkIdent: + result = eqIdent(node.strVal, s) + of nnkOpenSymChoice, nnkClosedSymChoice: + result = eqIdent($node[0], s) + else: + result = false proc hasArgOfName*(params: NimNode; name: string): bool {.compiletime.}= ## Search nnkFormalParams for an argument. diff --git a/lib/pure/cstrutils.nim b/lib/pure/cstrutils.nim index 4371408925..fe9ceb68b6 100644 --- a/lib/pure/cstrutils.nim +++ b/lib/pure/cstrutils.nim @@ -45,8 +45,10 @@ proc endsWith*(s, suffix: cstring): bool {.noSideEffect, proc cmpIgnoreStyle*(a, b: cstring): int {.noSideEffect, rtl, extern: "csuCmpIgnoreStyle".} = - ## Compares two strings normalized (i.e. case and - ## underscores do not matter). Returns: + ## Semantically the same as ``cmp(normalize($a), normalize($b))``. It + ## is just optimized to not allocate temporary strings. This should + ## NOT be used to compare Nim identifier names. use `macros.eqIdent` + ## for that. Returns: ## ## | 0 iff a == b ## | < 0 iff a < b diff --git a/lib/pure/strutils.nim b/lib/pure/strutils.nim index 7b8a9a0d04..d772b066c2 100644 --- a/lib/pure/strutils.nim +++ b/lib/pure/strutils.nim @@ -385,8 +385,8 @@ proc normalize*(s: string): string {.noSideEffect, procvar, rtl, extern: "nsuNormalize".} = ## Normalizes the string `s`. ## - ## That means to convert it to lower case and remove any '_'. This is needed - ## for Nim identifiers for example. + ## That means to convert it to lower case and remove any '_'. This + ## should NOT be used to normalize Nim identifier names. result = newString(s.len) var j = 0 for i in 0..len(s) - 1: @@ -418,8 +418,10 @@ proc cmpIgnoreCase*(a, b: string): int {.noSideEffect, proc cmpIgnoreStyle*(a, b: string): int {.noSideEffect, rtl, extern: "nsuCmpIgnoreStyle", procvar.} = - ## Compares two strings normalized (i.e. case and - ## underscores do not matter). Returns: + ## Semantically the same as ``cmp(normalize(a), normalize(b))``. It + ## is just optimized to not allocate temporary strings. This should + ## NOT be used to compare Nim identifier names. use `macros.eqIdent` + ## for that. Returns: ## ## | 0 iff a == b ## | < 0 iff a < b @@ -436,7 +438,6 @@ proc cmpIgnoreStyle*(a, b: string): int {.noSideEffect, inc(i) inc(j) - proc strip*(s: string, leading = true, trailing = true, chars: set[char] = Whitespace): string {.noSideEffect, rtl, extern: "nsuStrip".} = diff --git a/tests/macros/tmacro1.nim b/tests/macros/tmacro1.nim index ac6bd02a51..0b83345a14 100644 --- a/tests/macros/tmacro1.nim +++ b/tests/macros/tmacro1.nim @@ -18,5 +18,47 @@ macro test*(a: untyped): untyped = t.b = true t.z = 4.5 + test: "hi" + +import strutils + +template assertNot(arg: untyped): untyped = + assert(not(arg)) + +static: + ## test eqIdent + let a = "abc_def" + let b = "abcDef" + let c = "AbcDef" + + assert eqIdent( a , b ) + assert eqIdent(newIdentNode(a), b ) + assert eqIdent( a , newIdentNode(b)) + assert eqIdent(newIdentNode(a), newIdentNode(b)) + + assert eqIdent( a , b ) + assert eqIdent(genSym(nskLet, a), b ) + assert eqIdent( a , genSym(nskLet, b)) + assert eqIdent(genSym(nskLet, a), genSym(nskLet, b)) + + assert eqIdent(newIdentNode( a), newIdentNode( b)) + assert eqIdent(genSym(nskLet, a), newIdentNode( b)) + assert eqIdent(newIdentNode( a), genSym(nskLet, b)) + assert eqIdent(genSym(nskLet, a), genSym(nskLet, b)) + + assertNot eqIdent( c , b ) + assertNot eqIdent(newIdentNode(c), b ) + assertNot eqIdent( c , newIdentNode(b)) + assertNot eqIdent(newIdentNode(c), newIdentNode(b)) + + assertNot eqIdent( c , b ) + assertNot eqIdent(genSym(nskLet, c), b ) + assertNot eqIdent( c , genSym(nskLet, b)) + assertNot eqIdent(genSym(nskLet, c), genSym(nskLet, b)) + + assertNot eqIdent(newIdentNode( c), newIdentNode( b)) + assertNot eqIdent(genSym(nskLet, c), newIdentNode( b)) + assertNot eqIdent(newIdentNode( c), genSym(nskLet, b)) + assertNot eqIdent(genSym(nskLet, c), genSym(nskLet, b))