Skip to content

Commit

Permalink
FFI: disallow interface specification on locals (#886)
Browse files Browse the repository at this point in the history
## Summary

Change the language specification and implementation to disallow using
the `.nodecl`, `.header`, `.importc`, `.importjs`, `.exportc`,
`.extern`, and `.dynlib` pragmas on local 'var'/'let' definitions --
locals exist only inside routines and thus cannot be foreign.

## Details

Except for parameters, which can be detected through the `skParam` kind,
locals cannot be distinguished from globals through just the symbol
kind. Therefore, they need to be detected through dedicated logic that
inspects the full symbol.

This check is implemented by the new `isLocal` procedure added to the
`pragmas` module. The `sfGlobal` flag cannot be used for detecting
globals at this point, as the flag is only included on the symbols
for declarations outside of routines *after* pragma processing is
finished. Instead, `isLocal` checks for whether the owner of the symbol
is a module -- nonetheless testing for the `sfGlobal` flag ensures that
declarations with `.global` can also use the FFI.

During application of the FFI pragmas, it is now first checked whether
the applied-to symbol is that of a local, and if it is, an error is
returned. The error uses the new `adSemExternalLocalNotAllowed`
diagnostic plus associated report.

Finally, the documentation for the pragmas is updated.

### Code-generator cleanup

- the name mangling in the C code generator is updated to not inspect
  the `extname` for parameters and locals -- the string is now always
  empty for those
- testing for the `sfImportc` and `extNoDecl` flag for locals is removed
  from the JS code generator -- the flags are never present on the
  symbol now

### Other adjustments

The only place in the compiler where using the FFI with locals was
relied on is with the `asyncjs` module. There, `.importjs` is used for
getting access to JavaScript's `undefined` value. However, using a
local is not necessary for achieving this -- a `proc` using `.importjs`
works as well.
  • Loading branch information
zerbina authored Sep 12, 2023
1 parent 341dc66 commit 3d71c76
Show file tree
Hide file tree
Showing 9 changed files with 64 additions and 22 deletions.
4 changes: 3 additions & 1 deletion compiler/ast/ast_types.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1064,6 +1064,7 @@ type
# pragmas
adSemInvalidPragma
adSemIllegalCustomPragma
adSemExternalLocalNotAllowed
adSemStringLiteralExpected
adSemIntLiteralExpected
adSemOnOrOffExpected
Expand Down Expand Up @@ -1341,7 +1342,8 @@ type
adSemExpectedRangeType,
adSemExpectedLabel,
adSemContinueCannotHaveLabel,
adSemUnavailableLocation:
adSemUnavailableLocation,
adSemExternalLocalNotAllowed:
discard
of adSemExpectedIdentifierInExpr:
notIdent*: PNode
Expand Down
2 changes: 2 additions & 0 deletions compiler/ast/report_enums.nim
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,8 @@ type
rsemPropositionExpected
rsemIllegalCustomPragma
## supplied pragma is not a legal custom pragma, and cannot be attached
rsemExternalLocalNotAllowed
## a local is specified to be part of an external interface
rsemNoReturnHasReturn
## a routine marked as no return, has a return type
rsemImplicitPragmaError
Expand Down
6 changes: 2 additions & 4 deletions compiler/backend/ccgtypes.nim
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@ proc mangleParamName(c: ConfigRef; s: PSym): Rope =
## we cannot use 'sigConflicts' here since we don't have access to a BProc.
## Fortunately C's scoping rules are sane enough so that that doesn't
## cause any trouble.
result = s.extname
if result == "":
if true:
var res = s.name.s.mangle
if isKeyword(s.name) or c.cppDefines.contains(res):
res.add "_0"
Expand All @@ -57,8 +56,7 @@ proc mangleParamName(c: ConfigRef; s: PSym): Rope =
proc mangleLocalName(p: BProc; s: PSym): Rope =
assert s.kind in skLocalVars+{skTemp}
#assert sfGlobal notin s.flags
result = s.extname
if result == "":
if true:
var key: string
shallowCopy(key, s.name.s.mangle)
let counter = p.sigConflicts.getOrDefault(key)
Expand Down
2 changes: 1 addition & 1 deletion compiler/backend/jsgen.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1703,7 +1703,7 @@ proc genDef(p: PProc, it: CgNode) =
assert it[0].kind == cnkSym
let v = it[0].sym
let name = mangleName(p.module, v)
if exfNoDecl notin v.extFlags and sfImportc notin v.flags:
if true:
genLineDir(p, it)
genVarInit(p, v, name, it[1])

Expand Down
7 changes: 6 additions & 1 deletion compiler/front/cli_reporter.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1822,6 +1822,10 @@ proc reportBody*(conf: ConfigRef, r: SemReport): string =
of rsemIllegalCustomPragma:
result = "cannot attach a custom pragma to '$1'" % r.symstr

of rsemExternalLocalNotAllowed:
result = "parameters and local 'let'/'var' cannot be part of an" &
" external interface"

of rsemCallingConventionMismatch:
assert false, "REMOVE"

Expand Down Expand Up @@ -3290,7 +3294,8 @@ func astDiagToLegacyReport(conf: ConfigRef, diag: PAstDiag): Report {.inline.} =
adSemExpectedRangeType,
adSemExpectedLabel,
adSemContinueCannotHaveLabel,
adSemUnavailableLocation:
adSemUnavailableLocation,
adSemExternalLocalNotAllowed:
semRep = SemReport(
location: some diag.location,
reportInst: diag.instLoc.toReportLineInfo,
Expand Down
1 change: 1 addition & 0 deletions compiler/front/msgs.nim
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,7 @@ func astDiagToLegacyReportKind*(
of adSemCannotImportItself: rsemCannotImportItself
of adSemInvalidPragma: rsemInvalidPragma
of adSemIllegalCustomPragma: rsemIllegalCustomPragma
of adSemExternalLocalNotAllowed: rsemExternalLocalNotAllowed
of adSemStringLiteralExpected: rsemStringLiteralExpected
of adSemIntLiteralExpected: rsemIntLiteralExpected
of adSemOnOrOffExpected: rsemOnOrOffExpected
Expand Down
37 changes: 34 additions & 3 deletions compiler/sem/pragmas.nim
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,16 @@ proc illegalCustomPragma*(c: PContext; n: PNode, s: PSym): PNode =
n,
PAstDiag(kind: adSemIllegalCustomPragma, customPragma: s))

func isLocal(s: PSym): bool =
## Returns whether `s` is a parameter or local `var`/`let`.
# XXX: ideally, we could rely on ``sfGlobal``, but for declarations
# outside of routines, the flag is currently only included after pragma
# processing
s.kind in skLocalVars and sfGlobal notin s.flags and s.owner.kind != skModule

proc disallowedExternalLocal(c: PContext, n: PNode): PNode =
c.config.newError(n, PAstDiag(kind: adSemExternalLocalNotAllowed))

proc pragmaAsm*(c: PContext, n: PNode): tuple[marker: char, err: PNode] =
## Gets the marker out of an asm stmts pragma list
## Returns ` as the default marker if no other markers are found
Expand Down Expand Up @@ -1063,6 +1073,9 @@ proc applySymbolPragma(c: PContext, sym: PSym, it: PNode): PNode =
let k = whichPragma(it)
case k
of wExportc:
if isLocal(sym):
return disallowedExternalLocal(c, it)

let extLit = semExternName(c.config, getOptionalStrLit(c, it, "$1"))

result =
Expand All @@ -1074,6 +1087,9 @@ proc applySymbolPragma(c: PContext, sym: PSym, it: PNode): PNode =

incl(sym.flags, sfUsed) # avoid wrong hints
of wImportc:
if isLocal(sym):
return disallowedExternalLocal(c, it)

let nameLit = semExternName(c.config, getOptionalStrLit(c, it, "$1"))
case nameLit.kind
of nkError:
Expand All @@ -1099,6 +1115,9 @@ proc applySymbolPragma(c: PContext, sym: PSym, it: PNode): PNode =
processImportCompilerProc(c, sym, name)
result = it
of wExtern:
if isLocal(sym):
return disallowedExternalLocal(c, it)

let name = semExternName(c.config, getStrLitNode(c, it))
result =
if name.isError:
Expand All @@ -1111,6 +1130,9 @@ proc applySymbolPragma(c: PContext, sym: PSym, it: PNode): PNode =
assert sym.kind == skTemplate
incl(sym.flags, sfDirty)
of wImportJs:
if isLocal(sym):
return disallowedExternalLocal(c, it)

# note: don't use ``semExternName`` here. The operand is *not* an
# external name, but rather a *pattern* string
let nameLit = getOptionalStrLit(c, it, "$1")
Expand Down Expand Up @@ -1156,8 +1178,11 @@ proc applySymbolPragma(c: PContext, sym: PSym, it: PNode): PNode =
else:
c.config.newError(it, PAstDiag(kind: adSemAlignRequiresPowerOfTwo))
of wNodecl:
result = noVal(c, it)
incl(sym.extFlags, exfNoDecl)
if isLocal(sym):
result = disallowedExternalLocal(c, it)
else:
result = noVal(c, it)
incl(sym.extFlags, exfNoDecl)
of wPure, wAsmNoStackFrame:
result = noVal(c, it)
incl(sym.flags, sfPure)
Expand Down Expand Up @@ -1189,6 +1214,9 @@ proc applySymbolPragma(c: PContext, sym: PSym, it: PNode): PNode =
result = noVal(c, it)
sym.flags.incl {sfGlobal, sfPure}
of wHeader:
if isLocal(sym):
return disallowedExternalLocal(c, it)

let path = getStrLitNode(c, it) # the path or an error
if path.isError:
return path
Expand Down Expand Up @@ -1219,7 +1247,10 @@ proc applySymbolPragma(c: PContext, sym: PSym, it: PNode): PNode =
result = noVal(c, it)
incl(sym.flags, sfWasForwarded)
of wDynlib:
result = processDynLib(c, it, sym)
if isLocal(sym):
result = disallowedExternalLocal(c, it)
else:
result = processDynLib(c, it, sym)
of wCompilerProc, wCore:
result = noVal(c, it) # compilerproc may not get a string!
cppDefine(c.graph.config, sym.name.s)
Expand Down
20 changes: 11 additions & 9 deletions doc/manual.rst
Original file line number Diff line number Diff line change
Expand Up @@ -6945,8 +6945,8 @@ in C).

nodecl pragma
-------------
The `nodecl` pragma can be applied to almost any symbol (variable, proc,
type, etc.) and is sometimes useful for interoperability with C:
The `nodecl` pragma can be applied to the symbol of procedures, types,
globals, and constants and is sometimes useful for interoperability with C:
It tells Nim that it should not generate a declaration for the symbol in
the C code. For example:

Expand All @@ -6963,8 +6963,9 @@ However, the `header` pragma is often the better alternative.
Header pragma
-------------
The `header` pragma is very similar to the `nodecl` pragma: It can be
applied to almost any symbol and specifies that it should not be declared
and instead, the generated code should contain an `#include`:c:\:
applied to the symbol of procedures, types, globals, and constants and
specifies that it should not be declared and instead, the generated code
should contain an `#include`:c:\:

.. code-block:: Nim
type
Expand Down Expand Up @@ -7340,9 +7341,9 @@ are documented here.

Importc pragma
--------------
The `importc` pragma provides a means to import a proc or a variable
from C. The optional argument is a string containing the C identifier. If
the argument is missing, the C name is the Nim identifier *exactly as
The `importc` pragma provides a means to import a procecdure, type, or global
variable from C. The optional argument is a string containing the C identifier.
If the argument is missing, the C name is the Nim identifier *exactly as
spelled*:

.. code-block::
Expand Down Expand Up @@ -7371,7 +7372,7 @@ is available and a literal dollar sign must be written as ``$$``.

Exportc pragma
--------------
The `exportc` pragma provides a means to export a type, a variable, or a
The `exportc` pragma provides a means to export a type, global, or a
procedure to C. Enums and constants can't be exported. The optional argument
is a string containing the C identifier. If the argument is missing, the C
name is the Nim identifier *exactly as spelled*:
Expand Down Expand Up @@ -7408,6 +7409,7 @@ mangling. The string literal passed to `extern` can be a format string:
In the example, the external name of `p` is set to `prefixp`. Only ``$1``
is available and a literal dollar sign must be written as ``$$``.

Only the name mangling of procedures, globals, and constants can be changed.

Bycopy pragma
-------------
Expand Down Expand Up @@ -7467,7 +7469,7 @@ a static error. Usage with inheritance should be defined and documented.

Dynlib pragma for import
------------------------
With the `dynlib` pragma, a procedure or a variable can be imported from
With the `dynlib` pragma, a procedure or global variable can be imported from
a dynamic library (``.dll`` files for Windows, ``lib*.so`` files for UNIX).
The non-optional argument has to be the name of the dynamic library:

Expand Down
7 changes: 4 additions & 3 deletions lib/js/asyncjs.nim
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ proc replaceReturn(node: var NimNode) =
var son = node[z]
let jsResolve = ident("jsResolve")
if son.kind == nnkReturnStmt:
let value = if son[0].kind != nnkEmpty: nnkCall.newTree(jsResolve, son[0]) else: jsResolve
let value = if son[0].kind != nnkEmpty: nnkCall.newTree(jsResolve, son[0])
else: nnkCall.newTree(jsResolve)
node[z] = nnkReturnStmt.newTree(value)
elif son.kind == nnkAsgn and son[0].kind == nnkIdent and $son[0] == "result":
node[z] = nnkAsgn.newTree(son[0], nnkCall.newTree(jsResolve, son[1]))
Expand Down Expand Up @@ -119,7 +120,7 @@ proc generateJsasync(arg: NimNode): NimNode =
var resolve: NimNode
if isVoid:
resolve = quote:
var `jsResolve` {.importjs: "undefined".}: Future[void]
proc `jsResolve`: Future[void] {.importjs: "(undefined)", used.}
else:
resolve = quote:
proc jsResolve[T](a: T): Future[T] {.importjs: "#", used.}
Expand All @@ -132,7 +133,7 @@ proc generateJsasync(arg: NimNode): NimNode =

if len(code) > 0 and isVoid:
var voidFix = quote:
return `jsResolve`
return `jsResolve`()
result.body.add(voidFix)

let asyncPragma = quote:
Expand Down

0 comments on commit 3d71c76

Please sign in to comment.