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

Exported functions should be public by default #2989

Open
FireFox317 opened this issue Aug 1, 2019 · 1 comment
Open

Exported functions should be public by default #2989

FireFox317 opened this issue Aug 1, 2019 · 1 comment
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@FireFox317
Copy link
Contributor

The windows subsystem detection did not properly work, because the exported WinMain function was not defined with the pub keyword. This caused the @hasDecl builtin to not find the WinMain function and caused the subsystem detection to be wrong. The following example is relevant:

const std = @import("std");
const builtin = @import("builtin");
usingnamespace std.os.windows;

extern "user32" stdcallcc fn MessageBoxA(hWnd: ?HANDLE, lpText: ?LPCTSTR, lpCaption: ?LPCTSTR, uType: UINT) c_int;

export fn WinMain(hInstance: HINSTANCE, hPrevInstance: HINSTANCE, lpCmdLine: PWSTR, nCmdShow: INT) INT {
    _ = MessageBoxA(null, c"test", c"title", 0);
    @compileLog(subsystem);
    return 0;
}

Without the pub in front of export fn WinMain(... this showed that the subsystem was .Console, whereas with the pub it correctly showed .Windows.

Solution

  1. Always mark a exported function as public internally in the compiler.
  2. Give an error when a exported function is not defined with the pub keyword.
  3. Change the implementation of @hasDecl to also return true for exported functions.

I would like to implement the fix for this, once I know which solution is the best.

@SamTebbs33
Copy link
Contributor

SamTebbs33 commented Aug 2, 2019

I vote for option 3 as export and pub have AFAIK different semantics: pub means that the symbol is visible to importers and export means that the symbol is visible to extern declarations and the linker. It seems most appropriate to change @hasDecl as conflating extern and pub isn't a good idea.

@andrewrk andrewrk added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Aug 2, 2019
@andrewrk andrewrk added this to the 0.6.0 milestone Aug 2, 2019
@andrewrk andrewrk modified the milestones: 0.6.0, 0.7.0 Feb 10, 2020
@andrewrk andrewrk modified the milestones: 0.7.0, 0.8.0 Oct 27, 2020
@andrewrk andrewrk modified the milestones: 0.8.0, 0.9.0 May 19, 2021
@andrewrk andrewrk modified the milestones: 0.9.0, 0.10.0 Nov 23, 2021
@andrewrk andrewrk modified the milestones: 0.10.0, 0.11.0 Apr 16, 2022
@andrewrk andrewrk modified the milestones: 0.11.0, 0.12.0 Apr 9, 2023
@andrewrk andrewrk modified the milestones: 0.13.0, 0.12.0 Jul 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

No branches or pull requests

3 participants