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.

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

exportc now mangles same as importc, fixes #10578 #12144

Merged
merged 5 commits into from
Sep 6, 2019

Conversation

timotheecour
Copy link
Member

@timotheecour timotheecour commented Sep 5, 2019

[EDIT] note

  • multiple {.emit: "NIM_EXTERNC".} is harmless so it's ok to have:
{.emit: "NIM_EXTERNC".}  # this can be omitted after this PR but does not cause harm after this PR
proc fun() {.exportc.} = discard

@Araq
Copy link
Member

Araq commented Sep 5, 2019

This is a breaking change. It needs a changelog entry and it requires us to introduce externCpp to compensate for the loss of functionality.

@timotheecour
Copy link
Member Author

timotheecour commented Sep 6, 2019

This is a breaking change. It needs a changelog entry and it requires us to introduce externCpp to compensate for the loss of functionality.

done.

  • I'm assuming you meant exportcpp, not externCpp (since we have wExportNims, wExportc already), I've added {.exportcpp.}

  • I refactored a bit the pragma sets in pragmas.nim to reuse a common declPragmas which makes it easier to spot the common part vs different parts; To avoid changing semantics (except for the added wExportcpp), I had to exclude some elements from lambdaPragmas and fieldPragmas, not clear to me whether these missing pragmas were omissions or intended, please double check

  • Instead of adding sfExportCpp and patching in many places, it was much simpler to add a sfMangleCpp that can combine with sfExportC; sfMangleCpp is then used in a single place

  • importcpp never worked for nim cpp (at least for free functions) and this PR doesn't try to fix it; making it work is left for future PR's; so I used instead this in the test:
    proc funx1() {.importc: "_Z5funx1v".}

  • [EDIT] exportcpp produces an error with nim c;

note

see timotheecour@fd86320 for an alternative to producing an error:
I had tried something different but it didn't work and would've required a bigger change, which can be done in future PR since it's a new (and more niche) use case anyway : instead of localError, trigger optMixedMode as follows:

  if c.config.cmd == cmdCompileToC:
    let m = s.getModule()
    incl(m.flags, sfCompileToCpp)
  incl c.config.globalOptions, optMixedMode

and adjust requiresExternC; however this didn't work, and (to my surprise) depends on ordering of declarations, eg:

proc fun1() {.exportc.} = discard # in cgen, will be processed as if sfCompileToCpp notin m
proc funx1() {.exportcpp.} = discard # triggers sfCompileToCpp in m
proc fun1() {.exportc.} = discard # in cgen, will be processed with sfCompileToCpp in m

not sure why cgen doesn't come after semantic is completed, but that can be investigated later to enable that niche use case

@Araq
Copy link
Member

Araq commented Sep 6, 2019

Thank you!

@Araq Araq merged commit 32769c4 into nim-lang:devel Sep 6, 2019
@kaushalmodi
Copy link
Contributor

kaushalmodi commented Sep 6, 2019

This is awesome! Does it mean that I do not need to scatter my Nim wrappers with this?

const
  externCDecl* = when defined(cpp):
                   """extern "C" $1 $2 $3"""
                 else:
                   """$1 $2 $3"""

# ..

proc foo() {.exportc, codegenDecl: externCDecl.} =
  # ..

i.e. Below will now result in unmangled proc symbols whether I do nim c or nim cpp?

proc foo() {.exportc.} =
  # ..

@timotheecour
Copy link
Member Author

indeed (let me know if that's not the case somehow), except if you need to support nim versions prior to this PR

@timotheecour timotheecour deleted the pr_fix_10578 branch September 6, 2019 16:22
timotheecour added a commit to timotheecour/Nim that referenced this pull request Sep 6, 2019
timotheecour added a commit to timotheecour/Nim that referenced this pull request Sep 6, 2019
timotheecour added a commit to timotheecour/Nim that referenced this pull request Sep 7, 2019
timotheecour added a commit to timotheecour/Nim that referenced this pull request Sep 7, 2019
timotheecour added a commit to timotheecour/Nim that referenced this pull request Sep 7, 2019
timotheecour added a commit to timotheecour/Nim that referenced this pull request Sep 8, 2019
timotheecour added a commit to timotheecour/Nim that referenced this pull request Sep 8, 2019
timotheecour added a commit to timotheecour/Nim that referenced this pull request Sep 8, 2019
timotheecour added a commit to timotheecour/Nim that referenced this pull request Jan 3, 2020
timotheecour added a commit to timotheecour/Nim that referenced this pull request Jan 28, 2020
timotheecour added a commit to timotheecour/Nim that referenced this pull request Mar 10, 2020
timotheecour added a commit to timotheecour/Nim that referenced this pull request May 15, 2020
timotheecour added a commit to timotheecour/Nim that referenced this pull request Jun 11, 2020
timotheecour added a commit to timotheecour/Nim that referenced this pull request Jun 11, 2020
timotheecour added a commit to timotheecour/Nim that referenced this pull request Jun 18, 2020
timotheecour added a commit to timotheecour/Nim that referenced this pull request Jun 18, 2020
timotheecour added a commit to timotheecour/Nim that referenced this pull request Jun 19, 2020
timotheecour added a commit to timotheecour/Nim that referenced this pull request Jun 22, 2020
timotheecour added a commit to timotheecour/Nim that referenced this pull request Jun 23, 2020
timotheecour added a commit to timotheecour/Nim that referenced this pull request Jun 23, 2020
timotheecour added a commit to timotheecour/Nim that referenced this pull request Jul 5, 2020
timotheecour added a commit to timotheecour/Nim that referenced this pull request Nov 2, 2020
timotheecour added a commit to timotheecour/Nim that referenced this pull request Nov 3, 2020
timotheecour added a commit to timotheecour/Nim that referenced this pull request Nov 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

exportc mangles as C++ and doesn't work with importc(C) with nim cpp
3 participants