Skip to content

Conversation

@MarijnS95
Copy link
Contributor

@MarijnS95 MarijnS95 commented Nov 17, 2020

For #3250 (comment)

WinAdapter shims some types and constructs that returned to applications interacting with libdxcompiler.so. Currently these applications have to "guess" or look into the code to understand how to work with these without provoking incorrect or unexpected behaviour. Even though they are unlikely to change, exporting these functions allow applications to work with Windows APIs "as normal" on Linux, without having to define fallback paths with heavily implementation dependent code.

For example, my usecase of DXC currently has to define these monstrosities:
https://github.com/Traverse-Research/hassle-rs/blob/5cfd0f545e27c54f592e54f73214d129ebfab1a8/src/os.rs#L26-L54

@MarijnS95 MarijnS95 force-pushed the winadapter-export-helper-functions branch from 55ffd35 to 74f1588 Compare November 17, 2020 22:31
@AppVeyorBot
Copy link

@MarijnS95 MarijnS95 force-pushed the winadapter-export-helper-functions branch from 74f1588 to 40e6cee Compare November 18, 2020 08:45
@AppVeyorBot
Copy link

@MarijnS95 MarijnS95 force-pushed the winadapter-export-helper-functions branch from 40e6cee to 76e5193 Compare November 18, 2020 09:47
@AppVeyorBot
Copy link

MarijnS95 added 3 commits May 22, 2021 17:06
The implementation of these support types completely depends on DXC.  As
such applications using libdxcompiler.so have to guess and/or look at
the source code to understand how they are implemented (eg prefix size
for BSTR), and assume this is not going to change.  Applications using
these newly exported symbols can do it "the right way" as if they are on
Windows.
Some functions return lists allocated with CoTaskMemAlloc.  Applications
using libdxcompiler.so currently have to look into the code to
understand that these should be deallocated with free() instead of
delete[].  Export the CoTaskMemFree function so that there is no
guesswork involved anymore: the application can now call this function
and always free it in the right way.
@MarijnS95 MarijnS95 force-pushed the winadapter-export-helper-functions branch from 76e5193 to 3c42468 Compare May 22, 2021 15:06
@MarijnS95 MarijnS95 marked this pull request as ready for review May 22, 2021 15:07
@MarijnS95
Copy link
Contributor Author

Still an RFC as per #3250 (comment), not sure if we actually want this - probably not worth the hassle as we're following the implementation of BSTR exactly.

@AppVeyorBot
Copy link

@damyanp
Copy link
Member

damyanp commented Jan 22, 2026

This PR was closed as it has not been updated in the last two years. Please feel free to reopen if this PR should be merged and is in a reviewable state.

@damyanp damyanp closed this Jan 22, 2026
@github-project-automation github-project-automation bot moved this from New to Done in HLSL Roadmap Jan 22, 2026
@MarijnS95
Copy link
Contributor Author

@damyanp A contributor cannot reopen a PR after it has been closed by a collaborator or member. If I were to rebase and (force)push to the branch GitHub dissociates it from the PR, never allowing it to be reopened again.

If you were to reopen this and I were to rebase it, do you think this change has any chance of being reviewed and considered? It's still something I want to have to get rid of the Linux-specific implementation details in downstream code.

@damyanp
Copy link
Member

damyanp commented Jan 22, 2026

This one seems a little more involved, or least touches on areas I'm less familiar with. I'll have a look around to see if anyone's able to pick this up.

@damyanp damyanp reopened this Jan 22, 2026
@github-project-automation github-project-automation bot moved this from Done to In Progress in Open-Source contributions Jan 22, 2026
@damyanp
Copy link
Member

damyanp commented Jan 22, 2026

I think we might be able to get this through, thanks!

Copy link
Member

@jenatali jenatali left a comment

Choose a reason for hiding this comment

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

This makes sense to me and seems relatively scoped. My only concerns would be interposition of other components exporting these same functions, but on Linux I don't expect that to be a real concern.

@damyanp damyanp self-assigned this Jan 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants