Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
#[link(kind="raw-dylib")] #2627
#[link(kind="raw-dylib")] #2627
Changes from 9 commits
5fa09f4
f631ed4
91a1a38
aa58318
6c38aec
8005182
385a210
b962c7b
68f74a4
21dbfba
20d63b0
bcc70fa
3b456f6
b0ca3b8
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you were to "speculate", what might that look like?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that might be a reference to my delayed=true optional extension discussed above e.g.
#[link(name = "kernel32.dll", kind = "dll", delayed = true)]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd personally prefer the option where the caller of the function chooses whether to do a lazy loaded call of it, and not having to choose at declaration time whether it is lazy loaded. I don't know what the syntax would look like for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@retep998 Can you say a bit more about that? I'm curious as to why that's useful. Going that way would seem to preclude (or at least complicate) the compiler from being able to optimize all the generated thunks so they are not doing any repeated LoadLibrary/GetProcAddress calls. It's likely you have a use case in mind that I've not come across before which warrants giving up that benefit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The compiler could still ensure there is only one
LoadLibrary
for a given dll and oneGetProcAddress
for each symbol. Allowing the caller to choose whether they want to lazy load it does not prevent the compiler from ensuringGetProcAddress
is only called once. If nodylib
crates are involved, the generation ofGetProcAddress
thunks can be lazily deferred all the way until binary creation time. Ifdylib
crates are involved, then either thedylib
that contains the crate with the declarations would have to generate all the thunks, or there could potentially be some duplication acrossdylib
s. Since nobody usesdylib
exceptrustc
and people who accidentally used it when they meant to usecdylib
, it doesn't seem like too big of an issue.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As for why, it depends on whether the caller is able to deal with the function not existing. If the caller has a fallback, then it can use the lazy loaded versions and fallback if it fails to load. If the caller doesn't have any fallback, then it can use the more efficient static version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zunzster The user can also write such thunks themselves. It would be preferable for the user to write them since they can choose how to handle the LL/GPA failure if the DLL / function doesn't exist (crash / no-op / return a custom
Err()
) rather than have the compiler enforce a specific choice.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Arnavion Oh, sure. I know users can write the LL/GPA thunks themselves. I've done that many a time myself. It's just tedious and hence sometimes error prone since writing robust error handling around each API call is annoying. Hence, it's nice if the compiler provides a 'pit of success' offering convenient lazy loading with robust error handling as the default.
@retep998 If you're making the lazy loading transparent to the user, you can't play with the return values since they're already a concrete type defined by some random API. So, yes, having a standard 'missing_API_handler' hook which the user can potentially override is nice. You can make the hook a no-op or your own custom handler (similar to panic handlers I suppose) but the default one should probably panic with a descriptive error and back trace.
Actually, I can't see the no-op change being especially easy though with regard to specifying what the return value (if any) should be in case of a missing API.
Some Windows API return BOOL (typedefed Integers) with 0 meaning failure.
Some APIs return HANDLE with -1 (aka INVALID_HANDLE_VALUE) for failure.
So, offering the no-op case would seem to require inelegant/complicated declarative support. I think if users want that kind of no-op behavior, they probably should have to do their own LL/GPA handling since the alternative isn't well-specified enough to be safe.
Of course, this is all just my opinion. Maybe there is a better solution I'm just not seeing since I'm used to the Delphi approach. When all you have is a hammer, every problem can start to look like a nail. :-)