Extended side effect error messages

compiler/ast.nim

@@ -857,6 +857,7 @@ type
     of routineKinds:
       #procInstCache*: seq[PInstantiation]
       gcUnsafetyReason*: PSym  # for better error messages wrt gcsafe
+      sideEffectReasons*: seq[(TLineInfo, PSym)]  # for better side effect error messages
       transformedBody*: PNode  # cached body after transf pass
     of skLet, skVar, skField, skForVar:
       guard*: PSym

compiler/sempass2.nim

@@ -224,8 +224,25 @@ proc markGcUnsafe(a: PEffects; reason: PNode) =
                                           a.owner, reason.info, {})
 
 when true:
-  template markSideEffect(a: PEffects; reason: typed) =
-    if not a.inEnforcedNoSideEffects: a.hasSideEffect = true
+  template markSideEffect(a: PEffects; reason: typed; useLoc: TLineInfo) =
+    if not a.inEnforcedNoSideEffects:
+      a.hasSideEffect = true
+      if a.owner.kind in routineKinds:
+        var sym: PSym
+        when reason is PNode:
+          if reason.kind == nkSym:
+            sym = reason.sym
+          else:
+            var kind = skUnknown
+            case reason.kind
+            of nkHiddenDeref:
+              kind = skParam
+            else: discard
+            sym = newSym(kind, a.owner.name, nextSymId a.c.idgen,
+                                              a.owner, reason.info, {})
+        else:
+          sym = reason
+        a.owner.sideEffectReasons.add (useLoc, sym)
 else:
   template markSideEffect(a: PEffects; reason: typed) =
     if not a.inEnforcedNoSideEffects: a.hasSideEffect = true
@@ -259,6 +276,29 @@ proc listGcUnsafety(s: PSym; onlyWarning: bool; conf: ConfigRef) =
   var cycleCheck = initIntSet()
   listGcUnsafety(s, onlyWarning, cycleCheck, conf)
 
+proc listSideEffects(result: var string; s: PSym; cycleCheck: var IntSet; conf: ConfigRef; indentLevel: int) =
+  template add(msg; level = indentLevel) =
+    result.add repeat("  ", level) & msg
+  for (useLineInfo, u) in s.sideEffectReasons:
+    if u != nil and not cycleCheck.containsOrIncl(u.id):
+      case u.kind
+      of skLet, skVar:
+        add("$# Hint: '$#' accesses global state '$#'\n" % [conf $ useLineInfo, s.name.s, u.name.s])
+        add("$# Hint: '$#' accessed by a `noSideEffect` routine\n" % [conf $ u.info, u.name.s], indentLevel + 1)
+      of routineKinds:
+        add("$# Hint: '$#' calls '$#' which has side effects\n" % [conf $ useLineInfo, s.name.s, u.name.s])
+        add("$# Hint: '$#' called by a `noSideEffect` routine, but has side effects\n" % [conf $ u.info, u.name.s], indentLevel + 1)
+        listSideEffects(result, u, cycleCheck, conf, indentLevel + 2)
+      of skParam, skForVar:
+        add("$# Hint: '$#' calls a routine with side effects via hidden pointer indirection\n" % [conf $ useLineInfo, s.name.s])
+      else:
+        add("$# Hint: '$#' calls a routine with side effects via pointer indirection\n" % [conf $ useLineInfo, s.name.s])
+
+proc listSideEffects(result: var string; s: PSym; conf: ConfigRef) =
+  var cycleCheck = initIntSet()
+  result.add "'$#' can have side effects\n" % s.name.s
+  listSideEffects(result, s, cycleCheck, conf, 1)
+
 proc useVarNoInitCheck(a: PEffects; n: PNode; s: PSym) =
   if {sfGlobal, sfThread} * s.flags != {} and s.kind in {skVar, skLet} and
       s.magic != mNimvm:
@@ -267,9 +307,7 @@ proc useVarNoInitCheck(a: PEffects; n: PNode; s: PSym) =
         (tfHasGCedMem in s.typ.flags or s.typ.isGCedMem):
       #if a.config.hasWarn(warnGcUnsafe): warnAboutGcUnsafe(n)
       markGcUnsafe(a, s)
-      markSideEffect(a, s)
-    else:
-      markSideEffect(a, s)
+    markSideEffect(a, s, n.info)
   if s.owner != a.owner and s.kind in {skVar, skLet, skForVar, skResult, skParam} and
      {sfGlobal, sfThread} * s.flags == {}:
     a.isInnerProc = true
@@ -502,7 +540,7 @@ proc propagateEffects(tracked: PEffects, n: PNode, s: PSym) =
     if tracked.config.hasWarn(warnGcUnsafe): warnAboutGcUnsafe(n, tracked.config)
     markGcUnsafe(tracked, s)
   if tfNoSideEffect notin s.typ.flags:
-    markSideEffect(tracked, s)
+    markSideEffect(tracked, s, n.info)
   mergeLockLevels(tracked, n, s.getLockLevel)
 
 proc procVarCheck(n: PNode; conf: ConfigRef) =
@@ -584,15 +622,15 @@ proc trackOperandForIndirectCall(tracked: PEffects, n: PNode, paramType: PType;
         if tracked.config.hasWarn(warnGcUnsafe): warnAboutGcUnsafe(n, tracked.config)
         markGcUnsafe(tracked, a)
       elif tfNoSideEffect notin op.flags and not isOwnedProcVar(a, tracked.owner):
-        markSideEffect(tracked, a)
+        markSideEffect(tracked, a, n.info)
     else:
       mergeRaises(tracked, effectList[exceptionEffects], n)
       mergeTags(tracked, effectList[tagEffects], n)
       if notGcSafe(op):
         if tracked.config.hasWarn(warnGcUnsafe): warnAboutGcUnsafe(n, tracked.config)
         markGcUnsafe(tracked, a)
       elif tfNoSideEffect notin op.flags:
-        markSideEffect(tracked, a)
+        markSideEffect(tracked, a, n.info)
   if paramType != nil and paramType.kind in {tyVar}:
     invalidateFacts(tracked.guards, n)
     if n.kind == nkSym and isLocalVar(tracked, n.sym):
@@ -749,7 +787,7 @@ proc trackCall(tracked: PEffects; n: PNode) =
     if tfNoSideEffect notin op.flags and not importedFromC(a):
       # and it's not a recursive call:
       if not (a.kind == nkSym and a.sym == tracked.owner):
-        markSideEffect(tracked, a)
+        markSideEffect(tracked, a, n.info)
 
   # p's effects are ours too:
   var a = n[0]
@@ -771,7 +809,7 @@ proc trackCall(tracked: PEffects; n: PNode) =
         if a.sym == tracked.owner: tracked.isRecursive = true
         # even for recursive calls we need to check the lock levels (!):
         mergeLockLevels(tracked, n, a.sym.getLockLevel)
-        if sfSideEffect in a.sym.flags: markSideEffect(tracked, a)
+        if sfSideEffect in a.sym.flags: markSideEffect(tracked, a, n.info)
       else:
         mergeLockLevels(tracked, n, op.lockLevel)
       var effectList = op.n[0]
@@ -1336,13 +1374,15 @@ proc trackProc*(c: PContext; s: PSym, body: PNode) =
     effects[ensuresEffects] = ensuresSpec
 
   var mutationInfo = MutationInfo()
+  var hasMutationSideEffect = false
   if {strictFuncs, views} * c.features != {}:
     var goals: set[Goal] = {}
     if strictFuncs in c.features: goals.incl constParameters
     if views in c.features: goals.incl borrowChecking
     var partitions = computeGraphPartitions(s, body, g, goals)
     if not t.hasSideEffect and t.hasDangerousAssign:
       t.hasSideEffect = varpartitions.hasSideEffect(partitions, mutationInfo)
+      hasMutationSideEffect = t.hasSideEffect
     if views in c.features:
       checkBorrowedLocations(partitions, body, g.config)
 
@@ -1357,7 +1397,14 @@ proc trackProc*(c: PContext; s: PSym, body: PNode) =
     when false:
       listGcUnsafety(s, onlyWarning=false, g.config)
     else:
-      localError(g.config, s.info, ("'$1' can have side effects" % s.name.s) & (g.config $ mutationInfo))
+      if hasMutationSideEffect:
+        localError(g.config, s.info, "'$1' can have side effects$2" % [s.name.s, g.config $ mutationInfo])
+      elif c.compilesContextId == 0: # don't render extended diagnostic messages in `system.compiles` context
+        var msg = ""
+        listSideEffects(msg, s, g.config)
+        message(g.config, s.info, errGenerated, msg)
+      else:
+        localError(g.config, s.info, "") # simple error for `system.compiles` context
   if not t.gcUnsafe:
     s.typ.flags.incl tfGcSafe
   if not t.hasSideEffect and sfSideEffect notin s.flags:

tests/effects/tdiagnostic_messages.nim

@@ -0,0 +1,32 @@
+discard """
+  errormsg: "\'a\' can have side effects"
+  nimout: '''  tdiagnostic_messages.nim(32, 30) Hint: 'a' calls 'callWithSideEffects' which has side effects
+    tdiagnostic_messages.nim(24, 6) Hint: 'callWithSideEffects' called by a `noSideEffect` routine, but has side effects
+      tdiagnostic_messages.nim(26, 34) Hint: 'callWithSideEffects' calls 'indirectCallViaVarParam' which has side effects
+        tdiagnostic_messages.nim(20, 6) Hint: 'indirectCallViaVarParam' called by a `noSideEffect` routine, but has side effects
+          tdiagnostic_messages.nim(21, 7) Hint: 'indirectCallViaVarParam' calls a routine with side effects via hidden pointer indirection
+      tdiagnostic_messages.nim(27, 33) Hint: 'callWithSideEffects' calls 'indirectCallViaPointer' which has side effects
+        tdiagnostic_messages.nim(22, 6) Hint: 'indirectCallViaPointer' called by a `noSideEffect` routine, but has side effects
+          tdiagnostic_messages.nim(23, 32) Hint: 'indirectCallViaPointer' calls a routine with side effects via pointer indirection
+      tdiagnostic_messages.nim(28, 8) Hint: 'callWithSideEffects' calls 'echo' which has side effects
+        ../../lib/system.nim(2004, 6) Hint: 'echo' called by a `noSideEffect` routine, but has side effects
+      tdiagnostic_messages.nim(29, 3) Hint: 'callWithSideEffects' accesses global state 'globalVar'
+        tdiagnostic_messages.nim(19, 5) Hint: 'globalVar' accessed by a `noSideEffect` routine'''
+  file: "tdiagnostic_messages.nim"
+  line: 31
+"""
+
+var globalVar = 0
+proc indirectCallViaVarParam(call: var proc(): int {.nimcall.}): int =
+  call()
+proc indirectCallViaPointer(call: pointer): int =
+  cast[ptr proc(): int](call)[]()
+proc callWithSideEffects(): int =
+  var p = proc (): int {.nimcall.} = 0
+  discard indirectCallViaVarParam(p)
+  discard indirectCallViaPointer(addr p)
+  echo ""
+  globalVar
+
+func a: int =
+  discard callWithSideEffects()