From 48b2e10fb1588b3139270bbc77889f0309df3218 Mon Sep 17 00:00:00 2001 From: Zahary Karadjov Date: Fri, 27 Mar 2020 15:47:49 +0200 Subject: [PATCH 01/20] First steps, the compiler can boot with enforced requiresInit --- compiler/ast.nim | 12 +++++++----- compiler/pragmas.nim | 2 +- compiler/semobjconstr.nim | 13 ++++--------- compiler/sempass2.nim | 8 ++++---- compiler/semstmts.nim | 2 +- compiler/semtypes.nim | 10 +++++----- tests/types/temptyseqs.nim | 8 ++++---- 7 files changed, 26 insertions(+), 29 deletions(-) diff --git a/compiler/ast.nim b/compiler/ast.nim index 4d45f0e5581ed..a4630732b3960 100644 --- a/compiler/ast.nim +++ b/compiler/ast.nim @@ -517,8 +517,10 @@ type tfPartial, # type is declared as 'partial' tfNotNil, # type cannot be 'nil' - tfNeedsInit, # type constains a "not nil" constraint somewhere or some - # other type so that it requires initialization + tfHasRequiresInit,# type constains a "not nil" constraint somewhere or + # a `requiresInit` field, so the default zero init + # is not appropriate + tfRequiresInit, # all fields of the type must be initialized tfVarIsPtr, # 'var' type is translated like 'ptr' even in C++ mode tfHasMeta, # type contains "wildcard" sub-types such as generic params # or other type classes @@ -1484,11 +1486,11 @@ proc propagateToOwner*(owner, elem: PType; propagateHasAsgn = true) = if owner.kind in {tyGenericInst, tyGenericBody, tyGenericInvocation}: owner.flags.incl tfNotNil elif owner.kind notin HaveTheirOwnEmpty: - owner.flags.incl tfNeedsInit + owner.flags.incl tfHasRequiresInit - if tfNeedsInit in elem.flags: + if tfRequiresInit in elem.flags: if owner.kind in HaveTheirOwnEmpty: discard - else: owner.flags.incl tfNeedsInit + else: owner.flags.incl tfHasRequiresInit if elem.isMetaType: owner.flags.incl tfHasMeta diff --git a/compiler/pragmas.nim b/compiler/pragmas.nim index 6056afb6c348d..e4b72bed7a9ad 100644 --- a/compiler/pragmas.nim +++ b/compiler/pragmas.nim @@ -1088,7 +1088,7 @@ proc singlePragma(c: PContext, sym: PSym, n: PNode, i: var int, of wRequiresInit: noVal(c, it) if sym.typ == nil: invalidPragma(c, it) - else: incl(sym.typ.flags, tfNeedsInit) + else: incl(sym.typ.flags, tfRequiresInit) of wByRef: noVal(c, it) if sym == nil or sym.typ == nil: diff --git a/compiler/semobjconstr.nim b/compiler/semobjconstr.nim index 489bc66840c20..bc1e85425b624 100644 --- a/compiler/semobjconstr.nim +++ b/compiler/semobjconstr.nim @@ -138,7 +138,7 @@ proc fieldsPresentInInitExpr(c: PContext, fieldsRecList, initExpr: PNode): strin proc missingMandatoryFields(c: PContext, fieldsRecList, initExpr: PNode): string = for r in directFieldsInRecList(fieldsRecList): - if {tfNotNil, tfNeedsInit} * r.sym.typ.flags != {}: + if {tfNotNil, tfRequiresInit} * r.sym.typ.flags != {}: let assignment = locateFieldInInitExpr(c, r.sym, initExpr) if assignment == nil: if result.len == 0: @@ -358,15 +358,10 @@ proc semObjConstr(c: PContext, n: PNode, flags: TExprFlags): PNode = # It's possible that the object was not fully initialized while # specifying a .requiresInit. pragma. - # XXX: Turn this into an error in the next release - if tfNeedsInit in t.flags and initResult != initFull: - # XXX: Disable this warning for now, because tfNeedsInit is propagated - # too aggressively from fields to object types (and this is not correct - # in case objects) - when false: message(n.info, warnUser, + if tfRequiresInit in t.flags and initResult != initFull: + localError(c.config, n.info, "object type uses the 'requiresInit' pragma, but not all fields " & - "have been initialized. future versions of Nim will treat this as " & - "an error") + "have been initialized.") # Since we were traversing the object fields, it's possible that # not all of the fields specified in the constructor was visited. diff --git a/compiler/sempass2.nim b/compiler/sempass2.nim index 4658f5454f6a1..65dce3ed8f75c 100644 --- a/compiler/sempass2.nim +++ b/compiler/sempass2.nim @@ -184,7 +184,7 @@ proc initVar(a: PEffects, n: PNode; volatileCheck: bool) = proc initVarViaNew(a: PEffects, n: PNode) = if n.kind != nkSym: return let s = n.sym - if {tfNeedsInit, tfNotNil} * s.typ.flags <= {tfNotNil}: + if {tfRequiresInit, tfNotNil} * s.typ.flags <= {tfNotNil}: # 'x' is not nil, but that doesn't mean its "not nil" children # are initialized: initVar(a, n, volatileCheck=true) @@ -253,7 +253,7 @@ proc useVar(a: PEffects, n: PNode) = # If the variable is explicitly marked as .noinit. do not emit any error a.init.add s.id elif s.id notin a.init: - if {tfNeedsInit, tfNotNil} * s.typ.flags != {}: + if {tfRequiresInit, tfNotNil} * s.typ.flags != {}: message(a.config, n.info, warnProveInit, s.name.s) else: message(a.config, n.info, warnUninit, s.name.s) @@ -838,7 +838,7 @@ proc track(tracked: PEffects, n: PNode) = # may not look like an assignment, but it is: let arg = n[1] initVarViaNew(tracked, arg) - if arg.typ.len != 0 and {tfNeedsInit} * arg.typ.lastSon.flags != {}: + if arg.typ.len != 0 and {tfRequiresInit} * arg.typ.lastSon.flags != {}: if a.sym.magic == mNewSeq and n[2].kind in {nkCharLit..nkUInt64Lit} and n[2].intVal == 0: # var s: seq[notnil]; newSeq(s, 0) is a special case! @@ -1203,7 +1203,7 @@ proc trackProc*(c: PContext; s: PSym, body: PNode) = createTypeBoundOps(t, typ, param.info) if not isEmptyType(s.typ[0]) and - ({tfNeedsInit, tfNotNil} * s.typ[0].flags != {} or + ({tfRequiresInit, tfNotNil} * s.typ[0].flags != {} or s.typ[0].skipTypes(abstractInst).kind == tyVar) and s.kind in {skProc, skFunc, skConverter, skMethod}: var res = s.ast[resultPos].sym # get result symbol diff --git a/compiler/semstmts.nim b/compiler/semstmts.nim index b45c42fe15c9f..057972f0ab965 100644 --- a/compiler/semstmts.nim +++ b/compiler/semstmts.nim @@ -336,7 +336,7 @@ proc semIdentDef(c: PContext, n: PNode, kind: TSymKind): PSym = proc checkNilable(c: PContext; v: PSym) = if {sfGlobal, sfImportc} * v.flags == {sfGlobal} and - {tfNotNil, tfNeedsInit} * v.typ.flags != {}: + {tfNotNil, tfRequiresInit} * v.typ.flags != {}: if v.astdef.isNil: message(c.config, v.info, warnProveInit, v.name.s) elif tfNotNil in v.typ.flags and not v.astdef.typ.isNil and tfNotNil notin v.astdef.typ.flags: diff --git a/compiler/semtypes.nim b/compiler/semtypes.nim index 7cddc7520c5e8..92e7b8bd2b27e 100644 --- a/compiler/semtypes.nim +++ b/compiler/semtypes.nim @@ -139,7 +139,7 @@ proc semEnum(c: PContext, n: PNode, prev: PType): PType = if isPure and (let conflict = strTableInclReportConflict(symbols, e); conflict != nil): wrongRedefinition(c, e.info, e.name.s, conflict.info) inc(counter) - if tfNotNil in e.typ.flags and not hasNull: incl(result.flags, tfNeedsInit) + if tfNotNil in e.typ.flags and not hasNull: incl(result.flags, tfRequiresInit) proc semSet(c: PContext, n: PNode, prev: PType): PType = result = newOrPrevType(tySet, prev, c) @@ -254,15 +254,15 @@ proc semRange(c: PContext, n: PNode, prev: PType): PType = result = semRangeAux(c, n[1], prev) let n = result.n if n[0].kind in {nkCharLit..nkUInt64Lit} and n[0].intVal > 0: - incl(result.flags, tfNeedsInit) + incl(result.flags, tfRequiresInit) elif n[1].kind in {nkCharLit..nkUInt64Lit} and n[1].intVal < 0: - incl(result.flags, tfNeedsInit) + incl(result.flags, tfRequiresInit) elif n[0].kind in {nkFloatLit..nkFloat64Lit} and n[0].floatVal > 0.0: - incl(result.flags, tfNeedsInit) + incl(result.flags, tfRequiresInit) elif n[1].kind in {nkFloatLit..nkFloat64Lit} and n[1].floatVal < 0.0: - incl(result.flags, tfNeedsInit) + incl(result.flags, tfRequiresInit) else: if n[1].kind == nkInfix and considerQuotedIdent(c, n[1][0]).s == "..<": localError(c.config, n[0].info, "range types need to be constructed with '..', '..<' is not supported") diff --git a/tests/types/temptyseqs.nim b/tests/types/temptyseqs.nim index 28c3b87b837c3..6e7b58cedaa0b 100644 --- a/tests/types/temptyseqs.nim +++ b/tests/types/temptyseqs.nim @@ -75,16 +75,16 @@ varargso("foo", "bar", "baz") type Flago = enum - tfNeedsInit, tfNotNil + tfRequiresInit, tfNotNil -var s: set[Flago] = {tfNeedsInit} +var s: set[Flago] = {tfRequiresInit} -if {tfNeedsInit, tfNotNil} * s != {}: +if {tfRequiresInit, tfNotNil} * s != {}: echo "yes" else: echo "no" -if {tfNeedsInit, tfNotNil} * s <= {tfNotNil}: +if {tfRequiresInit, tfNotNil} * s <= {tfNotNil}: echo "yes" else: echo "no" From d8ab0e1ac1ac077e66b6066c6a9a9b5f9653d8c4 Mon Sep 17 00:00:00 2001 From: Zahary Karadjov Date: Fri, 27 Mar 2020 18:43:23 +0200 Subject: [PATCH 02/20] More sophistication; Allow requiresInit to be specified per-field --- compiler/ast.nim | 3 +- compiler/pragmas.nim | 10 ++++-- compiler/semobjconstr.nim | 32 +++++++++++++------- tests/constructors/tinvalid_construction.nim | 31 +++++++++++++++++++ 4 files changed, 61 insertions(+), 15 deletions(-) diff --git a/compiler/ast.nim b/compiler/ast.nim index a4630732b3960..a7eb2cac5ebcf 100644 --- a/compiler/ast.nim +++ b/compiler/ast.nim @@ -259,6 +259,7 @@ type # needed for the code generator sfProcvar, # proc can be passed to a proc var sfDiscriminant, # field is a discriminant in a record/object + sfRequiresInit, # field must be initialized during construction sfDeprecated, # symbol is deprecated sfExplain, # provide more diagnostics when this symbol is used sfError, # usage of symbol should trigger a compile-time error @@ -1488,7 +1489,7 @@ proc propagateToOwner*(owner, elem: PType; propagateHasAsgn = true) = elif owner.kind notin HaveTheirOwnEmpty: owner.flags.incl tfHasRequiresInit - if tfRequiresInit in elem.flags: + if {tfRequiresInit, tfHasRequiresInit} * elem.flags != {}: if owner.kind in HaveTheirOwnEmpty: discard else: owner.flags.incl tfHasRequiresInit diff --git a/compiler/pragmas.nim b/compiler/pragmas.nim index e4b72bed7a9ad..604ba063fcba6 100644 --- a/compiler/pragmas.nim +++ b/compiler/pragmas.nim @@ -65,7 +65,7 @@ const wInheritable, wGensym, wInject, wRequiresInit, wUnchecked, wUnion, wPacked, wBorrow, wGcSafe, wPartial, wExplain, wPackage} fieldPragmas* = declPragmas + { - wGuard, wBitsize, wCursor} - {wExportNims, wNodecl} # why exclude these? + wGuard, wBitsize, wCursor, wRequiresInit} - {wExportNims, wNodecl} # why exclude these? varPragmas* = declPragmas + {wVolatile, wRegister, wThreadVar, wMagic, wHeader, wCompilerProc, wCore, wDynlib, wNoInit, wCompileTime, wGlobal, @@ -1087,8 +1087,12 @@ proc singlePragma(c: PContext, sym: PSym, n: PNode, i: var int, else: incl(sym.typ.flags, tfUnion) of wRequiresInit: noVal(c, it) - if sym.typ == nil: invalidPragma(c, it) - else: incl(sym.typ.flags, tfRequiresInit) + if sym.kind == skField: + sym.flags.incl sfRequiresInit + elif sym.typ != nil: + incl(sym.typ.flags, tfRequiresInit) + else: + invalidPragma(c, it) of wByRef: noVal(c, it) if sym == nil or sym.typ == nil: diff --git a/compiler/semobjconstr.nim b/compiler/semobjconstr.nim index bc1e85425b624..ee79f1a677316 100644 --- a/compiler/semobjconstr.nim +++ b/compiler/semobjconstr.nim @@ -136,9 +136,11 @@ proc fieldsPresentInInitExpr(c: PContext, fieldsRecList, initExpr: PNode): strin if result.len != 0: result.add ", " result.add field.sym.name.s.quoteStr -proc missingMandatoryFields(c: PContext, fieldsRecList, initExpr: PNode): string = +proc missingMandatoryFields(c: PContext, fieldsRecList, initExpr: PNode, + requiresFullInit = false): string = for r in directFieldsInRecList(fieldsRecList): - if {tfNotNil, tfRequiresInit} * r.sym.typ.flags != {}: + if requiresFullInit or sfRequiresInit in r.sym.flags or + {tfNotNil, tfRequiresInit, tfHasRequiresInit} * r.sym.typ.flags != {}: let assignment = locateFieldInInitExpr(c, r.sym, initExpr) if assignment == nil: if result.len == 0: @@ -147,19 +149,22 @@ proc missingMandatoryFields(c: PContext, fieldsRecList, initExpr: PNode): string result.add ", " result.add r.sym.name.s -proc checkForMissingFields(c: PContext, recList, initExpr: PNode) = - let missing = missingMandatoryFields(c, recList, initExpr) +proc checkForMissingFields(c: PContext, recList, initExpr: PNode, + requiresFullInit = false) = + let missing = missingMandatoryFields(c, recList, initExpr, requiresFullInit) if missing.len > 0: localError(c.config, initExpr.info, "fields not initialized: $1.", [missing]) proc semConstructFields(c: PContext, recNode: PNode, - initExpr: PNode, flags: TExprFlags): InitStatus = + initExpr: PNode, flags: TExprFlags, + requiresFullInit = false): InitStatus = result = initUnknown case recNode.kind of nkRecList: for field in recNode: - let status = semConstructFields(c, field, initExpr, flags) + let status = semConstructFields(c, field, initExpr, + flags, requiresFullInit) mergeInitStatus(result, status) of nkRecCase: @@ -171,7 +176,7 @@ proc semConstructFields(c: PContext, recNode: PNode, template checkMissingFields(branchNode: PNode) = if branchNode != nil: let fields = branchNode[^1] - checkForMissingFields(c, fields, initExpr) + checkForMissingFields(c, fields, initExpr, requiresFullInit) let discriminator = recNode[0] internalAssert c.config, discriminator.kind == nkSym @@ -179,7 +184,8 @@ proc semConstructFields(c: PContext, recNode: PNode, for i in 1.. Date: Fri, 27 Mar 2020 20:22:37 +0200 Subject: [PATCH 03/20] Don't allow 'var x: T' for objects that require initialization --- compiler/sem.nim | 18 ++++++ compiler/semobjconstr.nim | 68 +++++++++----------- compiler/semstmts.nim | 6 +- tests/constructors/tinvalid_construction.nim | 8 +++ 4 files changed, 63 insertions(+), 37 deletions(-) diff --git a/compiler/sem.nim b/compiler/sem.nim index 48f767af7c569..f4d992e25c46a 100644 --- a/compiler/sem.nim +++ b/compiler/sem.nim @@ -58,6 +58,24 @@ proc isArrayConstr(n: PNode): bool {.inline.} = result = n.kind == nkBracket and n.typ.skipTypes(abstractInst).kind == tyArray +type + ObjConstrContext = object + typ: PType # The constructed type + initExpr: PNode # The init expression (nkObjConstr) + requiresFullInit: bool # A `requiresInit` derived type will + # set this to true while visiting + # parent types. + + InitStatus = enum # This indicates the result of object construction + initUnknown + initFull # All of the fields have been initialized + initPartial # Some of the fields have been initialized + initNone # None of the fields have been initialized + initConflict # Fields from different branches have been initialized + +proc semConstructType(c: PContext, initExpr: PNode, + t: PType, flags: TExprFlags): InitStatus + template semIdeForTemplateOrGenericCheck(conf, n, requiresCheck) = # we check quickly if the node is where the cursor is when defined(nimsuggest): diff --git a/compiler/semobjconstr.nim b/compiler/semobjconstr.nim index ee79f1a677316..2d226aa3394fa 100644 --- a/compiler/semobjconstr.nim +++ b/compiler/semobjconstr.nim @@ -11,14 +11,6 @@ # included from sem.nim -type - InitStatus = enum - initUnknown - initFull # All of the fields have been initialized - initPartial # Some of the fields have been initialized - initNone # None of the fields have been initialized - initConflict # Fields from different branches have been initialized - proc mergeInitStatus(existing: var InitStatus, newStatus: InitStatus) = case newStatus of initConflict: @@ -136,12 +128,12 @@ proc fieldsPresentInInitExpr(c: PContext, fieldsRecList, initExpr: PNode): strin if result.len != 0: result.add ", " result.add field.sym.name.s.quoteStr -proc missingMandatoryFields(c: PContext, fieldsRecList, initExpr: PNode, - requiresFullInit = false): string = +proc missingMandatoryFields(c: PContext, fieldsRecList: PNode, + constrCtx: ObjConstrContext): string = for r in directFieldsInRecList(fieldsRecList): - if requiresFullInit or sfRequiresInit in r.sym.flags or + if constrCtx.requiresFullInit or sfRequiresInit in r.sym.flags or {tfNotNil, tfRequiresInit, tfHasRequiresInit} * r.sym.typ.flags != {}: - let assignment = locateFieldInInitExpr(c, r.sym, initExpr) + let assignment = locateFieldInInitExpr(c, r.sym, constrCtx.initExpr) if assignment == nil: if result.len == 0: result = r.sym.name.s @@ -149,34 +141,35 @@ proc missingMandatoryFields(c: PContext, fieldsRecList, initExpr: PNode, result.add ", " result.add r.sym.name.s -proc checkForMissingFields(c: PContext, recList, initExpr: PNode, - requiresFullInit = false) = - let missing = missingMandatoryFields(c, recList, initExpr, requiresFullInit) +proc checkForMissingFields(c: PContext, recList: PNode, + constrCtx: ObjConstrContext) = + let missing = missingMandatoryFields(c, recList, constrCtx) if missing.len > 0: - localError(c.config, initExpr.info, "fields not initialized: $1.", [missing]) + localError(c.config, constrCtx.initExpr.info, + "The $1 type requires the following fields to be initialized: $2.", + [constrCtx.typ.sym.name.s, missing]) proc semConstructFields(c: PContext, recNode: PNode, - initExpr: PNode, flags: TExprFlags, - requiresFullInit = false): InitStatus = + constrCtx: ObjConstrContext, + flags: TExprFlags): InitStatus = result = initUnknown case recNode.kind of nkRecList: for field in recNode: - let status = semConstructFields(c, field, initExpr, - flags, requiresFullInit) + let status = semConstructFields(c, field, constrCtx, flags) mergeInitStatus(result, status) of nkRecCase: template fieldsPresentInBranch(branchIdx: int): string = let branch = recNode[branchIdx] let fields = branch[^1] - fieldsPresentInInitExpr(c, fields, initExpr) + fieldsPresentInInitExpr(c, fields, constrCtx.initExpr) template checkMissingFields(branchNode: PNode) = if branchNode != nil: let fields = branchNode[^1] - checkForMissingFields(c, fields, initExpr, requiresFullInit) + checkForMissingFields(c, fields, constrCtx) let discriminator = recNode[0] internalAssert c.config, discriminator.kind == nkSym @@ -184,14 +177,13 @@ proc semConstructFields(c: PContext, recNode: PNode, for i in 1.. Date: Fri, 27 Mar 2020 20:53:09 +0200 Subject: [PATCH 04/20] Plug another hole: default(T) forbidden for objects requiring initialization --- compiler/sem.nim | 19 +------------------ compiler/semexprs.nim | 6 ++++++ compiler/semobjconstr.nim | 18 ++++++++++++++++++ compiler/semstmts.nim | 2 +- tests/constructors/tinvalid_construction.nim | 5 +++++ 5 files changed, 31 insertions(+), 19 deletions(-) diff --git a/compiler/sem.nim b/compiler/sem.nim index f4d992e25c46a..3a4ff2cd9ba9d 100644 --- a/compiler/sem.nim +++ b/compiler/sem.nim @@ -46,6 +46,7 @@ proc addParams(c: PContext, n: PNode, kind: TSymKind) proc maybeAddResult(c: PContext, s: PSym, n: PNode) proc tryExpr(c: PContext, n: PNode, flags: TExprFlags = {}): PNode proc activate(c: PContext, n: PNode) +proc checkDefaultConstruction(c: PContext, typ: PType, info: TLineInfo) proc semQuoteAst(c: PContext, n: PNode): PNode proc finishMethod(c: PContext, s: PSym) proc evalAtCompileTime(c: PContext, n: PNode): PNode @@ -58,24 +59,6 @@ proc isArrayConstr(n: PNode): bool {.inline.} = result = n.kind == nkBracket and n.typ.skipTypes(abstractInst).kind == tyArray -type - ObjConstrContext = object - typ: PType # The constructed type - initExpr: PNode # The init expression (nkObjConstr) - requiresFullInit: bool # A `requiresInit` derived type will - # set this to true while visiting - # parent types. - - InitStatus = enum # This indicates the result of object construction - initUnknown - initFull # All of the fields have been initialized - initPartial # Some of the fields have been initialized - initNone # None of the fields have been initialized - initConflict # Fields from different branches have been initialized - -proc semConstructType(c: PContext, initExpr: PNode, - t: PType, flags: TExprFlags): InitStatus - template semIdeForTemplateOrGenericCheck(conf, n, requiresCheck) = # we check quickly if the node is where the cursor is when defined(nimsuggest): diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index d6659f76b5127..58f3f93162680 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -2264,6 +2264,12 @@ proc semMagic(c: PContext, n: PNode, s: PSym, flags: TExprFlags): PNode = of mSizeOf: markUsed(c, n.info, s) result = semSizeof(c, setMs(n, s)) + of mDefault: + result = semDirectOp(c, n, flags) + c.config.internalAssert result[1].typ.kind == tyTypeDesc + let typ = result[1].typ.base + if typ.kind in {tyObject, tyTuple}: + checkDefaultConstruction(c, typ, n.info) else: result = semDirectOp(c, n, flags) diff --git a/compiler/semobjconstr.nim b/compiler/semobjconstr.nim index 2d226aa3394fa..b9931f5270840 100644 --- a/compiler/semobjconstr.nim +++ b/compiler/semobjconstr.nim @@ -11,6 +11,21 @@ # included from sem.nim +type + ObjConstrContext = object + typ: PType # The constructed type + initExpr: PNode # The init expression (nkObjConstr) + requiresFullInit: bool # A `requiresInit` derived type will + # set this to true while visiting + # parent types. + + InitStatus = enum # This indicates the result of object construction + initUnknown + initFull # All of the fields have been initialized + initPartial # Some of the fields have been initialized + initNone # None of the fields have been initialized + initConflict # Fields from different branches have been initialized + proc mergeInitStatus(existing: var InitStatus, newStatus: InitStatus) = case newStatus of initConflict: @@ -336,6 +351,9 @@ proc semConstructType(c: PContext, initExpr: PNode, constrCtx.requiresFullInit = constrCtx.requiresFullInit or tfRequiresInit in t.flags +proc checkDefaultConstruction(c: PContext, typ: PType, info: TLineInfo) = + discard semConstructType(c, newNodeI(nkObjConstr, info), typ, {}) + proc semObjConstr(c: PContext, n: PNode, flags: TExprFlags): PNode = var t = semTypeNode(c, n[0], nil) result = newNodeIT(nkObjConstr, n.info, t) diff --git a/compiler/semstmts.nim b/compiler/semstmts.nim index 812d1ff71beb7..373e5fcdbe3d8 100644 --- a/compiler/semstmts.nim +++ b/compiler/semstmts.nim @@ -616,7 +616,7 @@ proc semVarOrLet(c: PContext, n: PNode, symkind: TSymKind): PNode = b[j] = newSymNode(v) if def.kind == nkEmpty: if v.typ.kind in {tyObject, tyTuple}: - discard semConstructType(c, newNodeI(nkObjConstr, v.info), v.typ, {}) + checkDefaultConstruction(c, v.typ, v.info) else: checkNilable(c, v) if sfCompileTime in v.flags: diff --git a/tests/constructors/tinvalid_construction.nim b/tests/constructors/tinvalid_construction.nim index 9afee3cd4efc9..f84a18e68aba6 100644 --- a/tests/constructors/tinvalid_construction.nim +++ b/tests/constructors/tinvalid_construction.nim @@ -125,12 +125,15 @@ accept PartialRequiresInit(a: 10, b: "x") accept PartialRequiresInit(a: 20) reject PartialRequiresInit(b: "x") reject PartialRequiresInit() +reject default(PartialRequiresInit) reject: var obj: PartialRequiresInit accept FullRequiresInit(a: 10, b: 20) reject FullRequiresInit(a: 10) reject FullRequiresInit(b: 20) +reject FullRequiresInit() +reject default(FullRequiresInit) reject: var obj: FullRequiresInit @@ -140,11 +143,13 @@ reject FullRequiresInitWithParent(a: notNilRef, b: nil, c: nil, e: 10, d: 20) # reject FullRequiresInitWithParent(a: notNilRef, b: notNilRef, e: 10, d: 20) # c should not be missing reject FullRequiresInitWithParent(a: notNilRef, b: notNilRef, c: nil, e: 10) # d should not be missing reject FullRequiresInitWithParent() +reject default(FullRequiresInitWithParent) reject: var obj: FullRequiresInitWithParent # this will be accepted, because the false outer branch will be taken and the inner A branch accept TNestedChoices() +accept default(TNestedChoices) accept: var obj: TNestedChoices From 5b3fe7b53b03790ae44aaec5c090a4f25bf66e1b Mon Sep 17 00:00:00 2001 From: Zahary Karadjov Date: Fri, 27 Mar 2020 21:58:32 +0200 Subject: [PATCH 05/20] Enable the requiresInit checks only for objects --- compiler/semexprs.nim | 2 +- compiler/semstmts.nim | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index 58f3f93162680..d39fe146e7f93 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -2268,7 +2268,7 @@ proc semMagic(c: PContext, n: PNode, s: PSym, flags: TExprFlags): PNode = result = semDirectOp(c, n, flags) c.config.internalAssert result[1].typ.kind == tyTypeDesc let typ = result[1].typ.base - if typ.kind in {tyObject, tyTuple}: + if typ.kind == tyObject: checkDefaultConstruction(c, typ, n.info) else: result = semDirectOp(c, n, flags) diff --git a/compiler/semstmts.nim b/compiler/semstmts.nim index 373e5fcdbe3d8..fc624c7101cd5 100644 --- a/compiler/semstmts.nim +++ b/compiler/semstmts.nim @@ -615,7 +615,7 @@ proc semVarOrLet(c: PContext, n: PNode, symkind: TSymKind): PNode = else: v.typ = tup b[j] = newSymNode(v) if def.kind == nkEmpty: - if v.typ.kind in {tyObject, tyTuple}: + if v.typ.kind == tyObject: checkDefaultConstruction(c, v.typ, v.info) else: checkNilable(c, v) From 7beaf46f883de52be55310a9d5f0fdaee47e77c6 Mon Sep 17 00:00:00 2001 From: Zahary Karadjov Date: Fri, 27 Mar 2020 22:48:40 +0200 Subject: [PATCH 06/20] Fix tests/notnil/tnotnil_in_objconstr.nim --- tests/notnil/tnotnil_in_objconstr.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/notnil/tnotnil_in_objconstr.nim b/tests/notnil/tnotnil_in_objconstr.nim index fb8ac8d6f350b..030c40d32144f 100644 --- a/tests/notnil/tnotnil_in_objconstr.nim +++ b/tests/notnil/tnotnil_in_objconstr.nim @@ -1,5 +1,5 @@ discard """ - errormsg: "fields not initialized: bar" + errormsg: "The Foo type requires the following fields to be initialized: bar" line: "13" """ {.experimental: "notnil".} From c0b97437f317321795dc2a0a5b2f34c4935d4d29 Mon Sep 17 00:00:00 2001 From: Zahary Karadjov Date: Sun, 29 Mar 2020 02:06:39 +0200 Subject: [PATCH 07/20] Turn the warning for uninitialized (result) variables into errors --- compiler/ast.nim | 3 + compiler/lineinfos.nim | 2 + compiler/semobjconstr.nim | 5 +- compiler/sempass2.nim | 17 ++-- compiler/semstmts.nim | 5 +- compiler/semtypes.nim | 5 +- tests/constructors/tinvalid_construction.nim | 83 +++++++++++++++++++- 7 files changed, 103 insertions(+), 17 deletions(-) diff --git a/compiler/ast.nim b/compiler/ast.nim index a7eb2cac5ebcf..01f9ed29bd4a0 100644 --- a/compiler/ast.nim +++ b/compiler/ast.nim @@ -1396,6 +1396,9 @@ proc copyType*(t: PType, owner: PSym, keepId: bool): PType = proc exactReplica*(t: PType): PType = copyType(t, t.owner, true) +template requiresInit*(t: PType): bool = + t.flags * {tfRequiresInit, tfHasRequiresInit, tfNotNil} != {} + proc copySym*(s: PSym): PSym = result = newSym(s.kind, s.name, s.owner, s.info, s.options) #result.ast = nil # BUGFIX; was: s.ast which made problems diff --git a/compiler/lineinfos.nim b/compiler/lineinfos.nim index f7f3b7706aa9b..585a95c806eee 100644 --- a/compiler/lineinfos.nim +++ b/compiler/lineinfos.nim @@ -23,6 +23,7 @@ type errGeneralParseError, errNewSectionExpected, errInvalidDirectiveX, + errProveInit, errGenerated, errUser, warnCannotOpenFile, @@ -63,6 +64,7 @@ const errGeneralParseError: "general parse error", errNewSectionExpected: "new section expected", errInvalidDirectiveX: "invalid directive: '$1'", + errProveInit: "Cannot prove that '$1' is initialized.", errGenerated: "$1", errUser: "$1", warnCannotOpenFile: "cannot open '$1'", diff --git a/compiler/semobjconstr.nim b/compiler/semobjconstr.nim index b9931f5270840..7222433b31d04 100644 --- a/compiler/semobjconstr.nim +++ b/compiler/semobjconstr.nim @@ -146,8 +146,9 @@ proc fieldsPresentInInitExpr(c: PContext, fieldsRecList, initExpr: PNode): strin proc missingMandatoryFields(c: PContext, fieldsRecList: PNode, constrCtx: ObjConstrContext): string = for r in directFieldsInRecList(fieldsRecList): - if constrCtx.requiresFullInit or sfRequiresInit in r.sym.flags or - {tfNotNil, tfRequiresInit, tfHasRequiresInit} * r.sym.typ.flags != {}: + if constrCtx.requiresFullInit or + sfRequiresInit in r.sym.flags or + r.sym.typ.requiresInit: let assignment = locateFieldInInitExpr(c, r.sym, constrCtx.initExpr) if assignment == nil: if result.len == 0: diff --git a/compiler/sempass2.nim b/compiler/sempass2.nim index 65dce3ed8f75c..2d5e3e7212c7d 100644 --- a/compiler/sempass2.nim +++ b/compiler/sempass2.nim @@ -184,7 +184,7 @@ proc initVar(a: PEffects, n: PNode; volatileCheck: bool) = proc initVarViaNew(a: PEffects, n: PNode) = if n.kind != nkSym: return let s = n.sym - if {tfRequiresInit, tfNotNil} * s.typ.flags <= {tfNotNil}: + if {tfRequiresInit, tfHasRequiresInit, tfNotNil} * s.typ.flags <= {tfNotNil}: # 'x' is not nil, but that doesn't mean its "not nil" children # are initialized: initVar(a, n, volatileCheck=true) @@ -253,8 +253,8 @@ proc useVar(a: PEffects, n: PNode) = # If the variable is explicitly marked as .noinit. do not emit any error a.init.add s.id elif s.id notin a.init: - if {tfRequiresInit, tfNotNil} * s.typ.flags != {}: - message(a.config, n.info, warnProveInit, s.name.s) + if s.typ.requiresInit: + localError(a.config, n.info, errProveInit, s.name.s) else: message(a.config, n.info, warnUninit, s.name.s) # prevent superfluous warnings about the same variable: @@ -838,13 +838,13 @@ proc track(tracked: PEffects, n: PNode) = # may not look like an assignment, but it is: let arg = n[1] initVarViaNew(tracked, arg) - if arg.typ.len != 0 and {tfRequiresInit} * arg.typ.lastSon.flags != {}: + if arg.typ.len != 0 and {tfRequiresInit, tfHasRequiresInit} * arg.typ.lastSon.flags != {}: if a.sym.magic == mNewSeq and n[2].kind in {nkCharLit..nkUInt64Lit} and n[2].intVal == 0: # var s: seq[notnil]; newSeq(s, 0) is a special case! discard else: - message(tracked.config, arg.info, warnProveInit, $arg) + localError(tracked.config, arg.info, errProveInit, $arg) # check required for 'nim check': if n[1].typ.len > 0: @@ -1203,12 +1203,11 @@ proc trackProc*(c: PContext; s: PSym, body: PNode) = createTypeBoundOps(t, typ, param.info) if not isEmptyType(s.typ[0]) and - ({tfRequiresInit, tfNotNil} * s.typ[0].flags != {} or - s.typ[0].skipTypes(abstractInst).kind == tyVar) and - s.kind in {skProc, skFunc, skConverter, skMethod}: + (s.typ[0].requiresInit or s.typ[0].skipTypes(abstractInst).kind == tyVar) and + s.kind in {skProc, skFunc, skConverter, skMethod}: var res = s.ast[resultPos].sym # get result symbol if res.id notin t.init: - message(g.config, body.info, warnProveInit, "result") + localError(g.config, body.info, errProveInit, "result") let p = s.ast[pragmasPos] let raisesSpec = effectSpec(p, wRaises) if not isNil(raisesSpec): diff --git a/compiler/semstmts.nim b/compiler/semstmts.nim index fc624c7101cd5..23e581a53385c 100644 --- a/compiler/semstmts.nim +++ b/compiler/semstmts.nim @@ -335,8 +335,7 @@ proc semIdentDef(c: PContext, n: PNode, kind: TSymKind): PSym = suggestSym(c.config, info, result, c.graph.usageSym) proc checkNilable(c: PContext; v: PSym) = - if {sfGlobal, sfImportc} * v.flags == {sfGlobal} and - {tfNotNil, tfRequiresInit} * v.typ.flags != {}: + if {sfGlobal, sfImportc} * v.flags == {sfGlobal} and v.typ.requiresInit: if v.astdef.isNil: message(c.config, v.info, warnProveInit, v.name.s) elif tfNotNil in v.typ.flags and not v.astdef.typ.isNil and tfNotNil notin v.astdef.typ.flags: @@ -485,7 +484,7 @@ proc semVarOrLet(c: PContext, n: PNode, symkind: TSymKind): PNode = var a = n[i] if c.config.cmd == cmdIdeTools: suggestStmt(c, a) if a.kind == nkCommentStmt: continue - if a.kind notin {nkIdentDefs, nkVarTuple, nkConstDef}: illFormedAst(a, c.config) + if a.kind notin {nkIdentDefs, nkVarTuple}: illFormedAst(a, c.config) checkMinSonsLen(a, 3, c.config) var typ: PType = nil diff --git a/compiler/semtypes.nim b/compiler/semtypes.nim index 92e7b8bd2b27e..1de181dbed9ac 100644 --- a/compiler/semtypes.nim +++ b/compiler/semtypes.nim @@ -139,7 +139,8 @@ proc semEnum(c: PContext, n: PNode, prev: PType): PType = if isPure and (let conflict = strTableInclReportConflict(symbols, e); conflict != nil): wrongRedefinition(c, e.info, e.name.s, conflict.info) inc(counter) - if tfNotNil in e.typ.flags and not hasNull: incl(result.flags, tfRequiresInit) + if tfNotNil in e.typ.flags and not hasNull: + result.flags.incl tfRequiresInit proc semSet(c: PContext, n: PNode, prev: PType): PType = result = newOrPrevType(tySet, prev, c) @@ -769,6 +770,8 @@ proc semRecordNodeAux(c: PContext, n: PNode, check: var IntSet, pos: var int, f.typ = typ f.position = pos f.options = c.config.options + if sfRequiresInit in f.flags: + rectype.flags.incl tfHasRequiresInit if fieldOwner != nil and {sfImportc, sfExportc} * fieldOwner.flags != {} and not hasCaseFields and f.loc.r == nil: diff --git a/tests/constructors/tinvalid_construction.nim b/tests/constructors/tinvalid_construction.nim index f84a18e68aba6..46156db6c589c 100644 --- a/tests/constructors/tinvalid_construction.nim +++ b/tests/constructors/tinvalid_construction.nim @@ -32,10 +32,14 @@ type a {.requiresInit.}: int b: string + PartialRequiresInitRef = ref PartialRequiresInit + FullRequiresInit {.requiresInit.} = object a: int b: int + FullRequiresInitRef = ref FullRequiresInit + FullRequiresInitWithParent {.requiresInit.} = object of THasNotNils e: int d: int @@ -72,8 +76,13 @@ var nilRef: TRefObj var notNilRef = TRefObj(x: 20) proc makeHasNotNils: ref THasNotNils = - result.a = TRefObj(x: 10) - result.b = TRefObj(x: 20) + (ref THasNotNils)(a: TRefObj(x: 10), + b: TRefObj(x: 20)) + +proc userDefinedDefault(T: typedesc): T = + # We'll use that to make sure the user cannot cheat + # with constructing requiresInit types + discard accept TObj() accept TObj(choice: A) @@ -125,7 +134,17 @@ accept PartialRequiresInit(a: 10, b: "x") accept PartialRequiresInit(a: 20) reject PartialRequiresInit(b: "x") reject PartialRequiresInit() +accept PartialRequiresInitRef(a: 10, b: "x") +accept PartialRequiresInitRef(a: 20) +reject PartialRequiresInitRef(b: "x") +reject PartialRequiresInitRef() +accept((ref PartialRequiresInit)(a: 10, b: "x")) +accept((ref PartialRequiresInit)(a: 20)) +reject((ref PartialRequiresInit)(b: "x")) +reject((ref PartialRequiresInit)()) + reject default(PartialRequiresInit) +reject userDefinedDefault(PartialRequiresInit) reject: var obj: PartialRequiresInit @@ -133,7 +152,17 @@ accept FullRequiresInit(a: 10, b: 20) reject FullRequiresInit(a: 10) reject FullRequiresInit(b: 20) reject FullRequiresInit() +accept FullRequiresInitRef(a: 10, b: 20) +reject FullRequiresInitRef(a: 10) +reject FullRequiresInitRef(b: 20) +reject FullRequiresInitRef() +accept((ref FullRequiresInit)(a: 10, b: 20)) +reject((ref FullRequiresInit)(a: 10)) +reject((ref FullRequiresInit)(b: 20)) +reject((ref FullRequiresInit)()) + reject default(FullRequiresInit) +reject userDefinedDefault(FullRequiresInit) reject: var obj: FullRequiresInit @@ -144,6 +173,7 @@ reject FullRequiresInitWithParent(a: notNilRef, b: notNilRef, e: 10, d: 20) # reject FullRequiresInitWithParent(a: notNilRef, b: notNilRef, c: nil, e: 10) # d should not be missing reject FullRequiresInitWithParent() reject default(FullRequiresInitWithParent) +reject userDefinedDefault(FullRequiresInitWithParent) reject: var obj: FullRequiresInitWithParent @@ -153,6 +183,55 @@ accept default(TNestedChoices) accept: var obj: TNestedChoices +reject: + # This proc is illegal, because it tries to produce + # a default object of a type that requires initialization: + proc defaultHasNotNils: THasNotNils = + discard + +reject: + # You cannot cheat by using the result variable to specify + # only some of the fields + proc invalidPartialTHasNotNils: THasNotNils = + result.c = nilRef + +reject: + # The same applies for requiresInit types + proc invalidPartialRequiersInit: PartialRequiresInit = + result.b = "x" + +# All code paths must return a value when the result requires initialization: +reject: + proc ifWithoutAnElse: THasNotNils = + if stdin.readLine == "": + return THasNotNils(a: notNilRef, b: notNilRef, c: nilRef) + +accept: + # All code paths must return a value when the result requires initialization: + proc wellFormedIf: THasNotNils = + if stdin.readLine == "": + return THasNotNils(a: notNilRef, b: notNilRef, c: nilRef) + else: + return THasNotNIls(a: notNilRef, b: notNilRef) + +reject: + proc caseWithoutAllCasesCovered: FullRequiresInit = + # Please note that these is no else branch here: + case stdin.readLine + of "x": + return FullRequiresInit(a: 10, b: 20) + of "y": + return FullRequiresInit(a: 30, b: 40) + +accept: + proc wellFormedCase: FullRequiresInit = + case stdin.readLine + of "x": + result = FullRequiresInit(a: 10, b: 20) + else: + # Mixing result and return is fine: + return FullRequiresInit(a: 30, b: 40) + # but if we supply a run-time value for the inner branch, the compiler won't be able to prove # that the notnil field was initialized reject TNestedChoices(outerChoice: false, innerChoice: x) # XXX: The error message is not very good here From ec0319031551b76c85d078e79fea6630a9dbaa56 Mon Sep 17 00:00:00 2001 From: Zahary Karadjov Date: Sun, 29 Mar 2020 02:34:50 +0200 Subject: [PATCH 08/20] not nil types are illegal to construct through default(T) --- compiler/semexprs.nim | 2 ++ tests/constructors/tinvalid_construction.nim | 14 ++++++++++++++ 2 files changed, 16 insertions(+) diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index d39fe146e7f93..d4f33eb41635c 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -2270,6 +2270,8 @@ proc semMagic(c: PContext, n: PNode, s: PSym, flags: TExprFlags): PNode = let typ = result[1].typ.base if typ.kind == tyObject: checkDefaultConstruction(c, typ, n.info) + elif typ.kind in {tyRef, tyPtr} and tfNotNil in typ.flags: + localError(c.config, n.info, "not nil types don't have a default value") else: result = semDirectOp(c, n, flags) diff --git a/tests/constructors/tinvalid_construction.nim b/tests/constructors/tinvalid_construction.nim index 46156db6c589c..eb84c53c8c719 100644 --- a/tests/constructors/tinvalid_construction.nim +++ b/tests/constructors/tinvalid_construction.nim @@ -17,6 +17,8 @@ type THasNotNilsRef = ref THasNotNils + TRefObjNotNil = TRefObj not nil + TChoice = enum A, B, C, D, E, F TBaseHasNotNils = object of THasNotNils @@ -84,6 +86,9 @@ proc userDefinedDefault(T: typedesc): T = # with constructing requiresInit types discard +proc genericDefault(T: typedesc): T = + result = default(T) + accept TObj() accept TObj(choice: A) reject TObj(choice: A, bc: 10) # bc is in the wrong branch @@ -102,9 +107,18 @@ accept THasNotNils(a: notNilRef, b: notNilRef, c: nilRef) reject THasNotNils(b: notNilRef, c: notNilRef) # there is a missing not nil field reject THasNotNils() # again, missing fields accept THasNotNils(a: notNilRef, b: notNilRef) # it's OK to omit a non-mandatory field +reject default(THasNotNils) +reject userDefinedDefault(THasNotNils) + +reject default(TRefObjNotNil) +reject userDefinedDefault(TRefObjNotNil) +reject genericDefault(TRefObjNotNil) # missing not nils in base reject TBaseHasNotNils() +reject default(TBaseHasNotNils) +reject userDefinedDefault(TBaseHasNotNils) +reject genericDefault(TBaseHasNotNils) # once you take care of them, it's ok accept TBaseHasNotNils(a: notNilRef, b: notNilRef, choice: D) From d9952261224eeea0aaf92570d2109178fee06f36 Mon Sep 17 00:00:00 2001 From: Zahary Karadjov Date: Sun, 29 Mar 2020 02:17:47 +0200 Subject: [PATCH 09/20] Hrm, the new errors highlighted some code that seems to be broken New issue: since `Table[A, B]` allocates its backing storage with `newSeq[KeyValuePair[A, B]]`, it's no longer legal to create a table with `not nil` types used as either keys or values. --- lib/pure/collections/sequtils.nim | 5 ++--- lib/pure/collections/sharedtables.nim | 18 ++++++++---------- lib/pure/ioselects/ioselectors_epoll.nim | 3 +-- lib/pure/ioselects/ioselectors_kqueue.nim | 3 +-- lib/pure/ioselects/ioselectors_select.nim | 8 +++++++- lib/pure/parsecsv.nim | 4 ++-- tests/collections/ttables.nim | 2 +- tests/discard/tneedsdiscard_in_for.nim | 2 +- 8 files changed, 23 insertions(+), 22 deletions(-) diff --git a/lib/pure/collections/sequtils.nim b/lib/pure/collections/sequtils.nim index e32c784c65c4c..893f31389155b 100644 --- a/lib/pure/collections/sequtils.nim +++ b/lib/pure/collections/sequtils.nim @@ -371,9 +371,8 @@ proc map*[T, S](s: openArray[T], op: proc (x: T): S {.closure.}): b = map(a, proc(x: int): string = $x) assert b == @["1", "2", "3", "4"] - newSeq(result, s.len) - for i in 0 ..< s.len: - result[i] = op(s[i]) + result = newSeqOfCap[S](s.len) + for elem in s: result.add op(elem) proc apply*[T](s: var openArray[T], op: proc (x: var T) {.closure.}) {.inline.} = diff --git a/lib/pure/collections/sharedtables.nim b/lib/pure/collections/sharedtables.nim index 2a1c0543f3b3b..96f934f503bf0 100644 --- a/lib/pure/collections/sharedtables.nim +++ b/lib/pure/collections/sharedtables.nim @@ -77,8 +77,7 @@ template withValue*[A, B](t: var SharedTable[A, B], key: A, try: var hc: Hash var index = rawGet(t, key, hc) - let hasKey = index >= 0 - if hasKey: + if index >= 0: var value {.inject.} = addr(t.data[index].val) body finally: @@ -104,8 +103,7 @@ template withValue*[A, B](t: var SharedTable[A, B], key: A, try: var hc: Hash var index = rawGet(t, key, hc) - let hasKey = index >= 0 - if hasKey: + if index >= 0: var value {.inject.} = addr(t.data[index].val) body1 else: @@ -119,13 +117,13 @@ proc mget*[A, B](t: var SharedTable[A, B], key: A): var B = withLock t: var hc: Hash var index = rawGet(t, key, hc) - let hasKey = index >= 0 - if hasKey: result = t.data[index].val - if not hasKey: - when compiles($key): - raise newException(KeyError, "key not found: " & $key) + if index >= 0: + result = t.data[index].val else: - raise newException(KeyError, "key not found") + when compiles($key): + raise newException(KeyError, "key not found: " & $key) + else: + raise newException(KeyError, "key not found") proc mgetOrPut*[A, B](t: var SharedTable[A, B], key: A, val: B): var B = ## retrieves value at ``t[key]`` or puts ``val`` if not present, either way diff --git a/lib/pure/ioselects/ioselectors_epoll.nim b/lib/pure/ioselects/ioselectors_epoll.nim index bf13cc83e8161..002bddac52fce 100644 --- a/lib/pure/ioselects/ioselectors_epoll.nim +++ b/lib/pure/ioselects/ioselectors_epoll.nim @@ -502,8 +502,7 @@ proc contains*[T](s: Selector[T], fd: SocketHandle|int): bool {.inline.} = proc getData*[T](s: Selector[T], fd: SocketHandle|int): var T = let fdi = int(fd) s.checkFd(fdi) - if fdi in s: - result = s.fds[fdi].data + result = s.fds[fdi].data proc setData*[T](s: Selector[T], fd: SocketHandle|int, data: T): bool = let fdi = int(fd) diff --git a/lib/pure/ioselects/ioselectors_kqueue.nim b/lib/pure/ioselects/ioselectors_kqueue.nim index 83e15d479c75e..c5e19a011320d 100644 --- a/lib/pure/ioselects/ioselectors_kqueue.nim +++ b/lib/pure/ioselects/ioselectors_kqueue.nim @@ -600,8 +600,7 @@ proc contains*[T](s: Selector[T], fd: SocketHandle|int): bool {.inline.} = proc getData*[T](s: Selector[T], fd: SocketHandle|int): var T = let fdi = int(fd) s.checkFd(fdi) - if fdi in s: - result = s.fds[fdi].data + result = s.fds[fdi].data proc setData*[T](s: Selector[T], fd: SocketHandle|int, data: T): bool = let fdi = int(fd) diff --git a/lib/pure/ioselects/ioselectors_select.nim b/lib/pure/ioselects/ioselectors_select.nim index 02a853b42948d..6a742df99d5c0 100644 --- a/lib/pure/ioselects/ioselectors_select.nim +++ b/lib/pure/ioselects/ioselectors_select.nim @@ -410,11 +410,17 @@ else: body proc getData*[T](s: Selector[T], fd: SocketHandle|int): var T = + # The compiler needs this to prove that all code paths return a value + result = (cast[ptr T](0'u))[] + s.withSelectLock(): let fdi = int(fd) for i in 0..= 0: - result = my.row[index] + assert index >= 0 + result = my.row[index] when not defined(testing) and isMainModule: import os diff --git a/tests/collections/ttables.nim b/tests/collections/ttables.nim index a59707865d07c..2a590dd26ce5b 100644 --- a/tests/collections/ttables.nim +++ b/tests/collections/ttables.nim @@ -80,7 +80,7 @@ block thashes: # Test with range block: type - R = range[1..10] + R = range[0..9] var t = initTable[R,int]() # causes warning, why? t[1] = 42 # causes warning, why? t[2] = t[1] + 1 diff --git a/tests/discard/tneedsdiscard_in_for.nim b/tests/discard/tneedsdiscard_in_for.nim index 499b06009176e..ab5216150ece0 100644 --- a/tests/discard/tneedsdiscard_in_for.nim +++ b/tests/discard/tneedsdiscard_in_for.nim @@ -8,7 +8,7 @@ type Rgba8 = object proc premultiply*(c: var Rgba8): var Rgba8 = - discard + return c type App = ref object From b1290c67b6b6344af1fd4d3a308b383b35163fe9 Mon Sep 17 00:00:00 2001 From: Zahary Karadjov Date: Sun, 29 Mar 2020 19:05:01 +0300 Subject: [PATCH 10/20] More precise error messages for uninitialized fields in the presence of inheritance --- compiler/astalgo.nim | 7 ++++ compiler/semobjconstr.nim | 60 +++++++++++---------------- tests/notnil/tnotnil_in_objconstr.nim | 10 +++-- 3 files changed, 39 insertions(+), 38 deletions(-) diff --git a/compiler/astalgo.nim b/compiler/astalgo.nim index 968918ba9711a..bd60ddd7db27a 100644 --- a/compiler/astalgo.nim +++ b/compiler/astalgo.nim @@ -1041,3 +1041,10 @@ proc isAddrNode*(n: PNode): bool = if n[0].kind == nkSym and n[0].sym.magic == mAddr: true else: false else: false + +proc listSymbolNames*(symbols: openArray[PSym]): string = + for sym in symbols: + if result.len > 0: + result.add ", " + result.add sym.name.s + diff --git a/compiler/semobjconstr.nim b/compiler/semobjconstr.nim index 7222433b31d04..9d39dd5f874e2 100644 --- a/compiler/semobjconstr.nim +++ b/compiler/semobjconstr.nim @@ -13,11 +13,12 @@ type ObjConstrContext = object - typ: PType # The constructed type - initExpr: PNode # The init expression (nkObjConstr) - requiresFullInit: bool # A `requiresInit` derived type will - # set this to true while visiting - # parent types. + typ: PType # The constructed type + initExpr: PNode # The init expression (nkObjConstr) + requiresFullInit: bool # A `requiresInit` derived type will + # set this to true while visiting + # parent types. + missingFields: seq[PSym] # Fields that the user failed to specify InitStatus = enum # This indicates the result of object construction initUnknown @@ -143,30 +144,18 @@ proc fieldsPresentInInitExpr(c: PContext, fieldsRecList, initExpr: PNode): strin if result.len != 0: result.add ", " result.add field.sym.name.s.quoteStr -proc missingMandatoryFields(c: PContext, fieldsRecList: PNode, - constrCtx: ObjConstrContext): string = +proc collectMissingFields(c: PContext, fieldsRecList: PNode, + constrCtx: var ObjConstrContext) = for r in directFieldsInRecList(fieldsRecList): if constrCtx.requiresFullInit or sfRequiresInit in r.sym.flags or r.sym.typ.requiresInit: let assignment = locateFieldInInitExpr(c, r.sym, constrCtx.initExpr) if assignment == nil: - if result.len == 0: - result = r.sym.name.s - else: - result.add ", " - result.add r.sym.name.s - -proc checkForMissingFields(c: PContext, recList: PNode, - constrCtx: ObjConstrContext) = - let missing = missingMandatoryFields(c, recList, constrCtx) - if missing.len > 0: - localError(c.config, constrCtx.initExpr.info, - "The $1 type requires the following fields to be initialized: $2.", - [constrCtx.typ.sym.name.s, missing]) + constrCtx.missingFields.add r.sym proc semConstructFields(c: PContext, recNode: PNode, - constrCtx: ObjConstrContext, + constrCtx: var ObjConstrContext, flags: TExprFlags): InitStatus = result = initUnknown @@ -182,10 +171,10 @@ proc semConstructFields(c: PContext, recNode: PNode, let fields = branch[^1] fieldsPresentInInitExpr(c, fields, constrCtx.initExpr) - template checkMissingFields(branchNode: PNode) = + template collectMissingFields(branchNode: PNode) = if branchNode != nil: let fields = branchNode[^1] - checkForMissingFields(c, fields, constrCtx) + collectMissingFields(c, fields, constrCtx) let discriminator = recNode[0] internalAssert c.config, discriminator.kind == nkSym @@ -299,7 +288,7 @@ proc semConstructFields(c: PContext, recNode: PNode, # When a branch is selected with a partial match, some of the fields # that were not initialized may be mandatory. We must check for this: if result == initPartial: - checkMissingFields branchNode + collectMissingFields branchNode else: result = initNone @@ -313,18 +302,18 @@ proc semConstructFields(c: PContext, recNode: PNode, # a result: let defaultValue = newIntLit(c.graph, constrCtx.initExpr.info, 0) let matchedBranch = recNode.pickCaseBranch defaultValue - checkMissingFields matchedBranch + collectMissingFields matchedBranch else: result = initPartial if discriminatorVal.kind == nkIntLit: # When the discriminator is a compile-time value, we also know # which branch will be selected: let matchedBranch = recNode.pickCaseBranch discriminatorVal - if matchedBranch != nil: checkMissingFields matchedBranch + if matchedBranch != nil: collectMissingFields matchedBranch else: # All bets are off. If any of the branches has a mandatory # fields we must produce an error: - for i in 1.. 0: + localError(c.config, constrCtx.initExpr.info, + "The $1 type requires the following fields to be initialized: $2.", + [constrCtx.typ.sym.name.s, listSymbolNames(constrCtx.missingFields)]) + proc checkDefaultConstruction(c: PContext, typ: PType, info: TLineInfo) = discard semConstructType(c, newNodeI(nkObjConstr, info), typ, {}) @@ -381,13 +378,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 - # specifying a .requiresInit. pragma. - if tfRequiresInit in t.flags and initResult != initFull: - localError(c.config, n.info, - "object type uses the 'requiresInit' pragma, but not all fields " & - "have been initialized.") - # Since we were traversing the object fields, it's possible that # not all of the fields specified in the constructor was visited. # We'll check for such fields here: diff --git a/tests/notnil/tnotnil_in_objconstr.nim b/tests/notnil/tnotnil_in_objconstr.nim index 030c40d32144f..471150f44625a 100644 --- a/tests/notnil/tnotnil_in_objconstr.nim +++ b/tests/notnil/tnotnil_in_objconstr.nim @@ -1,13 +1,17 @@ discard """ - errormsg: "The Foo type requires the following fields to be initialized: bar" - line: "13" + errormsg: "The Foo type requires the following fields to be initialized: bar, baz" + line: "17" """ {.experimental: "notnil".} # bug #2355 type - Foo = object + Base = object of RootObj + baz: ref int not nil + + Foo = object of Base foo: ref int bar: ref int not nil + var x: ref int = new(int) # Create instance without initializing the `bar` field var f = Foo(foo: x) From 85256ed773dfc3510bc3f84557232ab3f51e1ef2 Mon Sep 17 00:00:00 2001 From: Zahary Karadjov Date: Sun, 29 Mar 2020 20:31:37 +0300 Subject: [PATCH 11/20] Perform nil checks during object construction and within compiles() Close https://github.com/nim-lang/Nim/issues/6494 --- compiler/sem.nim | 2 +- compiler/semexprs.nim | 5 ++++- compiler/sempass2.nim | 5 +++-- tests/constructors/tinvalid_construction.nim | 5 ++--- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/compiler/sem.nim b/compiler/sem.nim index 3a4ff2cd9ba9d..07b765f88069c 100644 --- a/compiler/sem.nim +++ b/compiler/sem.nim @@ -584,7 +584,7 @@ proc semStmtAndGenerateGenerics(c: PContext, n: PNode): PNode = result = buildEchoStmt(c, result) if c.config.cmd == cmdIdeTools: appendToModule(c.module, result) - trackTopLevelStmt(c, c.module, result) + trackStmt(c, c.module, result, isTopLevel = true) proc recoverContext(c: PContext) = # clean up in case of a semantic error: We clean up the stacks, etc. This is diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index d4f33eb41635c..5484f4594a020 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -2082,7 +2082,10 @@ proc tryExpr(c: PContext, n: PNode, flags: TExprFlags = {}): PNode = var err: string try: result = semExpr(c, n, flags) - if c.config.errorCounter != oldErrorCount: result = nil + if result != nil: + trackStmt(c, c.module, result, isTopLevel = false) + if c.config.errorCounter != oldErrorCount: + result = nil except ERecoverableError: discard # undo symbol table changes (as far as it's possible): diff --git a/compiler/sempass2.nim b/compiler/sempass2.nim index 2d5e3e7212c7d..3ad0a1931c969 100644 --- a/compiler/sempass2.nim +++ b/compiler/sempass2.nim @@ -996,6 +996,7 @@ proc track(tracked: PEffects, n: PNode) = createTypeBoundOps(tracked, x[1].typ, n.info) if x.kind == nkExprColonExpr: + notNilCheck(tracked, x[1], x[0].sym.typ) checkForSink(tracked.config, tracked.owner, x[1]) else: checkForSink(tracked.config, tracked.owner, x) @@ -1260,7 +1261,7 @@ proc trackProc*(c: PContext; s: PSym, body: PNode) = dataflowAnalysis(s, body) when false: trackWrites(s, body) -proc trackTopLevelStmt*(c: PContext; module: PSym; n: PNode) = +proc trackStmt*(c: PContext; module: PSym; n: PNode, isTopLevel: bool) = if n.kind in {nkPragma, nkMacroDef, nkTemplateDef, nkProcDef, nkFuncDef, nkTypeSection, nkConverterDef, nkMethodDef, nkIteratorDef}: return @@ -1268,5 +1269,5 @@ proc trackTopLevelStmt*(c: PContext; module: PSym; n: PNode) = var effects = newNode(nkEffectList, n.info) var t: TEffects initEffects(g, effects, module, t, c) - t.isTopLevel = true + t.isTopLevel = isTopLevel track(t, n) diff --git a/tests/constructors/tinvalid_construction.nim b/tests/constructors/tinvalid_construction.nim index eb84c53c8c719..e6fdbcb85e380 100644 --- a/tests/constructors/tinvalid_construction.nim +++ b/tests/constructors/tinvalid_construction.nim @@ -75,7 +75,7 @@ type var x = D var nilRef: TRefObj -var notNilRef = TRefObj(x: 20) +let notNilRef = TRefObjNotNil(x: 20) proc makeHasNotNils: ref THasNotNils = (ref THasNotNils)(a: TRefObj(x: 10), @@ -102,8 +102,7 @@ reject TObj(a: 10, f: "") # conflicting fields accept TObj(choice: E, e1: TRefObj(x: 10), e2: 10) accept THasNotNils(a: notNilRef, b: notNilRef, c: nilRef) -# XXX: the "not nil" logic in the compiler is not strong enough to catch this one yet: -# reject THasNotNils(a: notNilRef, b: nilRef, c: nilRef) +reject THasNotNils(a: notNilRef, b: nilRef, c: nilRef) # `b` shouldn't be nil reject THasNotNils(b: notNilRef, c: notNilRef) # there is a missing not nil field reject THasNotNils() # again, missing fields accept THasNotNils(a: notNilRef, b: notNilRef) # it's OK to omit a non-mandatory field From 3205d4e9f4fa2cfb54bdb347603818a009714452 Mon Sep 17 00:00:00 2001 From: Zahary Karadjov Date: Sun, 29 Mar 2020 20:49:13 +0300 Subject: [PATCH 12/20] Close https://github.com/nim-lang/Nim/issues/11428 --- tests/constructors/tinvalid_construction.nim | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/constructors/tinvalid_construction.nim b/tests/constructors/tinvalid_construction.nim index e6fdbcb85e380..c17e2123c65cc 100644 --- a/tests/constructors/tinvalid_construction.nim +++ b/tests/constructors/tinvalid_construction.nim @@ -256,3 +256,15 @@ reject TNestedChoices(outerChoice: false, innerChoice: C) accept TNestedChoices(outerChoice: false, innerChoice: C, notnil: notNilRef) reject TNestedChoices(outerChoice: false, innerChoice: C, notnil: nil) +block: + # https://github.com/nim-lang/Nim/issues/11428 + type + Enum = enum A, B, C + Thing = object + case kind: Enum + of A: discard + of B: s: string + of C: r: range[1..1] # DateTime + + # Fine to not initialize 'r' because this is implicitly initialized and known to be branch 'A'. + let someThing = Thing() From eda7525219efaad7c1ee00ce60dddf2725e54e93 Mon Sep 17 00:00:00 2001 From: Zahary Karadjov Date: Mon, 30 Mar 2020 00:49:42 +0300 Subject: [PATCH 13/20] Fix https://github.com/nim-lang/Nim/issues/4907 --- compiler/semexprs.nim | 3 ++ compiler/semtypes.nim | 39 +++++++++++++++++-- tests/constructors/tinvalid_construction.nim | 41 +++++++++++++++++++- 3 files changed, 78 insertions(+), 5 deletions(-) diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index 5484f4594a020..e405ec0ef78f3 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -2655,6 +2655,9 @@ proc semExpr(c: PContext, n: PNode, flags: TExprFlags = {}): PNode = return var typ = semTypeNode(c, n, nil).skipTypes({tyTypeDesc}) result.typ = makeTypeDesc(c, typ) + of nkStmtListType: + let typ = semTypeNode(c, n, nil) + result.typ = makeTypeDesc(c, typ) of nkCall, nkInfix, nkPrefix, nkPostfix, nkCommand, nkCallStrLit: # check if it is an expression macro: checkMinSonsLen(n, 1, c.config) diff --git a/compiler/semtypes.nim b/compiler/semtypes.nim index 1de181dbed9ac..39df93b62f06e 100644 --- a/compiler/semtypes.nim +++ b/compiler/semtypes.nim @@ -1720,12 +1720,43 @@ proc semTypeNode(c: PContext, n: PNode, prev: PType): PType = case n.len of 3: result = semTypeNode(c, n[1], prev) - if result.skipTypes({tyGenericInst, tyAlias, tySink, tyOwned}).kind in NilableTypes+GenericTypes+{tyForward} and - n[2].kind == nkNilLit: + if result.kind == tyTypeDesc and tfUnresolved notin result.flags: + result = result.base + if n[2].kind != nkNilLit: + localError(c.config, n.info, + "Invalid syntax. When used with a type, 'not' can be followed only by 'nil'") + if notnil notin c.features: + localError(c.config, n.info, + "enable the 'not nil' annotation with {.experimental: \"notnil\".}") + let resolvedType = result.skipTypes({tyGenericInst, tyAlias, tySink, tyOwned}) + case resolvedType.kind + of tyGenericParam, tyTypeDesc, tyFromExpr: + # XXX: This is a really inappropraite hack, but it solves + # https://github.com/nim-lang/Nim/issues/4907 for now. + # + # A proper solution is to introduce a new type kind such + # as `tyNotNil[tyRef[SomeGenericParam]]`. This will allow + # semtypinst to replace the generic param correctly in + # situations like the following: + # + # type Foo[T] = object + # bar: ref T not nil + # baz: ref T + # + # The root of the problem is that `T` here must have a specific + # ID that is bound to a concrete type during instantiation. + # The use of `freshType` below breaks this. Another hack would + # be to reuse the same ID for the not nil type, but this will + # fail if the `T` parameter is referenced multiple times as in + # the example above. + # + # I suggest revisiting this once the language decides on whether + # `not nil` should be the default. We can then map nilable refs + # to other types such as `Option[T]`. + result = makeTypeFromExpr(c, newTree(nkStmtListType, n.copyTree)) + of NilableTypes + {tyGenericInvocation, tyForward}: result = freshType(result, prev) result.flags.incl(tfNotNil) - if notnil notin c.features: - localError(c.config, n.info, "enable the 'not nil' annotation with {.experimental: \"notnil\".}") else: localError(c.config, n.info, errGenerated, "invalid type") of 2: diff --git a/tests/constructors/tinvalid_construction.nim b/tests/constructors/tinvalid_construction.nim index c17e2123c65cc..6716bfb452fc6 100644 --- a/tests/constructors/tinvalid_construction.nim +++ b/tests/constructors/tinvalid_construction.nim @@ -267,4 +267,43 @@ block: of C: r: range[1..1] # DateTime # Fine to not initialize 'r' because this is implicitly initialized and known to be branch 'A'. - let someThing = Thing() + var x = Thing() + discard x + +block: + # https://github.com/nim-lang/Nim/issues/4907 + type + Foo = ref object + Bar = object + + Thing[A, B] = ref object + a: A not nil + b: ref B + c: ref B not nil + + proc allocNotNil(T: typedesc): T not nil = + new result + + proc mutateThing(t: var Thing[Foo, Bar]) = + let fooNotNil = allocNotNil(Foo) + var foo: Foo + + let barNotNil = allocNotNil(ref Bar) + var bar: ref Bar + + t.a = fooNotNil + t.b = bar + t.b = barNotNil + t.c = barNotNil + + reject: + t.a = foo + + reject: + t.c = bar + + var thing = Thing[Foo, Bar](a: allocNotNil(Foo), + b: allocNotNil(ref Bar), + c: allocNotNil(ref Bar)) + mutateThing thing + From 4ed360ef5b3a061a6f3e4a5208403e3ca3a9a86e Mon Sep 17 00:00:00 2001 From: Zahary Karadjov Date: Mon, 30 Mar 2020 01:38:59 +0300 Subject: [PATCH 14/20] Fix a CI failure during koch doc --- compiler/sempass2.nim | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/sempass2.nim b/compiler/sempass2.nim index 3ad0a1931c969..40b3b15ce4d5a 100644 --- a/compiler/sempass2.nim +++ b/compiler/sempass2.nim @@ -996,7 +996,8 @@ proc track(tracked: PEffects, n: PNode) = createTypeBoundOps(tracked, x[1].typ, n.info) if x.kind == nkExprColonExpr: - notNilCheck(tracked, x[1], x[0].sym.typ) + if x[0].kind == nkSym: + notNilCheck(tracked, x[1], x[0].sym.typ) checkForSink(tracked.config, tracked.owner, x[1]) else: checkForSink(tracked.config, tracked.owner, x) From 98ccf47dd46da898d95c2cbff6ae814a9fd73a95 Mon Sep 17 00:00:00 2001 From: Zahary Karadjov Date: Mon, 30 Mar 2020 03:47:02 +0300 Subject: [PATCH 15/20] Fix tests/parallel/tguard2.nim --- compiler/semdata.nim | 2 +- compiler/semexprs.nim | 2 +- compiler/sigmatch.nim | 4 +++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/compiler/semdata.nim b/compiler/semdata.nim index 367c0eaf673ae..7fa6717226bea 100644 --- a/compiler/semdata.nim +++ b/compiler/semdata.nim @@ -63,7 +63,7 @@ type # to the user. efWantStmt, efAllowStmt, efDetermineType, efExplain, efAllowDestructor, efWantValue, efOperand, efNoSemCheck, - efNoEvaluateGeneric, efInCall, efFromHlo, + efNoEvaluateGeneric, efInCall, efFromHlo, efNoSem2Check, efNoUndeclared # Use this if undeclared identifiers should not raise an error during # overload resolution. diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index e405ec0ef78f3..d920b0f25a211 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -2082,7 +2082,7 @@ proc tryExpr(c: PContext, n: PNode, flags: TExprFlags = {}): PNode = var err: string try: result = semExpr(c, n, flags) - if result != nil: + if result != nil and efNoSem2Check notin flags: trackStmt(c, c.module, result, isTopLevel = false) if c.config.errorCounter != oldErrorCount: result = nil diff --git a/compiler/sigmatch.nim b/compiler/sigmatch.nim index 79b3ea94c5384..04ceb47b8221d 100644 --- a/compiler/sigmatch.nim +++ b/compiler/sigmatch.nim @@ -1937,7 +1937,9 @@ proc localConvMatch(c: PContext, m: var TCandidate, f, a: PType, var call = newNodeI(nkCall, arg.info) call.add(f.n.copyTree) call.add(arg.copyTree) - result = c.semTryExpr(c, call) + # XXX: This would be much nicer if we don't use `semTryExpr` and + # instead we directly search for overloads with `resolveOverloads`: + result = c.semTryExpr(c, call, {efNoSem2Check}) if result != nil: if result.typ == nil: return nil From d0b86dd6e82bed35ebf5301c81931a071d2fbf00 Mon Sep 17 00:00:00 2001 From: Zahary Karadjov Date: Mon, 30 Mar 2020 18:56:03 +0300 Subject: [PATCH 16/20] Replace tfHasRequiresInit with a more accurate mechanism The new mechanism can deal with more complex scenarios such as not nil field appearing in a non-default case object branch or a field within a generic object that may depend on a when branch. The commit also plugs another hole: the user is no longer able to create illegal default values through seq.setLen(N). --- compiler/ast.nim | 14 +-- compiler/pragmas.nim | 2 +- compiler/sem.nim | 4 +- compiler/semdata.nim | 1 + compiler/semexprs.nim | 14 ++- compiler/semobjconstr.nim | 61 ++++++---- compiler/sempass2.nim | 6 +- compiler/semstmts.nim | 6 +- compiler/semtypes.nim | 4 +- compiler/semtypinst.nim | 2 + tests/constructors/tinvalid_construction.nim | 112 +++++++++++++++++++ 11 files changed, 181 insertions(+), 45 deletions(-) diff --git a/compiler/ast.nim b/compiler/ast.nim index 01f9ed29bd4a0..f4f68081172ae 100644 --- a/compiler/ast.nim +++ b/compiler/ast.nim @@ -517,11 +517,11 @@ type tfIterator, # type is really an iterator, not a tyProc tfPartial, # type is declared as 'partial' tfNotNil, # type cannot be 'nil' - - tfHasRequiresInit,# type constains a "not nil" constraint somewhere or + tfRequiresInit, # type constains a "not nil" constraint somewhere or # a `requiresInit` field, so the default zero init # is not appropriate - tfRequiresInit, # all fields of the type must be initialized + tfNeedsFullInit, # object type marked with {.requiresInit.} + # all fields must be initialized tfVarIsPtr, # 'var' type is translated like 'ptr' even in C++ mode tfHasMeta, # type contains "wildcard" sub-types such as generic params # or other type classes @@ -1397,7 +1397,7 @@ proc copyType*(t: PType, owner: PSym, keepId: bool): PType = proc exactReplica*(t: PType): PType = copyType(t, t.owner, true) template requiresInit*(t: PType): bool = - t.flags * {tfRequiresInit, tfHasRequiresInit, tfNotNil} != {} + t.flags * {tfRequiresInit, tfNotNil} != {} proc copySym*(s: PSym): PSym = result = newSym(s.kind, s.name, s.owner, s.info, s.options) @@ -1489,12 +1489,6 @@ proc propagateToOwner*(owner, elem: PType; propagateHasAsgn = true) = if tfNotNil in elem.flags: if owner.kind in {tyGenericInst, tyGenericBody, tyGenericInvocation}: owner.flags.incl tfNotNil - elif owner.kind notin HaveTheirOwnEmpty: - owner.flags.incl tfHasRequiresInit - - if {tfRequiresInit, tfHasRequiresInit} * elem.flags != {}: - if owner.kind in HaveTheirOwnEmpty: discard - else: owner.flags.incl tfHasRequiresInit if elem.isMetaType: owner.flags.incl tfHasMeta diff --git a/compiler/pragmas.nim b/compiler/pragmas.nim index 604ba063fcba6..100ce156021db 100644 --- a/compiler/pragmas.nim +++ b/compiler/pragmas.nim @@ -1090,7 +1090,7 @@ proc singlePragma(c: PContext, sym: PSym, n: PNode, i: var int, if sym.kind == skField: sym.flags.incl sfRequiresInit elif sym.typ != nil: - incl(sym.typ.flags, tfRequiresInit) + incl(sym.typ.flags, tfNeedsFullInit) else: invalidPragma(c, it) of wByRef: diff --git a/compiler/sem.nim b/compiler/sem.nim index 07b765f88069c..471a300995721 100644 --- a/compiler/sem.nim +++ b/compiler/sem.nim @@ -46,7 +46,6 @@ proc addParams(c: PContext, n: PNode, kind: TSymKind) proc maybeAddResult(c: PContext, s: PSym, n: PNode) proc tryExpr(c: PContext, n: PNode, flags: TExprFlags = {}): PNode proc activate(c: PContext, n: PNode) -proc checkDefaultConstruction(c: PContext, typ: PType, info: TLineInfo) proc semQuoteAst(c: PContext, n: PNode): PNode proc finishMethod(c: PContext, s: PSym) proc evalAtCompileTime(c: PContext, n: PNode): PNode @@ -54,6 +53,8 @@ proc indexTypesMatch(c: PContext, f, a: PType, arg: PNode): PNode proc semStaticExpr(c: PContext, n: PNode): PNode proc semStaticType(c: PContext, childNode: PNode, prev: PType): PType proc semTypeOf(c: PContext; n: PNode): PNode +proc computeRequiresInit(c: PContext, t: PType): bool +proc defaultConstructionError(c: PContext, t: PType, info: TLineInfo) proc hasUnresolvedArgs(c: PContext, n: PNode): bool proc isArrayConstr(n: PNode): bool {.inline.} = result = n.kind == nkBracket and @@ -512,6 +513,7 @@ proc myOpen(graph: ModuleGraph; module: PSym): PPassContext = c.semExpr = semExpr c.semTryExpr = tryExpr c.semTryConstExpr = tryConstExpr + c.computeRequiresInit = computeRequiresInit c.semOperand = semOperand c.semConstBoolExpr = semConstBoolExpr c.semOverloadedCall = semOverloadedCall diff --git a/compiler/semdata.nim b/compiler/semdata.nim index 7fa6717226bea..fbc9762f3bd24 100644 --- a/compiler/semdata.nim +++ b/compiler/semdata.nim @@ -103,6 +103,7 @@ type semExpr*: proc (c: PContext, n: PNode, flags: TExprFlags = {}): PNode {.nimcall.} semTryExpr*: proc (c: PContext, n: PNode, flags: TExprFlags = {}): PNode {.nimcall.} semTryConstExpr*: proc (c: PContext, n: PNode): PNode {.nimcall.} + computeRequiresInit*: proc (c: PContext, t: PType): bool {.nimcall.} semOperand*: proc (c: PContext, n: PNode, flags: TExprFlags = {}): PNode {.nimcall.} semConstBoolExpr*: proc (c: PContext, n: PNode): PNode {.nimcall.} # XXX bite the bullet semOverloadedCall*: proc (c: PContext, n, nOrig: PNode, diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index d920b0f25a211..56de00d56231c 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -2267,13 +2267,19 @@ proc semMagic(c: PContext, n: PNode, s: PSym, flags: TExprFlags): PNode = of mSizeOf: markUsed(c, n.info, s) result = semSizeof(c, setMs(n, s)) + of mSetLengthSeq: + result = semDirectOp(c, n, flags) + let seqType = result[1].typ.skipTypes({tyPtr, tyRef, # in case we had auto-dereferencing + tyVar, tyGenericInst, tyOwned, tySink, + tyAlias, tyUserTypeClassInst}) + if seqType.kind == tySequence and seqType.base.requiresInit: + localError(c.config, n.info, "setLen can potentially expand the sequence, " & + "but the element type $1 doesn't have a default value.", + [typeToString(seqType.base)]) of mDefault: result = semDirectOp(c, n, flags) c.config.internalAssert result[1].typ.kind == tyTypeDesc - let typ = result[1].typ.base - if typ.kind == tyObject: - checkDefaultConstruction(c, typ, n.info) - elif typ.kind in {tyRef, tyPtr} and tfNotNil in typ.flags: + if result[1].typ.base.requiresInit: localError(c.config, n.info, "not nil types don't have a default value") else: result = semDirectOp(c, n, flags) diff --git a/compiler/semobjconstr.nim b/compiler/semobjconstr.nim index 9d39dd5f874e2..eb05896011fd5 100644 --- a/compiler/semobjconstr.nim +++ b/compiler/semobjconstr.nim @@ -15,7 +15,7 @@ type ObjConstrContext = object typ: PType # The constructed type initExpr: PNode # The init expression (nkObjConstr) - requiresFullInit: bool # A `requiresInit` derived type will + needsFullInit: bool # A `requiresInit` derived type will # set this to true while visiting # parent types. missingFields: seq[PSym] # Fields that the user failed to specify @@ -147,7 +147,7 @@ proc fieldsPresentInInitExpr(c: PContext, fieldsRecList, initExpr: PNode): strin proc collectMissingFields(c: PContext, fieldsRecList: PNode, constrCtx: var ObjConstrContext) = for r in directFieldsInRecList(fieldsRecList): - if constrCtx.requiresFullInit or + if constrCtx.needsFullInit or sfRequiresInit in r.sym.flags or r.sym.typ.requiresInit: let assignment = locateFieldInInitExpr(c, r.sym, constrCtx.initExpr) @@ -323,14 +323,11 @@ proc semConstructFields(c: PContext, recNode: PNode, else: internalAssert c.config, false -proc semConstructType(c: PContext, initExpr: PNode, - t: PType, flags: TExprFlags): InitStatus = +proc semConstructTypeAux(c: PContext, + constrCtx: var ObjConstrContext, + flags: TExprFlags): InitStatus = result = initUnknown - - var - t = t - constrCtx = ObjConstrContext(typ: t, initExpr: initExpr, - requiresFullInit: tfRequiresInit in t.flags) + var t = constrCtx.typ while true: let status = semConstructFields(c, t.n, constrCtx, flags) mergeInitStatus(result, status) @@ -339,18 +336,30 @@ proc semConstructType(c: PContext, initExpr: PNode, let base = t[0] if base == nil: break t = skipTypes(base, skipPtrs) - constrCtx.requiresFullInit = constrCtx.requiresFullInit or - tfRequiresInit in t.flags - - # It's possible that the object was not fully initialized while - # specifying a .requiresInit. pragma: - if constrCtx.missingFields.len > 0: - localError(c.config, constrCtx.initExpr.info, - "The $1 type requires the following fields to be initialized: $2.", - [constrCtx.typ.sym.name.s, listSymbolNames(constrCtx.missingFields)]) - -proc checkDefaultConstruction(c: PContext, typ: PType, info: TLineInfo) = - discard semConstructType(c, newNodeI(nkObjConstr, info), typ, {}) + constrCtx.needsFullInit = constrCtx.needsFullInit or + tfNeedsFullInit in t.flags + +proc initConstrContext(t: PType, initExpr: PNode): ObjConstrContext = + ObjConstrContext(typ: t, initExpr: initExpr, + needsFullInit: tfNeedsFullInit in t.flags) + +proc computeRequiresInit(c: PContext, t: PType): bool = + assert t.kind == tyObject + var constrCtx = initConstrContext(t, newNode(nkObjConstr)) + let initResult = semConstructTypeAux(c, constrCtx, {}) + constrCtx.missingFields.len > 0 + +proc defaultConstructionError(c: PContext, t: PType, info: TLineInfo) = + var objType = t + while objType.kind != tyObject: + objType = objType.lastSon + assert objType != nil + var constrCtx = initConstrContext(objType, newNodeI(nkObjConstr, info)) + let initResult = semConstructTypeAux(c, constrCtx, {}) + assert constrCtx.missingFields.len > 0 + localError(c.config, info, + "The $1 type doesn't have a default value. The following fields must be initialized: $2.", + [typeToString(t), listSymbolNames(constrCtx.missingFields)]) proc semObjConstr(c: PContext, n: PNode, flags: TExprFlags): PNode = var t = semTypeNode(c, n[0], nil) @@ -376,7 +385,15 @@ proc semObjConstr(c: PContext, n: PNode, flags: TExprFlags): PNode = # Check if the object is fully initialized by recursively testing each # field (if this is a case object, initialized fields in two different # branches will be reported as an error): - let initResult = semConstructType(c, result, t, flags) + var constrCtx = initConstrContext(t, result) + let initResult = semConstructTypeAux(c, constrCtx, flags) + + # It's possible that the object was not fully initialized while + # specifying a .requiresInit. pragma: + if constrCtx.missingFields.len > 0: + localError(c.config, result.info, + "The $1 type requires the following fields to be initialized: $2.", + [t.sym.name.s, listSymbolNames(constrCtx.missingFields)]) # Since we were traversing the object fields, it's possible that # not all of the fields specified in the constructor was visited. diff --git a/compiler/sempass2.nim b/compiler/sempass2.nim index 40b3b15ce4d5a..79d969fbb2fb1 100644 --- a/compiler/sempass2.nim +++ b/compiler/sempass2.nim @@ -184,7 +184,7 @@ proc initVar(a: PEffects, n: PNode; volatileCheck: bool) = proc initVarViaNew(a: PEffects, n: PNode) = if n.kind != nkSym: return let s = n.sym - if {tfRequiresInit, tfHasRequiresInit, tfNotNil} * s.typ.flags <= {tfNotNil}: + if {tfRequiresInit, tfNotNil} * s.typ.flags <= {tfNotNil}: # 'x' is not nil, but that doesn't mean its "not nil" children # are initialized: initVar(a, n, volatileCheck=true) @@ -590,7 +590,7 @@ proc trackCase(tracked: PEffects, n: PNode) = track(tracked, n[0]) let oldState = tracked.init.len let oldFacts = tracked.guards.s.len - let stringCase = skipTypes(n[0].typ, + let stringCase = n[0].typ != nil and skipTypes(n[0].typ, abstractVarRange-{tyTypeDesc}).kind in {tyFloat..tyFloat128, tyString} let interesting = not stringCase and interestingCaseExpr(n[0]) and tracked.config.hasWarn(warnProveField) @@ -838,7 +838,7 @@ proc track(tracked: PEffects, n: PNode) = # may not look like an assignment, but it is: let arg = n[1] initVarViaNew(tracked, arg) - if arg.typ.len != 0 and {tfRequiresInit, tfHasRequiresInit} * arg.typ.lastSon.flags != {}: + if arg.typ.len != 0 and {tfRequiresInit} * arg.typ.lastSon.flags != {}: if a.sym.magic == mNewSeq and n[2].kind in {nkCharLit..nkUInt64Lit} and n[2].intVal == 0: # var s: seq[notnil]; newSeq(s, 0) is a special case! diff --git a/compiler/semstmts.nim b/compiler/semstmts.nim index 23e581a53385c..215d554fb3ad3 100644 --- a/compiler/semstmts.nim +++ b/compiler/semstmts.nim @@ -614,8 +614,10 @@ proc semVarOrLet(c: PContext, n: PNode, symkind: TSymKind): PNode = else: v.typ = tup b[j] = newSymNode(v) if def.kind == nkEmpty: - if v.typ.kind == tyObject: - checkDefaultConstruction(c, v.typ, v.info) + let actualType = v.typ.skipTypes({tyGenericInst, tyAlias, + tyUserTypeClassInst}) + if actualType.kind == tyObject and actualType.requiresInit: + defaultConstructionError(c, v.typ, v.info) else: checkNilable(c, v) if sfCompileTime in v.flags: diff --git a/compiler/semtypes.nim b/compiler/semtypes.nim index 39df93b62f06e..21b95d3f64419 100644 --- a/compiler/semtypes.nim +++ b/compiler/semtypes.nim @@ -770,8 +770,6 @@ proc semRecordNodeAux(c: PContext, n: PNode, check: var IntSet, pos: var int, f.typ = typ f.position = pos f.options = c.config.options - if sfRequiresInit in f.flags: - rectype.flags.incl tfHasRequiresInit if fieldOwner != nil and {sfImportc, sfExportc} * fieldOwner.flags != {} and not hasCaseFields and f.loc.r == nil: @@ -879,6 +877,8 @@ proc semObjectNode(c: PContext, n: PNode, prev: PType; isInheritable: bool): PTy pragma(c, s, n[0], typePragmas) if base == nil and tfInheritable notin result.flags: incl(result.flags, tfFinal) + if c.inGenericContext == 0 and computeRequiresInit(c, result): + result.flags.incl tfRequiresInit proc semAnyRef(c: PContext; n: PNode; kind: TTypeKind; prev: PType): PType = if n.len < 1: diff --git a/compiler/semtypinst.nim b/compiler/semtypinst.nim index 5aec3eed9039f..08ea85708aa43 100644 --- a/compiler/semtypinst.nim +++ b/compiler/semtypinst.nim @@ -613,6 +613,8 @@ proc replaceTypeVarsTAux(cl: var TReplTypeVars, t: PType): PType = of tyObject, tyTuple: propagateFieldFlags(result, result.n) + if result.kind == tyObject and cl.c.computeRequiresInit(cl.c, result): + result.flags.incl tfRequiresInit of tyProc: eraseVoidParams(result) diff --git a/tests/constructors/tinvalid_construction.nim b/tests/constructors/tinvalid_construction.nim index 6716bfb452fc6..e98b530bb5e3c 100644 --- a/tests/constructors/tinvalid_construction.nim +++ b/tests/constructors/tinvalid_construction.nim @@ -10,6 +10,9 @@ type TRefObj = ref object x: int + IllegalToConstruct = object + x: cstring not nil + THasNotNils = object of RootObj a: TRefObj not nil b: TRefObj not nil @@ -89,6 +92,10 @@ proc userDefinedDefault(T: typedesc): T = proc genericDefault(T: typedesc): T = result = default(T) +reject IllegalToConstruct() +reject: + var x: IllegalToConstruct + accept TObj() accept TObj(choice: A) reject TObj(choice: A, bc: 10) # bc is in the wrong branch @@ -256,6 +263,111 @@ reject TNestedChoices(outerChoice: false, innerChoice: C) accept TNestedChoices(outerChoice: false, innerChoice: C, notnil: notNilRef) reject TNestedChoices(outerChoice: false, innerChoice: C, notnil: nil) +# Tests involving generics and sequences: +# +block: + # This test aims to show that it's possible to instantiate and + # use a sequence with a requiresInit type: + + var legalSeq: seq[IllegalToConstruct] + legalSeq.add IllegalToConstruct(x: "one") + var two = IllegalToConstruct(x: "two") + legalSeq.add two + var one = legalSeq[0] + var twoAgain = legalSeq.pop + + # It's not possible to tell the sequence to create elements + # for us though: + reject: + var illegalSeq = newSeq[IllegalToConstruct](10) + + reject: + var illegalSeq: seq[IllegalToConstruct] + newSeq(illegalSeq, 10) + + reject: + var illegalSeq: seq[IllegalToConstruct] + illegalSeq.setLen 10 + + # You can still use newSeqOfCap to write efficient code: + var anotherLegalSequence = newSeqOfCap[IllegalToConstruct](10) + for i in 0..9: + anotherLegalSequence.add IllegalToConstruct(x: "x") + +type DefaultConstructible[yesOrNo: static[bool]] = object + when yesOrNo: + x: string + else: + x: cstring not nil + +block: + # Constructability may also depend on the generic parameters of the type: + accept: + var a: DefaultConstructible[true] + var b = DefaultConstructible[true]() + var c = DefaultConstructible[true](x: "test") + var d = DefaultConstructible[false](x: "test") + + reject: + var y: DefaultConstructible[false] + + reject: + var y = DefaultConstructible[false]() + +block: + type + Hash = int + + HashTableSlotType = enum + Free = Hash(0) + Deleted = Hash(1) + HasKey = Hash(2) + + KeyValuePair[A, B] = object + key: A + case hash: HashTableSlotType + of Free, Deleted: + discard + else: + value: B + + # The above KeyValuePair is an interesting type because it + # may become unconstructible depending on the generic parameters: + accept KeyValuePair[int, string](hash: Deleted) + accept KeyValuePair[int, IllegalToConstruct](hash: Deleted) + + accept KeyValuePair[int, string](hash: HasKey) + reject KeyValuePair[int, IllegalToConstruct](hash: HasKey) + + # Since all the above variations don't have a non-constructible + # field in the default branch of the case object, we can construct + # such values: + accept KeyValuePair[int, string]() + accept KeyValuePair[int, IllegalToConstruct]() + accept KeyValuePair[DefaultConstructible[true], string]() + accept KeyValuePair[DefaultConstructible[true], IllegalToConstruct]() + + var a: KeyValuePair[int, string] + var b: KeyValuePair[int, IllegalToConstruct] + var c: KeyValuePair[DefaultConstructible[true], string] + var d: KeyValuePair[DefaultConstructible[true], IllegalToConstruct] + var s1 = newSeq[KeyValuePair[int, IllegalToConstruct]](10) + var s2 = newSeq[KeyValuePair[DefaultConstructible[true], IllegalToConstruct]](10) + + # But let's put the non-constructible values as keys: + reject KeyValuePair[IllegalToConstruct, int](hash: Deleted) + reject KeyValuePair[IllegalToConstruct, int]() + + type IllegalPair = KeyValuePair[DefaultConstructible[false], string] + + reject: + var x: IllegalPair + + reject: + var s = newSeq[IllegalPair](10) + +# Specific issues: +# block: # https://github.com/nim-lang/Nim/issues/11428 type From 88ff797a1d65c7788a2fae5f852dfa3ad4735793 Mon Sep 17 00:00:00 2001 From: Zahary Karadjov Date: Wed, 1 Apr 2020 03:33:32 +0300 Subject: [PATCH 17/20] Turn some of the errors back into warnings --- compiler/lineinfos.nim | 6 +++ compiler/semexprs.nim | 14 ------ compiler/semmagic.nim | 17 +++++++- compiler/semobjconstr.nim | 5 +++ compiler/sempass2.nim | 6 +-- tests/constructors/tinvalid_construction.nim | 46 ++++++++++++++------ tests/objects/tobjects_issues.nim | 2 +- 7 files changed, 63 insertions(+), 33 deletions(-) diff --git a/compiler/lineinfos.nim b/compiler/lineinfos.nim index 585a95c806eee..658863676d786 100644 --- a/compiler/lineinfos.nim +++ b/compiler/lineinfos.nim @@ -37,6 +37,8 @@ type warnUnusedImportX, warnInheritFromException, warnEachIdentIsTuple, + warnUnsafeSetLen, + warnUnsafeDefault, warnProveInit, warnProveField, warnProveIndex, warnStaticIndexCheck, warnGcUnsafe, warnGcUnsafe2, warnUninit, warnGcMem, warnDestructor, warnLockLevel, warnResultShadowed, @@ -87,6 +89,9 @@ const warnUnusedImportX: "imported and not used: '$1'", warnInheritFromException: "inherit from a more precise exception type like ValueError, IOError or OSError", warnEachIdentIsTuple: "each identifier is a tuple", + warnUnsafeSetLen: "setLen can potentially expand the sequence, " & + "but the element type '$1' doesn't have a valid default value", + warnUnsafeDefault: "The '$1' type doesn't have a valid default value", warnProveInit: "Cannot prove that '$1' is initialized. This will become a compile time error in the future.", warnProveField: "cannot prove that field '$1' is accessible", warnProveIndex: "cannot prove index '$1' is valid", @@ -146,6 +151,7 @@ const "TypelessParam", "UseBase", "WriteToForeignHeap", "UnsafeCode", "UnusedImport", "InheritFromException", "EachIdentIsTuple", + "UnsafeSetLen", "UnsafeDefault", "ProveInit", "ProveField", "ProveIndex", "IndexCheck", "GcUnsafe", "GcUnsafe2", "Uninit", "GcMem", "Destructor", "LockLevel", "ResultShadowed", diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index 56de00d56231c..5f82eb1e7d72a 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -2267,20 +2267,6 @@ proc semMagic(c: PContext, n: PNode, s: PSym, flags: TExprFlags): PNode = of mSizeOf: markUsed(c, n.info, s) result = semSizeof(c, setMs(n, s)) - of mSetLengthSeq: - result = semDirectOp(c, n, flags) - let seqType = result[1].typ.skipTypes({tyPtr, tyRef, # in case we had auto-dereferencing - tyVar, tyGenericInst, tyOwned, tySink, - tyAlias, tyUserTypeClassInst}) - if seqType.kind == tySequence and seqType.base.requiresInit: - localError(c.config, n.info, "setLen can potentially expand the sequence, " & - "but the element type $1 doesn't have a default value.", - [typeToString(seqType.base)]) - of mDefault: - result = semDirectOp(c, n, flags) - c.config.internalAssert result[1].typ.kind == tyTypeDesc - if result[1].typ.base.requiresInit: - localError(c.config, n.info, "not nil types don't have a default value") else: result = semDirectOp(c, n, flags) diff --git a/compiler/semmagic.nim b/compiler/semmagic.nim index 8c07d7d183744..7838466501083 100644 --- a/compiler/semmagic.nim +++ b/compiler/semmagic.nim @@ -530,4 +530,19 @@ proc magicsAfterOverloadResolution(c: PContext, n: PNode, result = semQuantifier(c, n) of mOld: result = semOld(c, n) - else: result = n + of mSetLengthSeq: + result = n + let seqType = result[1].typ.skipTypes({tyPtr, tyRef, # in case we had auto-dereferencing + tyVar, tyGenericInst, tyOwned, tySink, + tyAlias, tyUserTypeClassInst}) + if seqType.kind == tySequence and seqType.base.requiresInit: + message(c.config, n.info, warnUnsafeSetLen, typeToString(seqType.base)) + of mDefault: + result = n + c.config.internalAssert result[1].typ.kind == tyTypeDesc + let constructed = result[1].typ.base + if constructed.requiresInit: + message(c.config, n.info, warnUnsafeDefault, typeToString(constructed)) + else: + result = n + diff --git a/compiler/semobjconstr.nim b/compiler/semobjconstr.nim index eb05896011fd5..eeec429006e79 100644 --- a/compiler/semobjconstr.nim +++ b/compiler/semobjconstr.nim @@ -336,6 +336,11 @@ proc semConstructTypeAux(c: PContext, let base = t[0] if base == nil: break t = skipTypes(base, skipPtrs) + if t.kind == tyGenericParam: + # XXX: This is not supposed to happen, but apparently + # there are some issues in semtypinst. Luckily, it + # seems to affect only `computeRequiresInit`. + return constrCtx.needsFullInit = constrCtx.needsFullInit or tfNeedsFullInit in t.flags diff --git a/compiler/sempass2.nim b/compiler/sempass2.nim index 79d969fbb2fb1..a4e1dcf0073ec 100644 --- a/compiler/sempass2.nim +++ b/compiler/sempass2.nim @@ -254,7 +254,7 @@ proc useVar(a: PEffects, n: PNode) = a.init.add s.id elif s.id notin a.init: if s.typ.requiresInit: - localError(a.config, n.info, errProveInit, s.name.s) + message(a.config, n.info, warnProveInit, s.name.s) else: message(a.config, n.info, warnUninit, s.name.s) # prevent superfluous warnings about the same variable: @@ -844,7 +844,7 @@ proc track(tracked: PEffects, n: PNode) = # var s: seq[notnil]; newSeq(s, 0) is a special case! discard else: - localError(tracked.config, arg.info, errProveInit, $arg) + message(tracked.config, arg.info, warnProveInit, $arg) # check required for 'nim check': if n[1].typ.len > 0: @@ -1209,7 +1209,7 @@ proc trackProc*(c: PContext; s: PSym, body: PNode) = s.kind in {skProc, skFunc, skConverter, skMethod}: var res = s.ast[resultPos].sym # get result symbol if res.id notin t.init: - localError(g.config, body.info, errProveInit, "result") + message(g.config, body.info, warnProveInit, "result") let p = s.ast[pragmasPos] let raisesSpec = effectSpec(p, wRaises) if not isNil(raisesSpec): diff --git a/tests/constructors/tinvalid_construction.nim b/tests/constructors/tinvalid_construction.nim index e98b530bb5e3c..4b372d68a61a7 100644 --- a/tests/constructors/tinvalid_construction.nim +++ b/tests/constructors/tinvalid_construction.nim @@ -113,18 +113,18 @@ reject THasNotNils(a: notNilRef, b: nilRef, c: nilRef) # `b` shouldn't be n reject THasNotNils(b: notNilRef, c: notNilRef) # there is a missing not nil field reject THasNotNils() # again, missing fields accept THasNotNils(a: notNilRef, b: notNilRef) # it's OK to omit a non-mandatory field -reject default(THasNotNils) -reject userDefinedDefault(THasNotNils) +# produces only warning: reject default(THasNotNils) +# produces only warning: reject userDefinedDefault(THasNotNils) -reject default(TRefObjNotNil) -reject userDefinedDefault(TRefObjNotNil) -reject genericDefault(TRefObjNotNil) +# produces only warning: reject default(TRefObjNotNil) +# produces only warning: reject userDefinedDefault(TRefObjNotNil) +# produces only warning: reject genericDefault(TRefObjNotNil) # missing not nils in base reject TBaseHasNotNils() -reject default(TBaseHasNotNils) -reject userDefinedDefault(TBaseHasNotNils) -reject genericDefault(TBaseHasNotNils) +# produces only warning: reject default(TBaseHasNotNils) +# produces only warning: reject userDefinedDefault(TBaseHasNotNils) +# produces only warning: reject genericDefault(TBaseHasNotNils) # once you take care of them, it's ok accept TBaseHasNotNils(a: notNilRef, b: notNilRef, choice: D) @@ -163,8 +163,8 @@ accept((ref PartialRequiresInit)(a: 20)) reject((ref PartialRequiresInit)(b: "x")) reject((ref PartialRequiresInit)()) -reject default(PartialRequiresInit) -reject userDefinedDefault(PartialRequiresInit) +# produces only warning: reject default(PartialRequiresInit) +# produces only warning: reject userDefinedDefault(PartialRequiresInit) reject: var obj: PartialRequiresInit @@ -181,8 +181,8 @@ reject((ref FullRequiresInit)(a: 10)) reject((ref FullRequiresInit)(b: 20)) reject((ref FullRequiresInit)()) -reject default(FullRequiresInit) -reject userDefinedDefault(FullRequiresInit) +# produces only warning: reject default(FullRequiresInit) +# produces only warning: reject userDefinedDefault(FullRequiresInit) reject: var obj: FullRequiresInit @@ -192,8 +192,8 @@ reject FullRequiresInitWithParent(a: notNilRef, b: nil, c: nil, e: 10, d: 20) # reject FullRequiresInitWithParent(a: notNilRef, b: notNilRef, e: 10, d: 20) # c should not be missing reject FullRequiresInitWithParent(a: notNilRef, b: notNilRef, c: nil, e: 10) # d should not be missing reject FullRequiresInitWithParent() -reject default(FullRequiresInitWithParent) -reject userDefinedDefault(FullRequiresInitWithParent) +# produces only warning: reject default(FullRequiresInitWithParent) +# produces only warning: reject userDefinedDefault(FullRequiresInitWithParent) reject: var obj: FullRequiresInitWithParent @@ -203,28 +203,36 @@ accept default(TNestedChoices) accept: var obj: TNestedChoices +#[# produces only warning: reject: # This proc is illegal, because it tries to produce # a default object of a type that requires initialization: proc defaultHasNotNils: THasNotNils = discard +#]# +#[# produces only warning: reject: # You cannot cheat by using the result variable to specify # only some of the fields proc invalidPartialTHasNotNils: THasNotNils = result.c = nilRef +#]# +#[# produces only warning: reject: # The same applies for requiresInit types proc invalidPartialRequiersInit: PartialRequiresInit = result.b = "x" +#]# +#[# produces only warning: # All code paths must return a value when the result requires initialization: reject: proc ifWithoutAnElse: THasNotNils = if stdin.readLine == "": return THasNotNils(a: notNilRef, b: notNilRef, c: nilRef) +#]# accept: # All code paths must return a value when the result requires initialization: @@ -234,6 +242,7 @@ accept: else: return THasNotNIls(a: notNilRef, b: notNilRef) +#[# produces only warning: reject: proc caseWithoutAllCasesCovered: FullRequiresInit = # Please note that these is no else branch here: @@ -242,6 +251,7 @@ reject: return FullRequiresInit(a: 10, b: 20) of "y": return FullRequiresInit(a: 30, b: 40) +#]# accept: proc wellFormedCase: FullRequiresInit = @@ -276,18 +286,24 @@ block: var one = legalSeq[0] var twoAgain = legalSeq.pop + #[# produces only warning: # It's not possible to tell the sequence to create elements # for us though: reject: var illegalSeq = newSeq[IllegalToConstruct](10) + #]# + #[# produces only warning: reject: var illegalSeq: seq[IllegalToConstruct] newSeq(illegalSeq, 10) + #]# + #[# produces only warning: reject: var illegalSeq: seq[IllegalToConstruct] illegalSeq.setLen 10 + #]# # You can still use newSeqOfCap to write efficient code: var anotherLegalSequence = newSeqOfCap[IllegalToConstruct](10) @@ -363,8 +379,10 @@ block: reject: var x: IllegalPair + #[# produces only warning: reject: var s = newSeq[IllegalPair](10) + #]# # Specific issues: # diff --git a/tests/objects/tobjects_issues.nim b/tests/objects/tobjects_issues.nim index fddfff7d51447..f1a416d04838d 100644 --- a/tests/objects/tobjects_issues.nim +++ b/tests/objects/tobjects_issues.nim @@ -113,5 +113,5 @@ block t3038: Type = ref object of RootObj SubType[T] = ref object of Type data: Data[T] - SubSubType = ref object of SubType + SubSubType = ref object of SubType[int] SubSubSubType = ref object of SubSubType From 805447bc6f69bac44ab57e92a33152da9be97637 Mon Sep 17 00:00:00 2001 From: Zahary Karadjov Date: Wed, 1 Apr 2020 04:01:25 +0300 Subject: [PATCH 18/20] The raises list can now use expressions referencing the generic params --- compiler/pragmas.nim | 11 +++++++---- compiler/sem.nim | 1 + compiler/semdata.nim | 2 ++ compiler/sempass2.nim | 7 ++++++- 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/compiler/pragmas.nim b/compiler/pragmas.nim index 100ce156021db..6816a1705d743 100644 --- a/compiler/pragmas.nim +++ b/compiler/pragmas.nim @@ -637,10 +637,13 @@ proc processPragma(c: PContext, n: PNode, i: int) = proc pragmaRaisesOrTags(c: PContext, n: PNode) = proc processExc(c: PContext, x: PNode) = - var t = skipTypes(c.semTypeNode(c, x, nil), skipPtrs) - if t.kind != tyObject: - localError(c.config, x.info, errGenerated, "invalid type for raises/tags list") - x.typ = t + if c.hasUnresolvedArgs(c, x): + x.typ = makeTypeFromExpr(c, x) + else: + var t = skipTypes(c.semTypeNode(c, x, nil), skipPtrs) + if t.kind != tyObject and not t.isMetaType: + localError(c.config, x.info, errGenerated, "invalid type for raises/tags list") + x.typ = t if n.kind in nkPragmaCallKinds and n.len == 2: let it = n[1] diff --git a/compiler/sem.nim b/compiler/sem.nim index 471a300995721..1ced234f57463 100644 --- a/compiler/sem.nim +++ b/compiler/sem.nim @@ -521,6 +521,7 @@ proc myOpen(graph: ModuleGraph; module: PSym): PPassContext = c.semGenerateInstance = generateInstance c.semTypeNode = semTypeNode c.instTypeBoundOp = sigmatch.instTypeBoundOp + c.hasUnresolvedArgs = hasUnresolvedArgs pushProcCon(c, module) pushOwner(c, c.module) diff --git a/compiler/semdata.nim b/compiler/semdata.nim index fbc9762f3bd24..7ec4c423f9813 100644 --- a/compiler/semdata.nim +++ b/compiler/semdata.nim @@ -104,6 +104,8 @@ type semTryExpr*: proc (c: PContext, n: PNode, flags: TExprFlags = {}): PNode {.nimcall.} semTryConstExpr*: proc (c: PContext, n: PNode): PNode {.nimcall.} computeRequiresInit*: proc (c: PContext, t: PType): bool {.nimcall.} + hasUnresolvedArgs*: proc (c: PContext, n: PNode): bool + semOperand*: proc (c: PContext, n: PNode, flags: TExprFlags = {}): PNode {.nimcall.} semConstBoolExpr*: proc (c: PContext, n: PNode): PNode {.nimcall.} # XXX bite the bullet semOverloadedCall*: proc (c: PContext, n, nOrig: PNode, diff --git a/compiler/sempass2.nim b/compiler/sempass2.nim index a4e1dcf0073ec..86149330e5fa5 100644 --- a/compiler/sempass2.nim +++ b/compiler/sempass2.nim @@ -1083,7 +1083,12 @@ proc track(tracked: PEffects, n: PNode) = for i in 0.. Date: Wed, 1 Apr 2020 04:45:44 +0300 Subject: [PATCH 19/20] Fix tests/types/tparameterizedparent0 --- compiler/semobjconstr.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/semobjconstr.nim b/compiler/semobjconstr.nim index eeec429006e79..682e74440858c 100644 --- a/compiler/semobjconstr.nim +++ b/compiler/semobjconstr.nim @@ -336,7 +336,7 @@ proc semConstructTypeAux(c: PContext, let base = t[0] if base == nil: break t = skipTypes(base, skipPtrs) - if t.kind == tyGenericParam: + if t.kind != tyObject: # XXX: This is not supposed to happen, but apparently # there are some issues in semtypinst. Luckily, it # seems to affect only `computeRequiresInit`. From e4054cfa9d706785b11888a618dde57106d8bd04 Mon Sep 17 00:00:00 2001 From: Andreas Rumpf Date: Wed, 1 Apr 2020 14:34:24 +0200 Subject: [PATCH 20/20] revert stdlib changes which are not required anymore --- lib/pure/collections/sequtils.nim | 5 +++-- lib/pure/collections/sharedtables.nim | 18 ++++++++++-------- lib/pure/ioselects/ioselectors_epoll.nim | 3 ++- lib/pure/ioselects/ioselectors_kqueue.nim | 3 ++- lib/pure/ioselects/ioselectors_select.nim | 8 +------- lib/pure/parsecsv.nim | 4 ++-- 6 files changed, 20 insertions(+), 21 deletions(-) diff --git a/lib/pure/collections/sequtils.nim b/lib/pure/collections/sequtils.nim index 893f31389155b..e32c784c65c4c 100644 --- a/lib/pure/collections/sequtils.nim +++ b/lib/pure/collections/sequtils.nim @@ -371,8 +371,9 @@ proc map*[T, S](s: openArray[T], op: proc (x: T): S {.closure.}): b = map(a, proc(x: int): string = $x) assert b == @["1", "2", "3", "4"] - result = newSeqOfCap[S](s.len) - for elem in s: result.add op(elem) + newSeq(result, s.len) + for i in 0 ..< s.len: + result[i] = op(s[i]) proc apply*[T](s: var openArray[T], op: proc (x: var T) {.closure.}) {.inline.} = diff --git a/lib/pure/collections/sharedtables.nim b/lib/pure/collections/sharedtables.nim index 96f934f503bf0..2a1c0543f3b3b 100644 --- a/lib/pure/collections/sharedtables.nim +++ b/lib/pure/collections/sharedtables.nim @@ -77,7 +77,8 @@ template withValue*[A, B](t: var SharedTable[A, B], key: A, try: var hc: Hash var index = rawGet(t, key, hc) - if index >= 0: + let hasKey = index >= 0 + if hasKey: var value {.inject.} = addr(t.data[index].val) body finally: @@ -103,7 +104,8 @@ template withValue*[A, B](t: var SharedTable[A, B], key: A, try: var hc: Hash var index = rawGet(t, key, hc) - if index >= 0: + let hasKey = index >= 0 + if hasKey: var value {.inject.} = addr(t.data[index].val) body1 else: @@ -117,13 +119,13 @@ proc mget*[A, B](t: var SharedTable[A, B], key: A): var B = withLock t: var hc: Hash var index = rawGet(t, key, hc) - if index >= 0: - result = t.data[index].val + let hasKey = index >= 0 + if hasKey: result = t.data[index].val + if not hasKey: + when compiles($key): + raise newException(KeyError, "key not found: " & $key) else: - when compiles($key): - raise newException(KeyError, "key not found: " & $key) - else: - raise newException(KeyError, "key not found") + raise newException(KeyError, "key not found") proc mgetOrPut*[A, B](t: var SharedTable[A, B], key: A, val: B): var B = ## retrieves value at ``t[key]`` or puts ``val`` if not present, either way diff --git a/lib/pure/ioselects/ioselectors_epoll.nim b/lib/pure/ioselects/ioselectors_epoll.nim index 002bddac52fce..bf13cc83e8161 100644 --- a/lib/pure/ioselects/ioselectors_epoll.nim +++ b/lib/pure/ioselects/ioselectors_epoll.nim @@ -502,7 +502,8 @@ proc contains*[T](s: Selector[T], fd: SocketHandle|int): bool {.inline.} = proc getData*[T](s: Selector[T], fd: SocketHandle|int): var T = let fdi = int(fd) s.checkFd(fdi) - result = s.fds[fdi].data + if fdi in s: + result = s.fds[fdi].data proc setData*[T](s: Selector[T], fd: SocketHandle|int, data: T): bool = let fdi = int(fd) diff --git a/lib/pure/ioselects/ioselectors_kqueue.nim b/lib/pure/ioselects/ioselectors_kqueue.nim index c5e19a011320d..83e15d479c75e 100644 --- a/lib/pure/ioselects/ioselectors_kqueue.nim +++ b/lib/pure/ioselects/ioselectors_kqueue.nim @@ -600,7 +600,8 @@ proc contains*[T](s: Selector[T], fd: SocketHandle|int): bool {.inline.} = proc getData*[T](s: Selector[T], fd: SocketHandle|int): var T = let fdi = int(fd) s.checkFd(fdi) - result = s.fds[fdi].data + if fdi in s: + result = s.fds[fdi].data proc setData*[T](s: Selector[T], fd: SocketHandle|int, data: T): bool = let fdi = int(fd) diff --git a/lib/pure/ioselects/ioselectors_select.nim b/lib/pure/ioselects/ioselectors_select.nim index 6a742df99d5c0..02a853b42948d 100644 --- a/lib/pure/ioselects/ioselectors_select.nim +++ b/lib/pure/ioselects/ioselectors_select.nim @@ -410,17 +410,11 @@ else: body proc getData*[T](s: Selector[T], fd: SocketHandle|int): var T = - # The compiler needs this to prove that all code paths return a value - result = (cast[ptr T](0'u))[] - s.withSelectLock(): let fdi = int(fd) for i in 0..= 0 - result = my.row[index] + if index >= 0: + result = my.row[index] when not defined(testing) and isMainModule: import os