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

Improved "unmanaged" handling for delay loading #1852

Closed
wants to merge 1 commit into from

Conversation

peter-sabath
Copy link
Contributor

Checklist
Description of change

Used scoped disabling of managed code handling to ensure no other files get affected.

Used scoped disabling of managed code handling to ensure no other files get affected.
@rvagg rvagg requested a review from joaocgreis August 9, 2019 05:15
@joaocgreis
Copy link
Member

The change looks reasonable to me, from what I understand of the docs it should have the same effect as what is there now.

@peter-sabath I expected the unmanaged pragma to be effective only within this compilation unit. How does this affect other files? Is it a stateful thing in the compiler or does it actually become a runtime setting?

@ipetrovic11 please let me know if this causes any issue to you.

@peter-sabath
Copy link
Contributor Author

@joaocgreis You most likely are right, that only the current compilation unit is affected, but as good practice I use those #pragmas in a scoped way similar to #if #endif markers. This would even work fine if MS decides to compine compilation units like they already do for C# for faster compile times.

@joaocgreis
Copy link
Member

joaocgreis commented Aug 9, 2019

@ipetrovic11
Copy link
Contributor

@joaocgreis just tested, works fine for me with this improvement as well.
I am not proficient enough to comment if this is better practice or not.

@peter-sabath if you believe they will do something like that, I am pretty sure they will be handling solutions like first one, since they would break a lot of projects :)

@peter-sabath
Copy link
Contributor Author

@ipetrovic11 This was just my way of solving it. We ran into the abort problem from the loader. So we should not argue about good style / bad style, but be happy that this nasty problem is solved. So keep it or leave it, your solution also does what is required.

joaocgreis pushed a commit that referenced this pull request Aug 19, 2019
Used scoped disabling of managed code handling to ensure no other
files get affected.

PR-URL: #1852
Reviewed-By: João Reis <reis@janeasystems.com>
@joaocgreis
Copy link
Member

Landed in af876e1.

Thanks!

@joaocgreis joaocgreis closed this Aug 19, 2019
@rvagg rvagg mentioned this pull request Sep 26, 2019
rvagg pushed a commit that referenced this pull request Sep 26, 2019
Used scoped disabling of managed code handling to ensure no other
files get affected.

PR-URL: #1852
Reviewed-By: João Reis <reis@janeasystems.com>
@rvagg rvagg mentioned this pull request Sep 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants