Skip to content

Commit

Permalink
internal: less generic-expression workarounds (#821)
Browse files Browse the repository at this point in the history
## Summary

Prevent expressions-depending-on-unresolved-type-variables passed to
`tryConstExpr` from reaching into the compile-time evaluation machinery.
This allows for removing multiple workarounds from `mirgen` and `vmgen`.

## Details

Before `tryConstExpr` passes the expression to `evalConstExpr`, it now
first checks whether the expression either contains unresolved generic
parameters or is an unresolved type-parameter-lookup (e.g., `T.param`).
If either is the case, `nil` (which signals that the expression cannot
be evaluated at compile-time) is returned without passing the
expression to `evalConstExpr`.

While this approach is fundamentally also a workaround (expressions
containing unresolved generic parameters shouldn't be passed to
`tryConstExpr` in the first place), it at least prevents unresolved
expressions from reaching into `transf`, `mirgen`, and `vmgen`.

`mirgen` now doesn't have to push unresolved generic parameters or
type-parameter-lookups as `mnkLiteral`s through the MIR phase, and
`vmgen` doesn't have to special-case them. More generally, `vmgen` can
now treat unexpected nodes reaching it as a defect (via `unreachable`).

Since `vmGenDiagCodeGenUnexpectedSym` and `vmGenDiagCannotGenerateCode`
are now unused, they are, together with everything related to them,
removed.
  • Loading branch information
zerbina authored Jul 31, 2023
1 parent b529dcf commit f02934a
Show file tree
Hide file tree
Showing 8 changed files with 39 additions and 77 deletions.
8 changes: 3 additions & 5 deletions compiler/ast/ast_types.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1046,13 +1046,13 @@ type
adVmGenCannotFindBreakTarget # | module. There should be a
adVmGenNotUnused # | way to cross-reference data
# | without introducing direct
adVmGenCannotGenerateCode # | import or type
# | import or type
adVmGenCannotEvaluateAtComptime # | dependencies. Likely this
# | involves data oriented
adVmGenMissingImportcCompleteStruct # | design, use of handles and
adVmGenCodeGenUnhandledMagic # | the like, along with
# | breaking up the coupling
adVmGenCodeGenUnexpectedSym # | within the `compiler/vm`
# | within the `compiler/vm`
adVmGenCannotImportc # | package between pure VM and
adVmGenTooLargeOffset # | the VM for the compiler.
adVmGenCannotCallMethod # |
Expand All @@ -1064,14 +1064,12 @@ type
adVmGenCannotFindBreakTarget:
discard
of adVmGenNotUnused,
adVmGenCannotGenerateCode,
adVmGenCannotEvaluateAtComptime:
ast*: PNode
of adVmGenMissingImportcCompleteStruct,
adVmGenCodeGenUnhandledMagic:
magic*: TMagic
of adVmGenCodeGenUnexpectedSym,
adVmGenCannotImportc,
of adVmGenCannotImportc,
adVmGenTooLargeOffset,
adVmGenCannotCallMethod:
sym*: PSym
Expand Down
10 changes: 2 additions & 8 deletions compiler/front/cli_reporter.nim
Original file line number Diff line number Diff line change
Expand Up @@ -3968,22 +3968,16 @@ func astDiagToLegacyReport(conf: ConfigRef, diag: PAstDiag): Report {.inline.} =
kind: kind,
location: some location,
reportInst: diag.instLoc.toReportLineInfo)
of adVmGenCodeGenUnexpectedSym,
adVmGenCannotImportc,
of adVmGenCannotImportc,
adVmGenCannotCallMethod,
adVmGenTooLargeOffset:
vmRep = VMReport(
str: case diag.vmGenErr.kind
of adVmGenCodeGenUnexpectedSym:
"Unexpected symbol for VM code - " & $diag.vmGenErr.sym.kind
else:
"",
str: "",
sym: diag.vmGenErr.sym,
location: some location,
reportInst: diag.instLoc.toReportLineInfo,
kind: kind)
of adVmGenNotUnused,
adVmGenCannotGenerateCode,
adVmGenCannotEvaluateAtComptime:
vmRep = VMReport(
ast: diag.vmGenErr.ast,
Expand Down
2 changes: 0 additions & 2 deletions compiler/front/msgs.nim
Original file line number Diff line number Diff line change
Expand Up @@ -421,9 +421,7 @@ func astDiagVmGenToLegacyReportKind*(
of adVmGenCannotFindBreakTarget: rvmCannotFindBreakTarget
of adVmGenNotUnused: rvmNotUnused
of adVmGenTooLargeOffset: rvmTooLargetOffset
of adVmGenCannotGenerateCode: rvmCannotGenerateCode
of adVmGenCodeGenUnhandledMagic: rvmCannotGenerateCode
of adVmGenCodeGenUnexpectedSym: rvmCannotGenerateCode
of adVmGenCannotCast: rvmCannotCast
of adVmGenCannotEvaluateAtComptime: rvmCannotEvaluateAtComptime
of adVmGenCannotImportc: rvmCannotImportc
Expand Down
30 changes: 0 additions & 30 deletions compiler/mir/mirgen.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1510,26 +1510,6 @@ proc genx(c: var TCtx, n: PNode, consume: bool): EValue =
tempNode(c, s.typ, c.tmpMap[s.itemId])
of skProc, skFunc, skConverter, skMethod, skIterator:
procLit(c, s)
of skGenericParam:
case c.context
of skUnknown:
# HACK: during parameter type matching, sigmatch (``paramTypesMatchAux``)
# uses ``tryConstExpr`` in order to find out whether the argument
# expression to a ``static`` parameter is evaluatable at
# compile-time. If the expression contains a reference to an
# unresolved generic parameter, ``vmgen`` is expected to fail.
# The problem: ``mirgen`` is unable to report any errors, so we
# have to push the original ``skGenericParam`` symbol through
# the MIR stage to ``vmgen``
# The likely best solution is to introduce a dedicated analysis
# layer that makes sure that the AST passed to it is valid in the
# context of compile-time execution (i.e. does the checks that
# ``vmgen`` currently has to do). It would take place after
# ``semfold``.
genLit(c, n)
else:
unreachable()

else:
unreachable(s.kind)

Expand Down Expand Up @@ -1849,16 +1829,6 @@ proc generateCode*(graph: ModuleGraph, options: set[GenOption], n: PNode,
# simpler, faster, and more intuitive to either evaluate them directly
# when analying the type expression or during ``semfold``
discard genTypeExpr(c, n)
elif n.typ.kind == tyFromExpr:
assert goGenTypeExpr in options
# a type expression that uses unresolved generic parameters. As we're unable
# to report errors here, we push the expression through to ``vmgen`` as an
# ``mnkLiteral``
# TODO: we shouldn't have to do this here. ``paramTypesMatchAux`` should
# only try to run compile-time evaluation for expressions with no
# unknowns (e.g. unresolved generic parameters)
c.stmts.useSource(c.sp, n)
discard genLit(c, n)
else:
discard genx(c, n)

Expand Down
27 changes: 27 additions & 0 deletions compiler/sem/sem.nim
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,33 @@ proc tryConstExpr(c: PContext, n: PNode): PNode =
result = getConstExpr(c.module, e, c.idgen, c.graph)
if result != nil: return

proc containsUnresolvedTypeVar(n: PNode): bool {.nimcall.} =
## Returns whether the expression `n` contains an unresolved generic
## parameter. Only considers imperative contexts.
case n.kind
of nkSym:
if n.sym.kind == skGenericParam:
return true
of routineDefs, nkImportStmt, nkImportExceptStmt, nkExportStmt,
nkExportExceptStmt, nkFromStmt, nkBindStmt, nkMixinStmt, nkTypeSection,
nkConstSection:
result = false
of nkConv, nkCast, nkHiddenSubConv, nkHiddenStdConv, nkIdentDefs,
nkVarTuple:
result = containsUnresolvedTypeVar(n[^1])
else:
for it in n.items:
if containsUnresolvedTypeVar(it):
return true

if e.typ.kind == tyFromExpr or containsUnresolvedTypeVar(e):
# XXX: a work around for unresolved generic expressions reaching here. They
# shouldn't, but until they don't, we at least prevent them from
# reaching into the compile-time evaluation machinery. Known places
# from which this case is triggered:
# - ``paramTypesMatchAux``
return nil

let oldErrorCount = c.config.errorCounter
let oldErrorMax = c.config.errorMax
let oldErrorOutputs = c.config.m.errorOutputs
Expand Down
8 changes: 2 additions & 6 deletions compiler/vm/vmdef.nim
Original file line number Diff line number Diff line change
Expand Up @@ -567,13 +567,11 @@ type
vmGenDiagCannotFindBreakTarget
# has ast data
vmGenDiagNotUnused
vmGenDiagCannotGenerateCode
vmGenDiagCannotEvaluateAtComptime
# has magic data
vmGenDiagMissingImportcCompleteStruct
vmGenDiagCodeGenUnhandledMagic
# has sym data
vmGenDiagCodeGenUnexpectedSym
vmGenDiagCannotImportc
vmGenDiagTooLargeOffset
vmGenDiagCannotCallMethod
Expand All @@ -586,7 +584,7 @@ type
# diag construction functions -- see: `vmgen.fail`

VmGenDiagKindSymRelated* =
range[vmGenDiagCodeGenUnexpectedSym..vmGenDiagCannotCallMethod]
range[vmGenDiagCannotImportc..vmGenDiagCannotCallMethod]
# TODO: this is a somewhat silly type, the range allows creating type safe
# diag construction functions -- see: `vmgen.fail`

Expand All @@ -605,8 +603,7 @@ type
location*: TLineInfo ## diagnostic location
instLoc*: InstantiationInfo ## instantiation in VM Gen's source
case kind*: VmGenDiagKind
of vmGenDiagCodeGenUnexpectedSym,
vmGenDiagCannotImportc,
of vmGenDiagCannotImportc,
vmGenDiagTooLargeOffset,
vmGenDiagCannotCallMethod:
sym*: PSym
Expand All @@ -616,7 +613,6 @@ type
vmGenDiagCodeGenUnhandledMagic:
magic*: TMagic
of vmGenDiagNotUnused,
vmGenDiagCannotGenerateCode,
vmGenDiagCannotEvaluateAtComptime:
ast*: PNode
of vmGenDiagTooManyRegistersRequired,
Expand Down
19 changes: 3 additions & 16 deletions compiler/vm/vmgen.nim
Original file line number Diff line number Diff line change
Expand Up @@ -2918,17 +2918,8 @@ proc gen(c: var TCtx; n: PNode; dest: var TDest) =
let idx = c.lookupConst(s)
discard c.getOrCreate(s.typ)
c.gABx(n, opcLdCmplxConst, dest, idx)
of skGenericParam:
# note: this can't be replaced with an assert. ``tryConstExpr`` is
# sometimes used to check whether an expression can be evaluated
# at compile-time, in which case we need to report an error when
# encountering an unresolved generic parameter
fail(n.info, vmGenDiagCannotGenerateCode, n)
else:
fail(n.info,
vmGenDiagCodeGenUnexpectedSym,
sym = s
)
unreachable(s.kind)
of nkCall:
if n[0].kind == nkSym:
let s = n[0].sym
Expand Down Expand Up @@ -3069,7 +3060,7 @@ proc gen(c: var TCtx; n: PNode; dest: var TDest) =
of nkPragma, nkAsmStmt:
unused(c, n, dest)
of nkWithSons + nkWithoutSons - codegenExprNodeKinds:
fail(n.info, vmGenDiagCannotGenerateCode, n)
unreachable(n.kind)

proc genStmt*(c: var TCtx; n: PNode): Result[void, VmGenDiag] =
analyseIfAddressTaken(n, c.prc.addressTaken)
Expand Down Expand Up @@ -3291,11 +3282,9 @@ func vmGenDiagToAstDiagVmGenError*(diag: VmGenDiag): AstDiagVmGenError {.inline.
of vmGenDiagTooManyRegistersRequired: adVmGenTooManyRegistersRequired
of vmGenDiagCannotFindBreakTarget: adVmGenCannotFindBreakTarget
of vmGenDiagNotUnused: adVmGenNotUnused
of vmGenDiagCannotGenerateCode: adVmGenCannotGenerateCode
of vmGenDiagCannotEvaluateAtComptime: adVmGenCannotEvaluateAtComptime
of vmGenDiagMissingImportcCompleteStruct: adVmGenMissingImportcCompleteStruct
of vmGenDiagCodeGenUnhandledMagic: adVmGenCodeGenUnhandledMagic
of vmGenDiagCodeGenUnexpectedSym: adVmGenCodeGenUnexpectedSym
of vmGenDiagCannotImportc: adVmGenCannotImportc
of vmGenDiagTooLargeOffset: adVmGenTooLargeOffset
of vmGenDiagCannotCallMethod: adVmGenCannotCallMethod
Expand All @@ -3304,8 +3293,7 @@ func vmGenDiagToAstDiagVmGenError*(diag: VmGenDiag): AstDiagVmGenError {.inline.
{.cast(uncheckedAssign).}: # discriminants on both sides lead to saddness
result =
case diag.kind
of vmGenDiagCodeGenUnexpectedSym,
vmGenDiagCannotImportc,
of vmGenDiagCannotImportc,
vmGenDiagTooLargeOffset,
vmGenDiagCannotCallMethod:
AstDiagVmGenError(
Expand All @@ -3322,7 +3310,6 @@ func vmGenDiagToAstDiagVmGenError*(diag: VmGenDiag): AstDiagVmGenError {.inline.
kind: kind,
magic: diag.magic)
of vmGenDiagNotUnused,
vmGenDiagCannotGenerateCode,
vmGenDiagCannotEvaluateAtComptime:
AstDiagVmGenError(
kind: kind,
Expand Down
12 changes: 2 additions & 10 deletions compiler/vm/vmlegacy.nim
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,7 @@ func vmGenDiagToLegacyReportKind(diag: VmGenDiagKind): ReportKind {.inline.} =
of vmGenDiagCannotFindBreakTarget: rvmCannotFindBreakTarget
of vmGenDiagNotUnused: rvmNotUnused
of vmGenDiagTooLargeOffset: rvmTooLargetOffset
of vmGenDiagCannotGenerateCode: rvmCannotGenerateCode
of vmGenDiagCodeGenUnhandledMagic: rvmCannotGenerateCode
of vmGenDiagCodeGenUnexpectedSym: rvmCannotGenerateCode
of vmGenDiagCannotCast: rvmCannotCast
of vmGenDiagCannotEvaluateAtComptime: rvmCannotEvaluateAtComptime
of vmGenDiagCannotImportc: rvmCannotImportc
Expand Down Expand Up @@ -57,22 +55,16 @@ func vmGenDiagToLegacyVmReport*(diag: VmGenDiag): VMReport {.inline.} =
kind: kind,
location: std_options.some diag.location,
reportInst: diag.instLoc.toReportLineInfo)
of vmGenDiagCodeGenUnexpectedSym,
vmGenDiagCannotImportc,
of vmGenDiagCannotImportc,
vmGenDiagCannotCallMethod,
vmGenDiagTooLargeOffset:
VMReport(
str: case diag.kind
of vmGenDiagCodeGenUnexpectedSym:
"Unexpected symbol for VM code - " & $diag.sym.kind
else:
"",
str: "",
sym: diag.sym,
location: std_options.some diag.location,
reportInst: diag.instLoc.toReportLineInfo,
kind: kind)
of vmGenDiagNotUnused,
vmGenDiagCannotGenerateCode,
vmGenDiagCannotEvaluateAtComptime:
VMReport(
ast: diag.ast,
Expand Down

0 comments on commit f02934a

Please sign in to comment.