Fix issue 23177 - ModuleInfo is not exported on Windows #14647
Conversation
|
Thanks for your pull request and interest in making D better, @rikkimax! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please see CONTRIBUTING.md for more information. If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment. Bugzilla references
|
|
Okay the non-Windows behavior looks unchanged, excellent, so nothing there was broken. Windows has however. Two sets of errors I'm seeing:
and
The first may be a red haring, the second on the other hand looks a bit more worrisome. I'll try being a bit more eager on the adding of DllImport to see if that helps. |
|
Any ideas of what I'm messing up @kinke? |
|
Okay I understand why |
| @@ -361,6 +361,7 @@ extern (C++) final class Module : Package | |||
| int needmoduleinfo; | |||
| int selfimports; // 0: don't know, 1: does not, 2: does | |||
| Dsymbol[void*] tagSymTab; /// ImportC: tag symbols that conflict with other symbols used as the index | |||
| Symbol* isym; /// Import version of csym used for DllImport on Windows | |||
There was a problem hiding this comment.
Need to stop accepting dmd-specific back end symbols in the front-end. There are plenty of ways to keep compiler-specific information contained to the code generator.
There was a problem hiding this comment.
This is the existing solution for toImport for a different node and it is aligned with msym that is defined in a parent.
If there is a way to solve this assuming I can get this to go green, please do tell me what to do.
|
Ohhhh kay I think I know why the others are failing.
This is a problem we had in dub. It's generating an import library, which it is attempting to link against rather than the static library. |
Easy fix, which I have locally. Looks like OMF is passing, so the problem is certainly centered around MSVC link/PE-COFF. Still unsure of how to proceed, will need to leave it here for now I think. |
|
Okay not a sporadic dependency at link time issue. Welp I'm all out of ideas. |
| else | ||
| { | ||
| return cast(typeof(return))embeddedImportedModules; | ||
| } |
There was a problem hiding this comment.
This hack won't do. The manual dllimport relocation necessary at runtime affects all imported data symbols; ModuleInfos are just a tiny share of those.
There was a problem hiding this comment.
How is calling toImport which does the dllimport on importedModules member (and only there), affect other symbols?
There was a problem hiding this comment.
What I mean is that you need a general solution for all data symbols, not a hack which might work for ModuleInfos.
There was a problem hiding this comment.
I agree.
That is super out of what I can do and unfortunately, I seem to be the only one willing to even give it a go ;)
This doesn't include any tests, right now I want to know if this breaks anything existing.
If it doesn't, I'll re-add them.
Assuming this works, the patching can be done a different way in the future allowing
ModuleInfo.importedModulesto be@nogcand in doing so simplifying it back to how it was.