Skip to content

Commit

Permalink
fix #14314 do not analyze importc procs for effects (#14319)
Browse files Browse the repository at this point in the history
  • Loading branch information
timotheecour authored May 12, 2020
1 parent 06dfd31 commit de74362
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 7 deletions.
4 changes: 2 additions & 2 deletions compiler/lineinfos.nim
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ type
errGeneralParseError,
errNewSectionExpected,
errInvalidDirectiveX,
errProveInit,
errProveInit, # deadcode
errGenerated,
errUser,
warnCannotOpenFile,
Expand Down Expand Up @@ -66,7 +66,7 @@ const
errGeneralParseError: "general parse error",
errNewSectionExpected: "new section expected",
errInvalidDirectiveX: "invalid directive: '$1'",
errProveInit: "Cannot prove that '$1' is initialized.",
errProveInit: "Cannot prove that '$1' is initialized.", # deadcode
errGenerated: "$1",
errUser: "$1",
warnCannotOpenFile: "cannot open '$1'",
Expand Down
7 changes: 6 additions & 1 deletion compiler/sempass2.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1151,12 +1151,17 @@ proc initEffects(g: ModuleGraph; effects: PNode; s: PSym; t: var TEffects; c: PC
t.config = g.config
t.c = c

proc hasRealBody(s: PSym): bool =
## also handles importc procs with runnableExamples, which requires `=`,
## which is not a real implementation, refs #14314
result = {sfForward, sfImportc} * s.flags == {}

proc trackProc*(c: PContext; s: PSym, body: PNode) =
let g = c.graph
var effects = s.typ.n[0]
if effects.kind != nkEffectList: return
# effects already computed?
if sfForward in s.flags: return
if not s.hasRealBody: return
if effects.len == effectListLen: return

var t: TEffects
Expand Down
26 changes: 26 additions & 0 deletions tests/misc/mimportc.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#[
this test will grow with more importc+importcpp tests; see driver in trunner.nim
]#

{.emit:"""
struct A {
static int fun0(int a){
return a;
}
static int& fun1(int& a){
return a;
}
};
""".}

proc fun0*(a: cint): int {.importcpp:"A::$1(@)".}
proc fun1*(a: var cint): var int {.importcpp:"A::$1(@)".} =
## some comment; this test is for #14314
runnableExamples: discard

proc main()=
var a = 10.cint
doAssert fun0(a) == a
doAssert fun1(a).addr == a.addr
echo "witness"
main()
14 changes: 10 additions & 4 deletions tests/trunner.nim
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ const mode =
else: static: doAssert false

const testsDir = currentSourcePath().parentDir
const buildDir = testsDir.parentDir / "build"
const nimcache = buildDir / "nimcacheTrunner"
# `querySetting(nimcacheDir)` would also be possible, but we thus
# avoid stomping on other parallel tests

proc runCmd(file, options = ""): auto =
let fileabs = testsDir / file.unixToNativePath
Expand Down Expand Up @@ -108,10 +112,6 @@ else: # don't run twice the same test
block: # nim doc --backend:$backend --doccmd:$cmd
# test for https://github.com/nim-lang/Nim/issues/13129
# test for https://github.com/nim-lang/Nim/issues/13891
const buildDir = testsDir.parentDir / "build"
const nimcache = buildDir / "nimcacheTrunner"
# `querySetting(nimcacheDir)` would also be possible, but we thus
# avoid stomping on other parallel tests
let file = testsDir / "nimdoc/m13129.nim"
for backend in fmt"{mode} js".split:
let cmd = fmt"{nim} doc -b:{backend} --nimcache:{nimcache} -d:m13129Foo1 --doccmd:'-d:m13129Foo2 --hints:off' --usenimcache --hints:off {file}"
Expand All @@ -122,3 +122,9 @@ else: # don't run twice the same test
block: # mak sure --backend works with `nim r`
let cmd = fmt"{nim} r --backend:{mode} --hints:off --nimcache:{nimcache} {file}"
check execCmdEx(cmd) == ("ok3\n", 0)

block: # some importc tests
# issue #14314
let file = testsDir / "misc/mimportc.nim"
let cmd = fmt"{nim} r -b:cpp --hints:off --nimcache:{nimcache} --warningAsError:ProveInit {file}"
check execCmdEx(cmd) == ("witness\n", 0)

0 comments on commit de74362

Please sign in to comment.