-
Notifications
You must be signed in to change notification settings - Fork 206
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
[NativeAOT-LLVM] Fix most warnings for the HelloWasm
runtime test
#2168
[NativeAOT-LLVM] Fix most warnings for the HelloWasm
runtime test
#2168
Conversation
There are two throw helpers used for tentative methods: "ThrowBodyRemoved" and "ThrowInstanceBodyRemoved". We were always using the first one.
We import them manually from a shared location. Usually this is not a problem because batch build (via the build script) does its own restore thing, but it is a problem in case the project is restored on its own, such as when it is built with "dotnet build".
@dotnet/nativeaot-llvm |
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.
LGTM, modulo if @jkotas has any comment on the protected/public comment
@@ -35,11 +35,11 @@ public TentativeInstanceMethodNode(IMethodBodyNode methodNode) | |||
|
|||
public override bool HasConditionalStaticDependencies => true; | |||
|
|||
protected override ISymbolNode GetTarget(NodeFactory factory) | |||
public override IMethodNode GetTarget(NodeFactory factory) |
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.
This visibility change seems harmless to me, but maybe @jkotas you'd like to cast an eye on it?
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.
It looks good to me. @MichalStrehovsky Any concerns?
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.
Nope, looks good. All AssemblyStubNode descendants need some special treatment because on WASM we're not able to generate byte buffers with extra code in it (which is what we do elsewhere).
HelloWasm
runtime testHelloWasm
runtime test
Only two, related to the missing
dotnet_browser_entropy
symbol, remain.Contributes to #2165.