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

Feature request: make types marked with exportc always emitted #7448

Open
cooldome opened this issue Mar 30, 2018 · 16 comments
Open

Feature request: make types marked with exportc always emitted #7448

cooldome opened this issue Mar 30, 2018 · 16 comments
Labels

Comments

@cooldome
Copy link
Member

Currently compiler eliminates all unused types. Make it possible to force type definition to be emitted.
Pragma exportc seems to be suitable. Another option is used pragma.

@Araq
Copy link
Member

Araq commented Apr 3, 2018

exportc is what should be used.

@dom96
Copy link
Contributor

dom96 commented Apr 5, 2018

Is this an issue? What's wrong with exportc?

@cooldome
Copy link
Member Author

cooldome commented Apr 5, 2018

Currently exportc has no effect on types, types declarations will be eliminated if compiler will not find the evidence of type usage in the generated procs

@dom96
Copy link
Contributor

dom96 commented Oct 17, 2020

Can confirm that this is annoying. Just tried this:

{.push exportc, used.}

type
  DisplayKind* {.exportc.} = enum
    Text, Circle, RawPixels

  RGB565* {.exportc.}  = uint16

  Display* {.exportc: "Display", used.}  = object
    case kind*: DisplayKind
    of Text:
      text: cstring
      textColor: RGB565
    of Circle:
      radius: uint8
      circleColor: RGB565
    of RawPixels:
      data: array[16, array[16, RGB565]]
    pos: tuple[x, y: uint8]

  AnimationKind* {.exportc.}  = enum
    Scroll,

  Animation* {.exportc.} = object
    case kind*: AnimationKind
    of Scroll: discard

The C code does not have these types.

@timotheecour
Copy link
Member

Exportc affects mangling and shouldnt be conflated with feature request

@HugoP707
Copy link
Contributor

I would be fine with it
what do you suggest instead?

@timotheecour
Copy link
Member

timotheecour commented Oct 17, 2020

Used (extending its current meaning) or export

@disruptek
Copy link
Contributor

Based on @dom96 repro, I don't see how this isn't a bug and not a feature request.

@timotheecour
Copy link
Member

dead code elimination (DCE) (whether for procs or types) is useful, and exportc + exportcpp should IMO only affect mangling, not dead code elimination.
Granted, currently there is an inconsistency between types and procs:

  • proc fn(){.exportc.}=discard inhibits DCE
  • type Foo {.exportc.} = object honors DCE

if we change semantics to make exportc types inhibit DCE, we at least need a new pragma to honor DCE.

Alternatively (IMO, preferably), we can change semantics for exportc procs to honor DCE (with a legacy transition switch), and make {.used.} inhibit DCE. Currently IIUC used is only for modules to prevent import unused.

@Clyybber
Copy link
Contributor

Clyybber commented Oct 18, 2020

{.exportc.} should definitely prevent DCE, after all its use is to export something to C, regardless of wether the Nim side uses it or not.

@Araq
Copy link
Member

Araq commented Oct 19, 2020

Well if you export to C, you want a stable name too so this conflation of concerns is entirely reasonable. Not sure what exporting a type to C brings us though. If you export a proc, it ends up in your binary, if you export a type, it ends up in a single C file, what's the point? We don't generate C header files.

@dom96
Copy link
Contributor

dom96 commented Oct 19, 2020

We don't generate C header files.

We do. There is a --header flag that can be passed to get a header file.

@HugoP707
Copy link
Contributor

It was not supposed to be used and was going to be deprecated iirc

@timotheecour
Copy link
Member

Why? Any links to where this is discussed?

@Araq
Copy link
Member

Araq commented Oct 23, 2020

It's one of these features we cannot maintain and the code generator wasn't build for... It was useful to somebody once and now is a burden. Some lessons to be learned from this here...

@timotheecour
Copy link
Member

deprecating --header should be the type of thing that requires an RFC, IMO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants