Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

compiler/ccgtypes: hide exportc proc unless it has dynlib #13199

Merged
merged 1 commit into from
Jan 20, 2020
Merged

compiler/ccgtypes: hide exportc proc unless it has dynlib #13199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/ccgtypes.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -953,7 +953,7 @@ proc genProcHeader(m: BModule, prc: PSym, asPtr: bool = false): Rope =
result.add "N_LIB_EXPORT "
elif prc.typ.callConv == ccInline or asPtr or isNonReloadable(m, prc):
result.add "static "
elif {sfImportc, sfExportc} * prc.flags == {}:
elif sfImportc notin prc.flags:
result.add "N_LIB_PRIVATE "
var check = initIntSet()
fillLoc(prc.loc, locProc, prc.ast[namePos], mangleName(m, prc), OnUnknown)
Expand Down
4 changes: 4 additions & 0 deletions testament/categories.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,9 @@ proc runBasicDLLTest(c, r: var TResults, cat: Category, options: string) =
var test3 = makeTest("lib/nimhcr.nim", options & " --outdir:tests/dll" & rpath, cat)
test3.spec.action = actionCompile
testSpec c, test3
var test4 = makeTest("tests/dll/visibility.nim", options & " --app:lib" & rpath, cat)
Copy link
Member

@timotheecour timotheecour Jan 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could be a regular test (ie not requiring a custom entry in testament) eg
by doing this compilation as part of the test, see tests/stdlib/tosproc.nim for an example
(or tests/exception/tindexerrorformatbounds.nim or other tests with getCurrentCompilerExe())

Copy link
Collaborator Author

@alaviss alaviss Jan 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to build a dll + an executable out of it, so it has to be a custom run. And I can't build the app without the dll and vice versa.

Copy link
Member

@timotheecour timotheecour Jan 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here's a self contained example that doesn't require testament to be aware of this test file:

when defined(foo):
  proc fun1() {.exportc, dynlib.} = discard
  proc fun2() {.exportc.} = discard
else:
  import std/[strformat,os,strutils]
  const libF = "/tmp/" / (DynlibFormat % "foo")
  const nim = getCurrentCompilerExe()
  const file = currentSourcePath()
  static:
    let cmd = fmt"{nim} c -d:foo --app:lib --passc:-fvisibility=hidden -o:{libF} {file}"
    # echo cmd
    let (output, exitCode) = gorgeEx cmd
    doAssert exitCode == 0, output
  proc fun1() {.importc, dynlib: libF.}
  proc fun2() {.importc, dynlib: libF.}
  fun1()
  fun2() # expected to fail with: could not import: fun2

Copy link
Collaborator Author

@alaviss alaviss Jan 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So a simple, easy to understand 4 lines change in testament + a self-explanatory test sample and a complicated piece of code that only works on *nix?

I'm pretty sure I know what I'd choose.

test4.spec.action = actionCompile
testSpec c, test4

# windows looks in the dir of the exe (yay!):
when not defined(Windows):
Expand All @@ -141,6 +144,7 @@ proc runBasicDLLTest(c, r: var TResults, cat: Category, options: string) =

testSpec r, makeTest("tests/dll/client.nim", options & " --threads:on" & rpath, cat)
testSpec r, makeTest("tests/dll/nimhcr_unit.nim", options & rpath, cat)
testSpec r, makeTest("tests/dll/visibility.nim", options & rpath, cat)

if "boehm" notin options:
# force build required - see the comments in the .nim file for more details
Expand Down
19 changes: 19 additions & 0 deletions tests/dll/visibility.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
discard """
output: "could not import: foo"
exitcode: 1
"""

const LibName {.used.} =
when defined(windows):
"visibility.dll"
elif defined(macosx):
"libvisibility.dylib"
else:
"libvisibility.so"

when compileOption("app", "lib"):
proc foo() {.exportc.} =
echo "failed"
elif isMainModule:
proc foo() {.importc, dynlib: LibName.}
foo()