delete a lot of dead code #11981

krux02 · 2019-08-19T22:01:19Z

This PR is ruthless, when some code is deactivated with when false, I deleted it. One reason this PR is here is to show how much dead code has been accumulated over the years. I think at some point one should just dump it all.

narimiran · 2019-08-21T06:58:53Z

compiler/ccgexprs.nim

-    else:
-      genAssignment(p, d, tmpB, {})
-  else:
+  when true:


Nitpick: you could do the same as in https://github.com/nim-lang/Nim/pull/11981/files#diff-c771980a90bdbd4401f414dee2b14915R32 — without when true, just unindent its block. I think it is a cleaner solution.

yea I can. I decided against it because in such a big PR I wanted to focus on trivial code changes. In the code you reference I wasn't so strict yet.

juancarlospaco · 2019-08-21T16:22:02Z

Awesome work. 🙂 👍

timotheecour · 2019-08-21T23:44:43Z

compiler/parser.nim

@@ -1455,14 +1455,6 @@ proc parseExprStmt(p: var TParser): PNode =

 proc parseModuleName(p: var TParser, kind: TNodeKind): PNode =
  result = parseExpr(p)
-  when false:
-    # parseExpr already handles 'as' syntax ...


maybe keep the comment # parseExpr already handles 'as' syntax ...

timotheecour · 2019-08-21T23:47:34Z

compiler/sem.nim

@@ -150,15 +149,6 @@ proc commonType*(x, y: PType): PType =
    # run people expect ranges to work properly within a tuple.
    if not sameType(a, b):
      result = skipTypes(a, {tyRange}).skipIntLit
-    when false:
-      if a.kind != tyRange and b.kind == tyRange:
-        # XXX This really needs a better solution, but a proper fix now breaks


maybe at least summarize this deleted block with a comment

timotheecour · 2019-08-21T23:52:49Z

compiler/semobjconstr.nim

@@ -365,18 +365,6 @@ proc semObjConstr(c: PContext, n: PNode, flags: TExprFlags): PNode =
  # branches will be reported as an error):
  let initResult = semConstructType(c, result, t, flags)

-  # It's possible that the object was not fully initialized while


this definitely should show at least a summarizing comment; might as well keep this block IMO as it's still in TODO state

timotheecour · 2019-08-21T23:54:46Z

compiler/semstmts.nim

@@ -480,21 +480,12 @@ proc semVarOrLet(c: PContext, n: PNode, symkind: TSymKind): PNode =
                     ($typ.kind).substr(2).toLowerAscii)
        elif typ.kind == tyProc and tfUnresolved in typ.flags:
          localError(c.config, def.info, errProcHasNoConcreteType % def.renderTree)
-        when false:
-          # XXX This typing rule is neither documented nor complete enough to


at least some comment should stay

timotheecour · 2019-08-22T00:00:27Z

compiler/sigmatch.nim

@@ -133,14 +133,6 @@ proc initCandidate*(ctx: PContext, c: var TCandidate, callee: PType) =
  initIdTable(c.bindings)

 proc put(c: var TCandidate, key, val: PType) {.inline.} =
-  when false:


useful to keep for debugging (I've actually used it) ; maybe add a comment instead of deleting:

when false: # debugging ...

timotheecour · 2019-08-22T00:03:54Z

compiler/sigmatch.nim

@@ -2506,9 +2451,6 @@ proc matches*(c: PContext, n, nOrig: PNode, m: var TCandidate) =
    m.call = n
    # Note the following doesn't work as it would produce ambiguities.


this comment now makes no sense after the code is deleted; maybe enhance the comment instead

timotheecour · 2019-08-22T00:05:34Z

compiler/transf.nim

@@ -935,11 +911,6 @@ proc transform(c: PTransf, n: PNode): PTransNode =
      result = PTransNode(n)
  of nkMacroDef:
    # XXX no proper closure support yet:


this comment now makes no sense; maybe keep dead code (as a WIP)

timotheecour · 2019-08-22T00:14:23Z

koch.nim

@@ -477,10 +459,6 @@ proc runCI(cmd: string) =
  ## build nimble early on to enable remainder to depend on it if needed
  kochExecFold("Build Nimble", "nimble")

-  when false:


please keep this and maybe add a comment # consider re-enabling or similar; original PR #10150 showed when not defined(windows): and then was turned into when false in c5dbb03 ; as a result compile time FFI is not being tested in testament anymore and (unsurprisingly) it started to bitrot, cf my PR from yesterday #11991
CTFFI is a super powerful feature that I heavily rely on; it needs more eyes on it, not less

timotheecour · 2019-08-22T00:18:03Z

lib/core/refs.nim

@@ -87,11 +87,3 @@ proc newImpl(t: ptr TypeLayout): pointer =

 template new*[T](x: var ref T) =
  x = newImpl(getTypeLayout(x))
-
-
-when false:


I'd keep this block, this is a reference

timotheecour · 2019-08-22T00:24:25Z

lib/core/typeinfo.nim

@@ -294,8 +294,6 @@ iterator fields*(x: Any): tuple[name: string, any: Any] =
  var p = x.value
  var t = x.rawType
  # XXX BUG: does not work yet, however is questionable anyway


comment makes no sense after deletion; either keep that block (might as well) or enhance comment; likewise with other instances in this file

timotheecour · 2019-08-22T00:37:52Z

lib/pure/net.nim

@@ -812,58 +812,6 @@ proc acceptAddr*(server: Socket, client: var owned(Socket), address: var string,
        let ret = SSLAccept(client.sslHandle)
        socketError(client, ret, false)

-when false: #defineSsl:
-  proc acceptAddrSSL*(server: Socket, client: var Socket,


/cc @dom96 since you moved that code here in 2ce9f1c

timotheecour · 2019-08-22T00:44:24Z

lib/system/alloc.nim

@@ -410,15 +410,6 @@ proc pageAddr(p: pointer): PChunk {.inline.} =
  result = cast[PChunk](cast[ByteAddress](p) and not PageMask)
  #sysAssert(Contains(allocator.chunkStarts, pageIndex(result)))

-when false:


maybe keep with a comment when false: # debugging ?

timotheecour · 2019-08-22T00:46:51Z

lib/system/alloc.nim

@@ -1009,14 +901,6 @@ template instantiateForRegion(allocator: untyped) =
  proc realloc(p: pointer, newsize: Natural): pointer =
    result = realloc(allocator, p, newsize)

-  when false:


I'd keep this (used in commented out #sysAssert(result == countFreeMem()))
or if you do remove it, you'd need to also remove #sysAssert(result == countFreeMem())

timotheecour · 2019-08-22T00:51:38Z

lib/system/deepcopy.nim

@@ -142,11 +142,6 @@ proc genericDeepCopyAux(dest, src: pointer, mt: PNimType; tab: var PtrTable) =
          tab.put(s2, z)
          genericDeepCopyAux(z, s2, realType.base, tab)
        else:
-          when false:


I'd keep this; I think I've used this during a debugging session

timotheecour · 2019-08-22T00:55:56Z

lib/system/gc2.nim

@@ -451,13 +446,6 @@ proc GC_dumpHeap*(file: File) =
  ## can be translated into "dot" syntax via the "heapdump2dot" tool.
  gch.pDumpHeapFile = file
  var spaceIter: ObjectSpaceIter
-  when false:


seems useful (and non-trivial) for debugging

timotheecour · 2019-08-22T00:59:43Z

tests/metatype/ttypedesc3.nim

@@ -23,8 +23,6 @@ Base.pr
 Child.pr

 Base.me
-when false:
-  Child.me #<- bug #2710


disabled code for documentation purposes => I'd keep it

timotheecour · 2019-08-22T01:01:25Z

tests/stdlib/thttpclient.nim

@@ -63,17 +63,6 @@ proc asyncTest() {.async.} =
  except:
    doAssert(false, "HttpRequestError should have been raised")

-
-  when false:
-    # w3.org now blocks travis, so disabled:


seems useful code that can be run locally to test functionality

timotheecour · 2019-08-22T01:03:54Z

tests/vm/tvmmisc.nim

@@ -71,9 +71,6 @@ block:
 block:
  static:
    # for joint test, the project path is different, so I disabled it:


comment makes no sense after deletion

timotheecour

+1 for this; I went through everything; a small fraction should stay or require some minor changes, see comments

Araq · 2019-08-22T08:19:27Z

I won't merge this, in plenty of places the when false contains valuable source code, comments and information. And I don't want to review it again. I also don't want you to spend more time on this.

krux02 force-pushed the remove-dead-code branch from 16d6720 to b37e593 Compare August 20, 2019 18:23

delete a lot of dead code

9aef8a1

krux02 force-pushed the remove-dead-code branch from b37e593 to 9aef8a1 Compare August 20, 2019 19:11

narimiran reviewed Aug 21, 2019

View reviewed changes

timotheecour reviewed Aug 21, 2019

View reviewed changes

timotheecour reviewed Aug 22, 2019

View reviewed changes

Araq closed this Aug 22, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

delete a lot of dead code #11981

delete a lot of dead code #11981

krux02 commented Aug 19, 2019 •

edited

Loading

narimiran Aug 21, 2019

krux02 Aug 21, 2019 •

edited

Loading

juancarlospaco commented Aug 21, 2019

timotheecour Aug 21, 2019

timotheecour Aug 21, 2019

timotheecour Aug 21, 2019 •

edited

Loading

timotheecour Aug 21, 2019

timotheecour Aug 22, 2019

timotheecour Aug 22, 2019

timotheecour Aug 22, 2019

timotheecour Aug 22, 2019 •

edited

Loading

timotheecour Aug 22, 2019 •

edited

Loading

timotheecour Aug 22, 2019 •

edited

Loading

timotheecour Aug 22, 2019

timotheecour Aug 22, 2019

timotheecour Aug 22, 2019

timotheecour Aug 22, 2019

timotheecour Aug 22, 2019

timotheecour Aug 22, 2019

timotheecour Aug 22, 2019

timotheecour Aug 22, 2019

timotheecour left a comment •

edited

Loading

Araq commented Aug 22, 2019 •

edited

Loading

		@@ -2506,9 +2451,6 @@ proc matches*(c: PContext, n, nOrig: PNode, m: var TCandidate) =
		m.call = n
		# Note the following doesn't work as it would produce ambiguities.

delete a lot of dead code #11981

delete a lot of dead code #11981

Conversation

krux02 commented Aug 19, 2019 • edited Loading

Choose a reason for hiding this comment

krux02 Aug 21, 2019 • edited Loading

Choose a reason for hiding this comment

juancarlospaco commented Aug 21, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timotheecour Aug 21, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timotheecour Aug 22, 2019 • edited Loading

Choose a reason for hiding this comment

timotheecour Aug 22, 2019 • edited Loading

Choose a reason for hiding this comment

timotheecour Aug 22, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timotheecour left a comment • edited Loading

Choose a reason for hiding this comment

Araq commented Aug 22, 2019 • edited Loading

krux02 commented Aug 19, 2019 •

edited

Loading

krux02 Aug 21, 2019 •

edited

Loading

timotheecour Aug 21, 2019 •

edited

Loading

timotheecour Aug 22, 2019 •

edited

Loading

timotheecour Aug 22, 2019 •

edited

Loading

timotheecour Aug 22, 2019 •

edited

Loading

timotheecour left a comment •

edited

Loading

Araq commented Aug 22, 2019 •

edited

Loading