-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Mark Assembly.GetCallingAssembly
with [RequiresDynamicCode]
.
#71973
Conversation
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
Tagging subscribers to this area: @dotnet/area-system-reflection Issue DetailsI added
|
a68c138
to
31b5aac
Compare
@@ -329,6 +330,7 @@ public enum FileAttribute | |||
Directory = 16, | |||
Archive = 32, | |||
} | |||
[System.Diagnostics.CodeAnalysis.RequiresDynamicCodeAttribute("The FileSystem module is not supported in AOT environments. Use members of the System.IO namespace instead.")] | |||
[Microsoft.VisualBasic.CompilerServices.StandardModuleAttribute] | |||
public sealed partial class FileSystem |
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.
Many but not all APIs of this class use GetCallingAssembly
(some did call it only in debug asserts) but I didn't bother and marked the whole class. This class is very outdated either way.
I wanted to comment when I saw the original issue but I forgot. I don't know if we have the right way to annotate this.
The problem here is different:
I don't know what the right solution is. I'm just having a bit doubts about this one. |
The next best option would be to mark this API obsolete. dotnet/csharplang#4984 would be required for that to have viable replacement. |
Sounds like could use combination of |
Unfortunately WASM not supporting stackwalks is more nuanced - it works when we're running interpreter because we're not using the WASM execution stack. It doesn't work when the code was AOT'd. I would really love it if we could just obsolete this API and not have this problem :/. |
Thanks all, as we don't want to fix the issue this way closing the PR |
Fixes #71290
Fixes #53825
I added
[RequiresDynamicCode]
toAssembly.GetDynamicAssembly
across all runtimes, and also marked all APIs that use it. The only uses of it in non-test code areAssemblyBuilder.DefineDynamicAssembly
which obviously is already marked, and some APIs inMicrosoft.VisualBasic.Core
(c.c. @cston).